Skip to content

fix(#4964): hold the Normal high-water mark before a leading sequence gap#4972

Merged
jeremydmiller merged 1 commit into
masterfrom
fix/4964-gapdetector-leading-gap
Jul 17, 2026
Merged

fix(#4964): hold the Normal high-water mark before a leading sequence gap#4972
jeremydmiller merged 1 commit into
masterfrom
fix/4964-gapdetector-leading-gap

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

The bug

The async daemon could silently advance the high-water mark over a committed-but-not-yet-visible event — sealing a skipped event with no mt_high_water_skips row, no is_skipped, no daemon.skipping metric, and no log line. (From the #4953 investigation.)

Root cause

GapDetector's interior-gap query only compares consecutive visible rows:

select seq_id from (select seq_id, lead(seq_id) over (order by seq_id) as no
                    from mt_events where seq_id >= :start) ct
where no is not null and no - seq_id > 1 limit 1;
select max(seq_id) from mt_events where seq_id >= :start;   -- fallback

When the detection Start lands on a hole — a rolled-back/allocated seq_id, or a SafeZone HighestSequence - 32 boundary (GH-3865) that need not be a real event — a gap immediately above Start is invisible to query (1): there is no visible row at Start to pair with the first row above the gap. It falls through to max(seq_id), and the Normal detection path (which records nothing) advances the mark across the gap.

Fix

GapDetector gains a leading-gap probe (min(seq_id) where seq_id > :start) and a HoldBeforeLeadingGap flag. When the first committed sequence above Start sits beyond Start + 1, the Normal path holds at Start instead of advancing:

  • a still-in-flight append fills the gap → the next poll advances normally;
  • a genuinely permanent hole is skipped — and recorded via MarkSkippingAsync / mt_high_water_skips — by the existing SafeZone path once the mark has been stale past StaleSequenceThreshold.

So the Normal mark never crosses an unseen event, and every skip now flows through the observable SafeZone path.

The hold is Normal-path only: HighWaterDetector.findCurrentMark enables it for DetectionType.Normal; DetectInSafeZone explicitly disables it so the SafeZone path keeps its skip-forward-to-max behavior (its whole job is to break a permanent hole and record it).

Tests

Existing GapDetector / high-water tests unchanged (verified locally). Two new GapDetectorTest cases:

  • normal_path_holds_at_start_before_a_leading_gap_when_start_is_a_hole — the repro: Start on a hole with a leading gap holds at Start instead of leaping to max.
  • safe_zone_path_still_skips_a_leading_gap_forward_to_max — guards that the hold does not leak into the SafeZone path.

Closes #4964.

🤖 Generated with Claude Code

… gap

The async daemon could silently advance the high-water mark over a committed-but-not-yet-
visible event, sealing a skipped event with no mt_high_water_skips row, no log line, and no
metric. Root cause is a blind spot in GapDetector: its interior-gap query only compares
consecutive VISIBLE rows, so when the detection Start lands ON a hole (a rolled-back/allocated
seq_id, or a SafeZone "-32" boundary that need not be a real event), a gap immediately above
Start is invisible to it — there is no visible row at Start to pair with the first row above the
gap. It then falls through to max(seq_id) and the Normal mark crosses the gap.

Fix: GapDetector gains a leading-gap probe (min(seq_id) where seq_id > Start) and a
HoldBeforeLeadingGap flag. When the first committed sequence above Start sits beyond Start + 1,
the Normal path holds at Start instead of advancing. A still-in-flight append fills the gap and
the next poll advances normally; a genuinely permanent hole is skipped — and recorded via
MarkSkippingAsync / mt_high_water_skips — by the existing SafeZone path once the mark has been
stale past StaleSequenceThreshold. So every skip now goes through the observable path, and the
Normal mark never crosses an unseen event.

The hold is Normal-path only: HighWaterDetector.findCurrentMark enables it for
DetectionType.Normal, and DetectInSafeZone explicitly disables it so the SafeZone path keeps its
skip-forward-to-max behavior (its whole purpose is to break a permanent hole). Existing
GapDetector / high-water tests are unchanged; two new GapDetector tests cover the leading-gap
hold and the SafeZone skip.

Closes #4964.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XKKBPsFJ83jd2o4hyGJnj6
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.

Async daemon can silently skip a committed event when a partial page's ceiling advances over a visibility gap (from #4953)

1 participant