suvodeep-pyne opened a new pull request, #19739: URL: https://github.com/apache/pinot/pull/19739
## Summary `RealtimeSegmentValidationManager` repairs stuck partitions inside an IdealState updater (`PinotLLCRealtimeSegmentManager.ensureAllPartitionsConsuming`). When the IdealState write of a repair failed and the updater ran again, the retry skipped the segments the failed attempt had just created. The repair then landed only on a later run, one full `max.segment.completion.time.millis` later. This PR makes a retried attempt add those segments right away. ## Motivation For a segment whose ZK metadata is DONE (or COMMITTING on pauseless tables) while the IdealState still has it CONSUMING, the updater creates the ZK metadata of the next segment and adds that segment to the IdealState. On a large IdealState written by several controllers, the write often loses a version conflict (`Version changed while updating ideal state`), and the updater runs again on a fresh IdealState. On the retry, the latest segment of the partition is the one just created. It has ZK metadata but is not in the IdealState, which is the branch for a controller that failed between step 2 and step 3 of a commit. That branch waits until the segment's ZK metadata is older than `max.segment.completion.time.millis`, to avoid racing a commit in flight. So the retry wrote nothing for the partition, and the next run that repaired it came about 30 minutes later on a cluster with `max.segment.completion.time.millis` = 30 min. On a pauseless table with ~550K segments, this happened in 2 of 3 repair runs in one day: - 01:21:00: repair. 01:21:02: version conflict. 01:51:49: repair landed. - 03:08:59: repair. 03:09:01: version conflict. 03:40:27: repair landed. ## Changes - The public `ensureAllPartitionsConsuming` keeps a map of the segments created by its IdealState updater across attempts. Each entry records whether the segment replaces a CONSUMING segment. A segment is recorded as soon as its ZK metadata is created, before instances are assigned to it. - When the latest segment of a partition has ZK metadata but is not in the IdealState, and an earlier attempt of the same update created it, the segment is added right away instead of waiting for the max segment completion time. - The "potential data loss" error and the `LLC_STREAM_DATA_LOSS` meter are still reported when such a segment replaces a CONSUMING segment that is no longer CONSUMING on the retry. They are not reported for segments created for OFFLINE, ONLINE or new partitions, which have no previous CONSUMING segment. ## Testing - `testEnsureAllPartitionsConsumingRetryAddsSegmentsCreatedByEarlierAttempt`: a stuck commit (DONE + CONSUMING) and an all-OFFLINE partition are repaired by a first attempt whose write is discarded, then by a retry on the unchanged IdealState. The retry adds both new segments, creates no other segment, and reports no data loss. Without the change the retry leaves the committing segment CONSUMING. - `testEnsureAllPartitionsConsumingRetryReportsDataLossWhenReplacedSegmentGoesOffline`: data loss is still reported when the replaced segment goes OFFLINE between attempts. - `PinotLLCRealtimeSegmentManagerTest` and `RealtimeSegmentValidationManagerTest` pass. Related: #19170 moves the stream offset fetch out of this updater, which shortens each attempt. Related to #<PR1> and #<PR3>. -- 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]
