Skip to content

Skip recently committed (COMMITTING) segments in SegmentStatusChecker#18934

Draft
krishan1390 wants to merge 1 commit into
apache:masterfrom
krishan1390:fix-segment-status-checker-committing
Draft

Skip recently committed (COMMITTING) segments in SegmentStatusChecker#18934
krishan1390 wants to merge 1 commit into
apache:masterfrom
krishan1390:fix-segment-status-checker-committing

Conversation

@krishan1390

Copy link
Copy Markdown
Contributor

Summary

SegmentStatusChecker's logic to skip recently created/pushed segments predates the pauseless
COMMITTING status and only special-cased IN_PROGRESS. As a result, a COMMITTING segment — one
that has finished consuming but is still being built and uploaded by the server on a pauseless
table — was mishandled in two places in updateSegmentMetrics:

  1. Wait-for-push grace period. The reference timestamp was
    getStatus() == IN_PROGRESS ? getCreationTime() : getPushTime(). A realtime COMMITTING segment
    has no push time, so getPushTime() returns Long.MIN_VALUE and the segment is never skipped.
    While servers are still loading it, it gets counted as under-replicated/offline
    (SEGMENTS_WITH_LESS_REPLICAS, PERCENT_SEGMENTS_AVAILABLE < 100) and produces WARN log spam.
  2. Time-range validation. Guarded by getStatus() != IN_PROGRESS. A COMMITTING segment's
    start/end time is only finalized once it becomes DONE, so it was flagged as
    SEGMENTS_WITH_INVALID_START_TIME / SEGMENTS_WITH_INVALID_END_TIME.

Fix

Both checks now key off Status.isCompleted() (true only for DONE/UPLOADED), so IN_PROGRESS
and COMMITTING segments are handled the same way. A recently committed segment gets the
wait-for-push grace period and is not time-validated, while a genuinely stuck old COMMITTING
segment is still surfaced as under-replicated (but no longer as invalid-time noise).

Testing

Added SegmentStatusCheckerTest#realtimeCommittingSegmentTest: a recently created COMMITTING
segment is skipped entirely, while an older one is still checked for replica availability but never
flagged for an invalid time range. The test fails without the fix (expected [1] but found [2])
and passes with it. The full SegmentStatusCheckerTest suite (30 tests) passes.

🤖 Generated with Claude Code

SegmentStatusChecker's "skip recently created/pushed segments" logic
predated the pauseless COMMITTING status and only special-cased
IN_PROGRESS. A COMMITTING segment (consumed but still being built and
uploaded by the server) therefore:

- fell into the push-time branch, and since realtime segments have no
  push time, was never skipped during the wait-for-push grace period,
  so it was counted as under-replicated/offline while servers were
  still loading it; and
- was subjected to start/end time-range validation even though start
  and end time are only finalized once the segment becomes DONE, so it
  was flagged with an invalid time range.

Both checks now key off Status.isCompleted() (DONE/UPLOADED), so
IN_PROGRESS and COMMITTING segments are handled the same way, removing
the false alerts and log noise for pauseless tables.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.89%. Comparing base (b6c3ca6) to head (f986a92).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18934      +/-   ##
============================================
- Coverage     64.89%   64.89%   -0.01%     
  Complexity     1347     1347              
============================================
  Files          3398     3398              
  Lines        212550   212550              
  Branches      33503    33503              
============================================
- Hits         137927   137926       -1     
- Misses        63481    63489       +8     
+ Partials      11142    11135       -7     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.89% <100.00%> (-0.01%) ⬇️
temurin 64.89% <100.00%> (-0.01%) ⬇️
unittests 64.88% <100.00%> (-0.01%) ⬇️
unittests1 56.89% <ø> (+0.01%) ⬆️
unittests2 37.23% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@krishan1390 krishan1390 marked this pull request as draft July 8, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants