SkipStaleGapsDespiteLiveTransactionsAfter survives detector restarts (evidence-based stall onset)

[uniquelau]

Addresses #5108 (section 3).

⚠️ Please review with a healthy dose of skepticism. This comes from consumers of the async daemon, not maintainers: we reproduced the behavior, traced the detector/probe/fence code, and validated against the daemon test suites — but we don't have deep knowledge of these internals or the design history behind #4977/#5057, so take the approach with a pinch of salt. If a different mechanism fits better, we're glad to rework this, or for it to serve purely as an executable problem statement.

What

The cap's stuck-gap clock (_stuckGap.Since) is detector-scoped, so daemon churn — managed-distribution rebalancing, pause/resume cycles, restarts — resets it each time; with detector lifetimes shorter than threshold+cap, the documented "bounded override" never fires and a genuinely dead gap (rolled-back optimistic-concurrency loser) holds the high-water mark indefinitely, recoverable only via AdvanceHighWaterMarkToLatestAsync().

Fix: floor the cap comparison with a durable, evidence-based stall onset computed server-side:

transaction_timestamp() − greatest(min(timestamp of committed events above the pinned mark), time of the mark's last advance)

An event committed above the mark drew its sequence strictly after the gap's numbers — proof the gap has existed at least that long. When nothing is committed above the mark (an idle store's first live append), there is no durable floor and the in-memory clock governs, which holds.

Deliberately rejected design, pinned by a guard test: flooring with last_updated alone. It measures time-since-advance, conflating idle time with stall time — an overnight-idle store's first live append would have its in-flight gap cap-skipped instantly, committing events behind the mark (permanently unprojected). idle_store_first_live_append_is_not_cap_skipped pins against that shape.

Scope guarantees: null-cap default path byte-identical (#4977's evidence gating untouched); liveness probe consulted unchanged; the evidence query runs only when a cap is configured and the probe reports live; the UseTenantPartitionedEvents path is deliberately not floored (its progression last_updated is a per-poll heartbeat — any onset clamps to ~now; why-comment in code, follow-up conversation welcome). Rich-append caveat documented at the handler: mt_events.timestamp is client-written, so evidence age is bounded by client skew (QuickAppend writes transaction_timestamp()).

Tests (first direct coverage of the cap)

Test	master	this branch
fenceless cap skip, single detector	pass	pass
detector churn (300ms cycles, 600ms cap)	fails — never skips	pass (cycle-0 skip)
conjoined-tenancy churn + store-global routing pin	fails — never skips	pass
daemon-level resume	pass	pass
guard: idle-store first live append holds	pass	pass
guard: young live reserver holds until cap genuinely elapses	pass	pass

Runs (net10.0, dedicated database): new suite 6/6 (timing-sensitive trio stable across 3 consecutive runs); targeted store-global set 32/32; per-tenant suites 16/16; advanced_async_tracking 7/7. net9.0 TFM not run locally (no runtime installed) — deferring to CI.

The conjoined-tenancy test also pins routing: a TenancyStyle.Conjoined store reports SupportsTenantPartitioning == false and takes the store-global detection path.

🤖 Generated with Claude Code

[jeremydmiller]

@uniquelau Hey Laurence, I'm going to take this just a little bit different way. You can't trust IEvent.Timestamp enough to use it in this test.

[jeremydmiller]

Thanks for this — you asked for a steer, so here it is, along with what we found chasing it down. #5163 is the result, and it supersedes this PR.

Your diagnosis is correct. _stuckGap.Since really is detector-scoped, the cap really can be postponed indefinitely by churn shorter than threshold+cap, and the "bounded override" really is unbounded in that shape. That part isn't in question, and your churn test fails on master exactly as you describe.

Where we landed differently is on which mechanism to repair.

Every earlier change in this lineage (#4977 → #5057 → #5091) attacked the evidence — "is a transaction that could fill this gap actually alive?". Your PR is the first to concede that question and attack the clock instead. That's a legitimate reading, but it means the fix can only engage for people who have opted into SkipStaleGapsDespiteLiveTransactionsAfter, which is lossy by design. §1 of your own issue — defaults, permanent stall — is untouched by it, and against #5125's shape the evidence query is inert, because nothing is committed above the mark for it to date the gap from.

So we went back to the fence. It was already the sound mechanism; it just wasn't durable. Persisting it is cheaper than it looks — the lookup wants the newest observation whose allocated high is at or below the mark, and both values only move forward, so a single incrementally-maintained row is exactly equivalent to the in-memory history. No new table, no migration, engages on every existing database at upgrade.

Two findings from building it that are worth passing back:

First, the durable fence alone did not fix the restart case. Our end-to-end test still stalled, and the reason is that the replacement daemon's own leadership lock is a new session that postdates the fence — so it's never quiescent and holds the gap by itself. That's #5125's mechanism, and it turns out to dominate #5108 §1 too. It needed a second, structural fix: those connections exist only to hold the lock and never append, so they're ruled out by identity rather than by timing. We wouldn't have found that ordering without a test that drove real HotCold daemons, which is a point in favour of how you framed the problem.

Second, on the mt_events.timestamp arithmetic specifically — we'd have had to ask you to change that regardless. Reviewing it turned up #5136: now() at time zone 'utc' is written into a timestamptz column in three places, which double-converts and skews the stored instant by the server's UTC offset on any database whose TimeZone isn't UTC. On a UTC+2 server every event timestamp reads two hours old, which would have quietly collapsed a configured 5-minute cap into "skip as soon as the stale threshold passes". Your PR comment flagged client skew as the soft spot and you were right to — it was just worse than either of us thought. Genuinely useful thing to have surfaced.

What we'd like to keep from this PR: your detector-churn test. It's the clearest executable statement of the problem in the whole thread, and even though the mechanism changed, the property it pins — a fresh detector must not reset the clock on a gap that's already old — is still worth guarding. If you're happy for us to carry it over into #5163's suite (credited to you), say the word; equally happy for you to push it as its own PR.

We'll close this one as superseded rather than merged, but the analysis in it did real work. Thanks for taking the trouble to trace it properly and for saying plainly which parts you weren't sure about — that made it much faster to review.

[jeremydmiller]

Superseded by #5163, now merged. The reasoning is in my earlier comment; the short version is that we repaired the allocation fence (the sound mechanism) rather than the cap clock (the lossy one), so both #5108 §1 and #5125 recover at default settings without anyone opting into SkipStaleGapsDespiteLiveTransactionsAfter. #5164 has also merged since, stopping WaitForNonStaleProjectionDataAsync from treating high-water bookkeeping as a projection shard.

Carrying your detector_churn_does_not_postpone_the_cap_forever test forward now, with co-authorship on the commit.

To be straight about what that involves: the defect it pins — the cap clock being detector-scoped — is real and is still present on master, because #5163 did not touch it. So the test does not pass today, and salvaging it properly means fixing the clock rather than just filing the test. I plan to do that with a durable record of when a gap was first observed, keyed on the pinned mark. That needs no mt_events.timestamp arithmetic, so it avoids the skew in #5136 that the evidence query in your PR would have inherited.

If that lands, your test lands with it, unchanged in substance. Thanks again — the churn scenario was not on our radar and would not have been without your report.