fix(#4953): high water detection never crosses outstanding event sequences#4977
Merged
Conversation
Deterministic regressions for every mechanism where high water detection can advance across a sequence reserved by a still-running transaction (discussion 1: GapDetector/statistics commands must be a single statement (single snapshot under READ COMMITTED) so gap checks and the max() fallback cannot see different data mid-command. 2: DetectInSafeZone (invoked threshold-free by rebuilds/catch-up) must hold while the reserving transaction is provably alive, and only skip a DEAD gap after the stale threshold measured from first observation of that gap. 3: the GH-2681 stale fallback must not teleport to (reserved last_value - 32) across in-flight reservations on an idle-then-crushed store; once the tail is provably dead it advances to the reserved ceiling observed with the gap. 4: end-to-end — a rebuild kicked off during an in-flight append waits for it, and the running daemon's stale skip does not cross a merely-slow append (the reporter's pg_sleep trigger technique). 7 of 8 red at this commit; the Normal-path leading-gap hold (#4964) pin stays green. Fixes land in the following commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanism 1 of the #4953 outstanding-sequence skips: GapDetector issued THREE statements in one command and HighWaterStatisticsDetector two. Under READ COMMITTED each statement takes its own snapshot, so commits landing between the leading-gap probe, the interior-gap query, and the max() fallback could defeat both gap checks — the probe would return null (bypassing the #4964 hold, which requires a non-null first-committed-above-start), the interior query would see a contiguous tail, and the fallback would advance the Normal high-water mark over a sequence still held by an in-flight transaction. That skip is silent: no skips row, no log, no metric, and the page loader then seals it by advancing a partial page's ceiling to the high water. Both detectors now issue exactly ONE statement (scalar subqueries over one snapshot), making the three readings mutually consistent by construction. The statistics reading keeps the #4712 guarantees (one row, always-real Timestamp via a VALUES left join; partitioned height from max(seq_id)) and additionally returns pg_snapshot_xmax for the upcoming outstanding-transaction fencing (MartenHighWaterStatistics.CurrentXmax). Behavior of the returned readings is unchanged: same hold rule, same interior gap edge, same max fallback. GapDetectorTest suite + the new structural single-statement regressions are green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dead Mechanisms 2-4 of the #4953 outstanding-sequence skips. DetectInSafeZone is reachable with no staleness gating at all (rebuilds and catch-up drive it via HighWaterAgent.CheckNowAsync) and, even threshold-gated, wall clocks cannot tell a permanently dead sequence hole from one held by a slow in-flight append. The GH-2681 fallback compounded that by teleporting the mark to (reserved last_value - 32) — across sequence numbers whose transactions were still running — gated on mt_event_progression.last_updated, which on an idle store is ancient, so the gate passed instantly on the first crush of traffic. New model, on by default (ProjectionOptions.UseTransactionEvidenceForGapSkipping): - DetectInSafeZone first runs the same gap-holding walk as the Normal path; contiguous committed progress advances with no skipping (rebuild/catch-up on a healthy store behaves exactly as before). - A stuck gap starts a per-gap clock (StuckGapObservation) recording WHEN this detector first saw the mark pinned there, the snapshot xmax, and the reserved sequence ceiling from that same single-snapshot statistics reading. The stale threshold is measured from that observation — never from last_updated. - Past the threshold, a GapLivenessProbe checks for evidence the reserver may still be alive: granted RowExclusiveLocks on the mt_events lineage held by transactions from before the observation (pg_locks, visible to all roles), open transactions from before the observation (pg_stat_activity), and in-progress write xids below the observed xmax (pg_snapshot_xip, pure MVCC data). Any evidence => hold; a merely-slow append is waited out, bounded only by the optional SkipStaleGapsDespiteLiveTransactionsAfter escape hatch. - Proven dead => skip forward, but never beyond the reserved ceiling recorded at observation (numbers reserved later belong to newer, unproven transactions). This replaces the magic -32: a dead tail is now skipped fully and promptly instead of pinning the daemon Stale forever when the tail was shorter than 32. Every skip logs the exact range at Warning. Also: AdvanceHighWaterMarkToLatest now marks the highest COMMITTED sequence, never the reserved last_value; per-call GapDetector instances remove the shared-mutable-state race between the poll loop and concurrent CheckNowAsync calls; the persistence half of calculateHighWaterMark is extracted for reuse. The pre-#4953 behavior is preserved verbatim behind the option opt-out. All 8 Bug_4953 regressions green, including rebuild-during-in-flight-append and the reporter-technique trigger soak — no JasperFx change required for those, since CheckNowAsync's loop now simply waits until the append commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The per-tenant mirror of the store-global change: trySkipStaleTenantGapAsync (the #4867 per-tenant safe-harbor skip under UseTenantPartitionedEvents) now consults the same GapLivenessProbe before skipping a tenant's stale gap. The vectorized per-tenant poll records the snapshot xmax alongside the first-stale sighting (TenantStaleObservation), so the probe is fenced exactly like the store-global path: a tenant gap whose reserving transaction may still be alive is outstanding, not dead, and the skip holds until the transaction resolves or the SkipStaleGapsDespiteLiveTransactionsAfter cap expires. The evidence fence is store-wide (any mt_events lineage lock blocks every tenant's skip), erring conservative. Skip logs are promoted to Warning with the exact range. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… committed ceiling Bumps all four JasperFx pins 2.29.0 -> 2.29.1 (#530): CheckNowAsync (rebuilds, PrepareForRebuildsAsync, catch-up) now loops to the new IHighWaterDetector.FetchCommittedHighWaterCeilingAsync — overridden here with max(seq_id) — instead of the reserved mt_events_sequence last_value, and the loop is time-bounded (CheckNowTimeout, 5 minutes default). Combined with the evidence-gated DetectInSafeZone, a rebuild or forced catch-up during concurrent appends waits for in-flight events instead of pressuring the detection into skipping them. Also in this commit: - Fixes an over-broad pristine-store short-circuit in the new DetectInSafeZone that stopped a store with exactly ONE committed event (last_value = 1, is_called = true) from ever advancing via the safe-zone path — caught by Bug_catching_up_command_errors. - GapLivenessProbe fences pg_locks rows to the current database (relation OIDs are per-database; a numerically-colliding OID from another database could otherwise count as evidence). - HighWaterDetectorTests updated to the #4953 semantics: a stale gap is held on first sighting and skipped only after StaleSequenceThreshold measured from that observation; a dead reserved tail advances to the ceiling recorded at observation (the whole tail — the magic -32 is gone from the evidence path, and a later dead batch needs its own observation cycle). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 18, 2026
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
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.
Closes the remaining silent-event-skip mechanisms from discussion #4953 (follow-up to #4964/#4972): the async daemon's high water detection could advance across outstanding sequence numbers — reserved by transactions still in flight — permanently skipping those events once they committed. Four mechanisms, each reproduced deterministically against 9.16.0 before fixing:
GapDetectorper-statement snapshot skew (Normal path, silent, threshold-free). The detector batched three statements in one command; under READ COMMITTED each takes its own snapshot, so commits landing mid-command defeated both gap checks (a null leading-gap probe bypasses the Async daemon can silently skip a committed event when a partial page's ceiling advances over a visibility gap (from #4953) #4964 hold entirely) and themax(seq_id)fallback crossed in-flight sequences. This is the mechanism that ignoresStaleSequenceThresholdcompletely and the best explanation for skips persisting with a 10-minute threshold.CheckNowAsyncloopedDetectInSafeZone(which skips by design) to the reservedlast_value— any rebuild or forced catch-up during an import mowed through in-flight gaps.-32fallback teleported into pure reservations, gated onmt_event_progression.last_updated— ancient on an idle store, so the gate passed instantly on the first crush of traffic (idle store → sudden load with slow first commits = thousands of events skipped).The fix: prove the gap dead instead of guessing with wall clocks
GapDetectorandHighWaterStatisticsDetectorare now exactly one SQL statement each — the leading-gap probe, interior-gap query, and max fallback are mutually consistent by construction. Closes mechanism 1 outright.GapLivenessProbe, on by default viaProjectionOptions.UseTransactionEvidenceForGapSkipping): before any safe-zone skip, detection checks whether a transaction that could still fill the gap is alive — grantedRowExclusiveLocks on themt_eventslineage held by transactions from before the gap was first observed (pg_locks, visible to every role), open transactions from before the observation (pg_stat_activity), and in-progress write xids below the snapshot xmax recorded at observation (pg_snapshot_xip, pure MVCC data, no privileges needed). Evidence ⇒ hold: a slow append is waited out. All three signals are time-fenced to the first observation of the specific gap, so they drain naturally — steady new load never wedges skipping.last_updated.-32— a dead tail is skipped fully and promptly instead of pinning the daemon Stale forever when the tail was shorter than 32. Every skip logs its exact range at Warning.CheckNowAsync(JasperFx.Events 2.29.1, [Documentation] PostgreSQL Options #530) targets the newIHighWaterDetector.FetchCommittedHighWaterCeilingAsync(overridden here withmax(seq_id)), time-bounded — rebuild/catch-up now waits for in-flight appends instead of skipping them.AdvanceHighWaterMarkToLatestlikewise marks committed height only.trySkipStaleTenantGapAsyncapplies the same evidence rule underUseTenantPartitionedEvents.UseTransactionEvidenceForGapSkipping = falserestores the pre-Async daemon can advance projection progress past committed events during concurrent appends #4953 behavior verbatim;SkipStaleGapsDespiteLiveTransactionsAfter(default null = never knowingly skip a live appender) trades safety for guaranteed progress against leaked idle-in-transaction sessions — PostgreSQL'sidle_in_transaction_session_timeoutis the recommended backstop.Verification
Bug_4953_*regressions (7 red before the fixes), including the reporter'spg_sleep-trigger technique end-to-end: a rebuild during an in-flight append waits and projects everything; the running daemon holds for a slow append instead of stale-skipping it.pg_locks/pg_stat_activity/pg_current_snapshotvisibility and cost verified empirically (PG 17): all monitoring-grade proc-array scans, probed only while a stale gap exists; cross-role redaction degrades the probe conservatively.DaemonTestssuite green (230/230, net10.0) against published JasperFx 2.29.1; the two legacyHighWaterDetectorTeststheories are updated to the new semantics (hold on first sighting; dead tails advance to the observed ceiling).🤖 Generated with Claude Code