Skip to content

High-water health check no longer reads a heartbeat column nothing writes (#5174) - #5181

Merged
jeremydmiller merged 1 commit into
masterfrom
fix/5174-highwater-heartbeat
Aug 4, 2026
Merged

High-water health check no longer reads a heartbeat column nothing writes (#5174)#5181
jeremydmiller merged 1 commit into
masterfrom
fix/5174-highwater-heartbeat

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #5174.

The bug

HighWaterHealthCheckExtensions filtered AllProjectionProgress down to the HighWaterMark rows and used row.LastHeartbeat as its primary staleness signal. That column is never written for those rows:

  • ExtendedProgressionWriter.OnNext returns early for ShardState.HighWaterMark / AllProjections — pinned upstream by skips_high_water_mark_and_all_projections_states.
  • HighWaterDetector.MarkHighWaterMarkInDatabaseAsync / MarkHighWaterForTenantAsync and BulkEventAppender's high-water upserts write name and last_seq_id only.
  • WriteExtendedProgressionAsync is the only statement that sets heartbeat, and it only ever receives shard rows.

So the branch was unreachable in production and the check silently degraded to the gap heuristic — which, under UseTenantPartitionedEvents, is explicitly skipped for per-tenant rows. The per-tenant path asserted nothing. The tests didn't catch it because they seeded heartbeat with raw SQL, passing against a state the daemon never produces.

(JasperFx 2.39.1 already corrected the JasperFxAsyncDaemon doc comment that made this assumption look sound — #622.)

The fix

Not option 1 from the issue (start writing high-water heartbeats): that adds a periodic write stream at exactly the scale #5167 is about, and #622 has just turned the existing periodic beat off by default for cost reasons. Instead, the issue's option 2 — with the better signal it points at:

Per-tenant rows use last_updated. mt_mark_event_progression always sets last_updated = transaction_timestamp(), and TenantedHighWaterCoordinator calls it on every vectorized poll, mark advance or not. Its age is therefore a genuine "the poll loop is cycling" signal — the thing the phantom heartbeat was supposed to provide — at zero extra write cost and with no dependency on EnableExtendedProgressionTracking. This is a real capability the check did not have before.

The store-global row keeps the gap heuristic. There last_updated only moves when the mark advances (persistDetectedMarkAsync returns early when !HasChanged), so it is not a liveness signal and pretending otherwise would false-positive on a quiet store. The docs now state plainly that a store-global loop wedged while caught up is not detectable from the progression table, and point at IProjectionDaemon.HighWaterAgent.IsStale / LastPolledAt for the in-process case.

The heartbeatOnly flag is renamed skipGapHeuristic, which is what it actually meant.

Bonus

The row read no longer goes through AllProjectionProgress. That pulled every projection × tenant row on every probe to keep two of them, and still couldn't see last_updatedShardState has no field for it. It's now a targeted two-row query.

Tests

DaemonTests.ManualOnly/HealthChecks/HighWaterHealthCheckTests.cs reworked: the heartbeat seeding helper is gone, seedProgressionRowAsync now takes a last_updated. New/changed cases:

  • fresh per-tenant poll + mark behind → Healthy
  • stale per-tenant poll + mark caught up → Unhealthy (the case the gap heuristic is blind to)
  • the_extended_progression_heartbeat_column_is_never_consulted — the pin: with a fresh heartbeat written directly, a stuck store-global mark must still trip the gap heuristic
  • a per-tenant row is never gap-assessed
  • ExternallyManaged opt-in now asserts via the per-tenant poll signal

23/23 pass on net10.0. Docs updated (docs/events/projections/healthchecks.md), markdownlint + cspell clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_017CTtw2kVRSZKp1p5RTxgAy

…ites (#5174)

The check treated `ProjectionProgressRow.LastHeartbeat` as its primary staleness signal for
high-water rows. That column is never written for them: `ExtendedProgressionWriter.OnNext`
returns early for `ShardState.HighWaterMark` (pinned by
`skips_high_water_mark_and_all_projections_states`), and nothing else in Marten sets it. So
the branch was unreachable in any real deployment and the check silently degraded to the gap
heuristic — while its own tests passed against a state the daemon never produces, because
they seeded `heartbeat` with raw SQL.

Rather than add a periodic write to a path already under cost pressure (#5167), use the
signal that is already there: `mt_mark_event_progression` always sets `last_updated`, and the
vectorized per-tenant poll calls it on EVERY cycle whether or not the mark advances. Its age
is therefore a real "the loop is cycling" signal for `HighWaterMark:{tenant}` rows, at zero
extra write cost and with no dependency on ExtendedProgression.

On the store-global row `last_updated` only moves on an advance
(`persistDetectedMarkAsync` returns early when nothing changed), so it says nothing about
liveness and the gap heuristic remains the only honest signal there. Documented, along with
`IProjectionDaemon.HighWaterAgent.IsStale` / `LastPolledAt` for the in-process case the
database cannot answer.

The row read also stopped going through `AllProjectionProgress`, which pulled every
projection × tenant row on every probe to keep two of them — and could not see `last_updated`
anyway, since `ShardState` has no field for it.

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

heartbeat is never written for HighWaterMark rows, so the high-water health check's primary signal is unreachable

1 participant