Skip to content

#5022: kill the shutdown-telemetry race in extended_progression_batch_write - #5023

Merged
jeremydmiller merged 1 commit into
masterfrom
fix/5022-drain-shutdown-telemetry-race
Jul 22, 2026
Merged

#5022: kill the shutdown-telemetry race in extended_progression_batch_write#5023
jeremydmiller merged 1 commit into
masterfrom
fix/5022-drain-shutdown-telemetry-race

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Fixes #5022.

Stacked on #5008. This branch is #5008's commit + one test-only fix on top, because the flaky test only exists on #5008's branch. Review/merge #5008 first — this PR's diff collapses to just the second commit (#5022: kill the shutdown-telemetry race …) once #5008 lands. Alternatively, cherry-pick that commit straight into #5008.

Problem

DaemonTests.extended_progression_batch_write (#5008 / #553) fails intermittently in CI:

Shouldly.ShouldAssertException : second.status
    should be "Paused" but was "Stopped"

Its setup helper started a live daemon, waited for it to catch up, then StopAllAsync()'d it before asserting on the mt_event_progression rows. Daemon shutdown itself emits a "Stopped" extended-progression heartbeat asynchronously and does not drain it before the daemon is considered stopped — so it races back in after the explicit WriteExtendedProgressionAsync batch write and clobbers the rows under assertion. The helper's comment ("Stop the daemon so nothing else races telemetry writes") had it backwards: stopping the daemon was the source of the racing write.

The same shutdown path also produces the ObjectDisposedException on a disposed SemaphoreSlim noted in the issue (Symptom 2).

Fix (test-only — issue direction 1)

Nothing in these tests needs a running daemon. Seed the two committed progression rows directly via mt_mark_event_progression and exercise WriteExtendedProgressionAsync with no daemon running, so the only writer left against those rows is the call under test. This removes the race entirely and makes the tests deterministic.

#5008's production code is unchanged — only its new test is touched.

Verification

  • dotnet test -f net9.0 --filter FullyQualifiedName~extended_progression_batch_write3/3 passing across 5 consecutive runs (previously flaky).

Out of scope

The product-side fix (Symptom 2 — draining in-flight shutdown heartbeats before disposing the shutdown semaphore) lives in JasperFx.Events.Daemon.GroupedProjectionExecution, cross-repo in jasperfx, and is tracked separately.

🤖 Generated with Claude Code

jeremydmiller added a commit that referenced this pull request Jul 22, 2026
…tdown drain (#5024) (#5025)

Bumps JasperFx.Events 2.33.1 -> 2.34.0 to consume #555 (the two remaining
tenant-scoped explorer read paths) and #557 (the extended-progression
shutdown-drain fix), then implements/proves both from Marten's side.

#5021 — the second pair of explorer reads #503 left untenanted:
- Override IEventStore.QueryByTagsAsync(tags, tenantId, ct) in
  DocumentStore.EventStoreExplorer.cs with the same two-model scoping PR #5020
  established for the stream reads: a conjoined single-database store bounds the
  outer mt_events scan with an e.tenant_id predicate on the AllowAnyTenant explorer
  session (the tag sub-selects match by the globally-unique seq_id, so the outer
  predicate discards cross-tenant seq_ids); a database-per-tenant / sharded store
  opens the session against the tenant's own database. Null delegates to the
  tenant-less overload, byte-identical to today.
- Honour EventQuery.TenantId in QueryEventStore.QueryEventsAsync: a set tenant scopes
  the query with TenantIsOneOf (overriding the session's own filter); a null tenant is
  left untouched so the pre-existing per-tenant paging contract is unchanged.
- Explorer tests: same tag / same event under two tenants on a conjoined store reads
  each tenant's slice in isolation.

#5024 Phase 3 — consume + prove the #557 shutdown fix:
- Bug_5024_extended_progression_survives_shutdown drives the real live-then-stopped
  daemon path (#5023 deliberately stopped doing this to de-flake #5008): start a live
  daemon, catch it up, StopAllAsync(), then issue an explicit WriteExtendedProgression
  and assert the row is NOT clobbered by a late "Stopped" heartbeat (Symptom 1) and no
  ObjectDisposedException reached the log on shutdown (Symptom 2). Deterministic
  because the upstream drain is now awaited; green across repeated runs.


Claude-Session: https://claude.ai/code/session_01JQ9NHbg31EWJmrQK9EN6i8

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_write

The new extended_progression_batch_write tests (#553 / #5008) started a
live daemon, waited for it to catch up, then StopAllAsync()'d it before asserting
on the mt_event_progression rows. Daemon shutdown itself emits a "Stopped"
extended-progression heartbeat asynchronously and does not drain it before the
daemon is considered stopped, so it races back in AFTER the explicit
WriteExtendedProgressionAsync batch write and clobbers the rows under assertion
(the intermittent `should be "Paused" but was "Stopped"` CI failure). The helper's
own comment had it backwards: stopping the daemon was the source of the racing
write, not a guard against it.

Nothing in these tests needs a running daemon. Seed the two committed progression
rows directly via mt_mark_event_progression so the only writer left against those
rows is the WriteExtendedProgressionAsync call under test. This removes the race
(and the shutdown-path ObjectDisposedException) entirely and makes the tests
deterministic, unblocking #5008.

Test-only change (issue #5022 direction 1). The product-side drain of in-flight
shutdown heartbeats before disposing the JasperFx.Events shutdown semaphore
(Symptom 2) lives in JasperFx.Events.Daemon.GroupedProjectionExecution and is
tracked separately as the cross-repo fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jeremydmiller
jeremydmiller force-pushed the fix/5022-drain-shutdown-telemetry-race branch from 54db6da to 6ffc3e7 Compare July 22, 2026 13:05
@jeremydmiller
jeremydmiller merged commit d651232 into master Jul 22, 2026
9 checks passed
@jeremydmiller
jeremydmiller deleted the fix/5022-drain-shutdown-telemetry-race branch July 22, 2026 13:15
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.

Flaky daemon shutdown telemetry race clobbers mt_event_progression rows (blocks #5008)

1 participant