suvodeep-pyne opened a new pull request, #19740:
URL: https://github.com/apache/pinot/pull/19740
## Summary
On pauseless tables, a commit start whose IdealState update fails leaves the
segment COMMITTING. Every later attempt of the commit then failed until
`RealtimeSegmentValidationManager` repaired the partition after
`max.segment.completion.time.millis`. With this PR, the next attempt of the
commit start resumes the earlier one, so the server's own retry commits the
segment.
## Motivation
The pauseless commit start has three steps:
1. Move the segment ZK metadata from IN_PROGRESS to COMMITTING and set its
end offset.
2. Create the ZK metadata of the next segment.
3. Update the IdealState: the committing segment ONLINE, the new segment
CONSUMING.
A pauseless REALTIME table had ~550K segments (IdealState ~5 MB compressed).
While other controllers were deleting segments, each IdealState update attempt
took 6–11 s, and three group-commit batches lost all 10 attempts with `Version
changed while updating ideal state`.
- Each commit in those batches stayed COMMITTING.
- Every retry by the servers failed with `Segment status for segment: …
should be IN_PROGRESS, found: COMMITTING`.
- 18 partitions stopped consuming for 31 to 73 minutes, until the validation
manager repaired them (`max.segment.completion.time.millis` = 30 min).
A controller that stops between step 1 and step 3 leaves the same state.
## Changes
A commit start for a segment that is already COMMITTING with the same end
offset resumes the earlier attempt from step 2. This only applies before the
max segment completion time; after that, the validation manager still owns the
repair. Steps 2 and 3 are made idempotent, so all attempts of a commit start
converge, including attempts on two controllers around a lead change:
- **Same new segment for every attempt.** A resumed attempt reuses the new
segment of an earlier attempt, if there is one. LLC names carry the creation
time with minute granularity, and the new segment was created between step 1
and now, a window bounded by the max segment completion time. So the resumed
attempt probes the ZK path for each minute's name in that window, with one
minute of slack on each side.
- If there is none, it creates the new segment with the current time, as
the first attempt does. Segment names are never backdated, which matters
because brokers treat a segment as new based on the creation time in its name.
- New segment ZK metadata is created atomically, only if absent, and is
never overwritten, because the segment might already be consuming or committed.
- **Idempotent IdealState update.** The updater in
`updateIdealStateOnSegmentCompletion` skips an update that is already applied
(all replicas of the committing segment ONLINE and the new segment present).
Previously, a retry after a write that succeeded but was reported as failed
failed every remaining attempt on `Failed to find instance in CONSUMING state`,
and with group commit it failed every update batched with it. This also applies
to non-pauseless commits.
- **Failure handling after step 3.**
- The controller re-reads the IdealState and proceeds if the update was
applied.
- Otherwise it keeps the new segment's ZK metadata, for the next attempt
or for the validation manager, which already adds such a segment ("ZK metadata
but not in IdealState", the same as after a controller stop between steps 2 and
3).
- It removes the metadata only when the new segment is absent from the
IdealState and the committing segment is no longer CONSUMING. That is the only
state in which no attempt can still add it: adding requires the committing
segment to be CONSUMING, and the IdealState version check rejects a write based
on an older IdealState.
- **Duplicate-sequence segments in the validation manager.** Two new
segments with the same sequence number can still appear in two cases: attempts
on two controllers in different minutes, or a kept new segment racing a
validation repair that started before the segment was created. When several
segments of a partition have the highest sequence number,
`RealtimeSegmentValidationManager` now takes the one in the IdealState as the
latest segment, and logs a warning. Before, it took whichever was listed first,
and could fail every run with `Segment … is a duplicate of existing segment …`.
This also covers duplicates left behind by older code paths.
No ZK metadata is rolled back, so the IdealState version check of step 3
stays the only fence against a concurrent repair by the validation manager. A
retry with a different end offset still fails as before.
Cost: no change on a successful first attempt. A resumed attempt makes one
existence check per minute since step 1 (at most the max segment completion
time in minutes, plus 3). The failure path adds one IdealState read; step 2
already reads the IdealState on every commit.
## Testing
New tests in `PinotLLCRealtimeSegmentManagerTest`:
- Resume after an IdealState update failure. The retry is 2 minutes later
and uses the same new segment, unchanged.
- A first attempt that fails before step 2. The resume 6 minutes later
creates the new segment with its own creation time, and the next resume reuses
it.
- An update that was applied but reported as failed.
- A retry after a completed commit start does not overwrite the new segment.
- Another controller completes the commit start between step 1 and step 2.
The new segment, already committed, is not overwritten.
- Another controller adds the new segment while this attempt's IdealState
update fails. Its ZK metadata is kept.
- The validation manager repairs the partition with another segment. This
attempt's new segment is removed, and the IdealState is unchanged.
- No resume with a different end offset, or after the max segment completion
time.
- The production updater, through group commit and single commit, with a
write that is applied but reported as failed. It retries once and does not
write again.
- A commit start keeps its new segment while a concurrent repair adds
another one with the same sequence number. The validation manager then keeps
the IdealState segment as the latest, whichever order the segments are listed
in.
- The production atomic create, when another attempt creates the segment
between the existence check and the create.
I checked each of the main tests by removing the corresponding part of the
change and confirming the test fails: the updater no-op, the earlier-attempt
probe, create-if-absent, keeping the new segment, and the duplicate-sequence
tie-break.
Existing tests pass: `PinotLLCRealtimeSegmentManagerTest`,
`SegmentCompletionTest`, `SegmentCompletionFSMFactoryTest`,
`RealtimeSegmentValidationManagerTest`, and
`PauselessRealtimeIngestionIntegrationTest`. The integration test injects
failures before steps 2 and 3, and its scenarios still recover.
Related to #<PR2> (validation repair after a failed IdealState write) and
#<PR3> (cheaper IdealState update attempts).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
