Repository navigation
HighWaterAgent: survive a failed watchdog restart (#1027), and restart on a stalled mark - #1028
Merged
Merged
Conversation
RestartAsync stopped the watchdog timer and cleared IsRunning before it re-detected the mark. When that Detect threw because the database was still down, the agent was left with no loop, no timer and IsRunning == false, which also switched IsStale off. Nothing ever tried again, and the high water mark stayed frozen until the process was restarted. The wedged loop it gave up on would have recovered by itself once the database came back; clearing IsRunning is what made it exit. RestartAsync now detects first, bounded by HighWaterStalenessThreshold, and only retires the running loop once it has a mark to hand over. A failed restart leaves the agent as it found it, so the next health check retries. The watchdog's restart also backs off if StopAsync ran while it was detecting, so a deliberate stop is never undone. A loop retired by a restart can still be parked inside a Detect that answers much later. It now checks its generation after every such await and never writes _current or the tracker once replaced: before, it could publish a reading taken before the restart and move the tracker's mark backwards. The loop's exit log no longer claims a cancellation when it was in fact retired by a restart or stopped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018E7QLXkj5qDNMPTT8VcaMT
The #539 heartbeat proves the poll loop is cycling, not that the mark is getting anywhere, and it only exists while the loop is the one doing the detecting. The watchdog now also asks the database directly: once the mark has been still for the new DaemonSettings.HighWaterProgressStallThreshold (default 2 minutes, null or non-positive disables it), it reads the committed ceiling through IHighWaterDetector.FetchCommittedHighWaterCeilingAsync at most once per window, outside the poll loop, and restarts the agent if events are committed above the mark. The ceiling is the committed height, not HighestSequence, so a rolled-back tail of reserved sequence numbers is not a stall. A probe that fails is not a stall either. The probe runs under the remediation guard and is bounded by HighWaterStalenessThreshold so a hung probe cannot keep the watchdog from noticing a wedged loop. A restart re-reads the mark and never advances it, so this can never skip an event; a gap the detector holds on purpose stays held, and the warning logged on each restart says so. The setting is added to IReadOnlyDaemonSettings as a default interface member so an implementation compiled before it existed keeps loading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018E7QLXkj5qDNMPTT8VcaMT
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.
Fixes #1027. Two commits, so the fix can be reviewed (or taken) without the new health check.
1. The #1027 fix (
a97e5f2)What actually happened
RestartAsynctore the agent down before it knew it could bring it back up:When
Detect()threw, the agent was left with:checkStatenever ran again;IsRunning == false, which makesIsStalereturn false. Even a running timer would have seen nothing wrong. It also makesJasperFxAsyncDaemon.IsRunningfalse;IsRunning.That last point is the irony of the incident. The wedged loop was stuck in a Detect on a dead connection. That Detect errored at 01:48:52 (it was not cancelled). Left alone, that loop would have kept cycling and recovered on its own once the database came back at 01:50:35. Clearing
IsRunningis what killed it. The 01:48:53 log line, "has detected a cancellation", was a third, unrelated message, printed for every exit reason.The change
StartAsyncis split intoDetect()+launchAsync(initial).RestartAsyncrunsDetect()first, bounded byHighWaterStalenessThresholdso a hung connection cannot pin the watchdog. Only once it has a mark does it calllaunchAsync, which bumps the loop generation and retires the old loop. If the Detect fails,RestartAsyncthrows and leaves the agent exactly as it found it: same loop,IsRunningunchanged, timer running. The staleness check therefore keeps firing, and the watchdog retries once perHighWaterStalenessThresholduntil the database answers. Meanwhile the old loop can still recover by itself. This is the reporter's_restartPendingidea taken one step further: there is no pending state to track, because nothing gets torn down until the replacement exists.IsRunningnow stays true for the whole restart, so if it reads false after the Detect,StopAsyncran in the meantime. The watchdog's restart then backs off instead of bringing the loop and timer back. The publicRestartAsync()keeps its old "start it if it isn't running" behaviour.markProgressAsyncdoesn't guard against that, and neither doesShardStateTracker.MarkHighWaterAsync. The loop now checksisCurrentLoop(generation)after every Detect/DetectInSafeZone and insidemarkProgressAsync.2. A health check that watches the mark itself (
0c95eda)The #539 heartbeat proves the loop is cycling, not that the mark is getting anywhere. It also only means something while the loop is the thing doing the detecting. The new check watches the mark itself:
DaemonSettings.HighWaterProgressStallThreshold: default 2 minutes;nullor non-positive disables it. It is also onIReadOnlyDaemonSettingsas a default interface member, so an implementer compiled before it existed keeps loading.IHighWaterDetector.FetchCommittedHighWaterCeilingAsyncdirectly, outside the poll loop, at most once per window. That is Marten'sselect max(seq_id). If events are committed above the mark, it logs a warning with the mark and ceiling and restarts the agent.HighestSequence, so a rolled-back tail of reserved sequence numbers is not a stall. A probe that throws is not a stall either, because the outage path above already handles that. The probe is bounded byHighWaterStalenessThresholdbecause it holds the remediation guard.StopAsync). An externally paused agent, such asPauseHighWaterAgentAsyncunderExternallyManaged, is never second-guessed.Testing
JasperFx (net10.0; no .NET 9 runtime in my environment):
HighWaterAgent_restart_failure_Tests:main);StopAsynclands during a watchdog restart.HighWaterAgent_progress_stall_Tests: a stuck mark is restarted without advancing; a quiet store is left alone; a failing probe is not a stall;nulldisables the probe entirely.EventTests.Daemon.HighWater: 51 out of 51, three times in a row. FullEventTests: 1588 out of 1588.Marten, against a local pack of this branch (
2.90.3-local.1027.2, in place of 2.90.1) on PostgreSQL 16:DaemonTestssuite: 377 out of 377, retries off, net10.0.daemon.IsRunning == false🤖 Generated with Claude Code
https://claude.ai/code/session_018E7QLXkj5qDNMPTT8VcaMT