Grace pauseless COMMITTING/committed real-time segments in SegmentStatusChecker - #19094
Open
swaminathanmanish wants to merge 1 commit into
Conversation
…tStatusChecker SegmentStatusChecker skips just-created/pushed segments from the replica-availability check for a grace window (waitForPushTime). The grace timestamp was derived as: IN_PROGRESS -> creation time, everything else -> push time. Real-time (LLC) committed segments never populate push time (it stays Long.MIN_VALUE), so a just-committed segment was never graced. Pauseless ingestion makes this worse: a segment enters COMMITTING (done consuming, immutable segment still being built/loaded on the replicas) and is transiently under-replicated for the build window, yet COMMITTING was treated as non-IN_PROGRESS and checked immediately. This produces false percentOfReplicas dips (SegmentReplicasCriticallyLowForHATable) for high-ingest pauseless tables even though the table is fully redundant. Fix: grace the transient window by keying on a populated timestamp: - IN_PROGRESS and COMMITTING -> creation time - committed/pushed -> push time when set, else fall back to creation time The existing waitForPushTime window is reused; once a segment is older than it and still under-replicated it is checked normally, so genuinely stuck commits and real replica losses still alert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19094 +/- ##
============================================
+ Coverage 65.50% 65.52% +0.02%
Complexity 1421 1421
============================================
Files 3430 3430
Lines 218000 218003 +3
Branches 34642 34643 +1
============================================
+ Hits 142799 142856 +57
+ Misses 63641 63594 -47
+ Partials 11560 11553 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
SegmentStatusCheckercomputespercentOfReplicas(which drives theSegmentReplicasCriticallyLowForHATablealert) as a min across segments ofreplicasUpInEV / replicasInIS. It skips just-created/pushed segments for a grace window (controller.statuschecker.waitForPushTimePeriod) so servers have time to load them. The grace timestamp was derived as:Real-time (LLC) committed segments never populate push time (it stays
Long.MIN_VALUE), so a just-committed real-time segment is never graced — it is included in the replica check the moment it commits, while its replicas are still loading.Pauseless ingestion makes this a frequent false positive. A pauseless segment enters the
COMMITTINGstatus ("done consuming, immutable segment not yet committed") and its replicas rebuild/reload the immutable segment for the build window (tens of seconds to a few minutes). During that window it is transiently under-replicated in the ExternalView, butCOMMITTINGwas treated as non-IN_PROGRESSand checked immediately. Because the gauge is a min across segments, one just-committed segment drags the table'spercentOfReplicasdown and fires the alert even though the table is fully redundant. High-ingest tables (a segment sealing every few minutes per partition) page repeatedly.Fix
Key the grace window on a timestamp that is actually populated for these segments:
IN_PROGRESSandCOMMITTING→ creation timeDONEsegments)The existing
waitForPushTimewindow is reused (no new config). Once a segment is older than the window and still under-replicated, it is checked normally — so genuinely stuck commits and real replica losses still alert.Tests
Added to
SegmentStatusCheckerTest:realtimeCommittingSegmentWithinGraceNotUnderReplicated— aCOMMITTINGsegment at 1/3 replicas ONLINE, just created, within the grace window →percentOfReplicasstays 100,segmentsWithLessReplicas= 0 (the false-alert case is now graced).realtimeCommittingSegmentBeyondGraceUnderReplicated— same segment created 2h ago (past the grace) →percentOfReplicasdrops to 33 andsegmentsWithLessReplicas= 1 (a stuck commit is still flagged).Existing
SegmentStatusCheckerTestcases pass unchanged (the change is behavior-preserving for them).Release Notes
Fixes false
SegmentReplicasCriticallyLowForHATable/ lowpercentOfReplicasalerts caused by real-time committed and pauselessCOMMITTINGsegments being counted as under-replicated during their normal post-commit load window.