Repository navigation
Conversation
Areson
added this pull request to stack #192
October 21, 2026 21:52
Areson
force-pushed
the
Areson/parallel-monitor-stop
branch
from
October 21, 2026 22:43
d6a23e0 to
8fd762f
Compare
Areson
force-pushed
the
Areson/parallel-monitor-stop
branch
from
October 22, 2026 14:54
8fd762f to
43ef39e
Compare
Areson
force-pushed
the
Areson/parallel-monitor-stop
branch
from
October 22, 2026 15:51
43ef39e to
e1eb533
Compare
Areson
force-pushed
the
Areson/parallel-monitor-stop
branch
2 times, most recently
from
October 22, 2026 17:28
ee4db51 to
f0e1912
Compare
Load unloaded removed and changed monitors one at a time while holding the loader mutex, and Monitor.Stop has no deadline. Each stop waits for the level collector's next 1s tick (about 0.5s on average), so removing many monitors at once held the mutex for n x 0.5s, and a monitor that never stops would hold it forever. Every reload, start, and stop waits on that mutex. Loader: - Take unloaded and stopped monitors out of the loader state under the mutex, then stop them in background goroutines, in parallel. - Load, Stop, and Unload wait for those stops without the mutex, but no longer than config.monitor-loader.stop-timeout (default 3s). - A monitor that misses the timeout is reported with event monitor-stop-timeout and monitor status error:stop, and keeps stopping in the background. Its status is cleared when it stops. - Track in-flight stops by monitor ID. A monitor with the same ID (a changed or re-added monitor, or a restart) is marked start-pending and starts only when the old one has stopped, so two monitors with the same ID never run at the same time. - Don't call Monitor.Stop on a monitor that never started (it panicked). - Monitor returns nil for an unknown ID instead of panicking. API: POST /monitors/start and /monitors/stop go through the loader instead of calling Monitor.Start and Monitor.Stop directly, so they can't restart a monitor the loader is stopping or unloading. A stop that misses the timeout returns 202. Monitor: fix start/stop races found by the new concurrency test: - Stop right after Start, before runLoop made runChan, panicked closing a nil channel. - Stop didn't wait for runLoop to return, and runLoop read the runLoopChan field, so Stop then Start could leave two runLoops running one monitor. The next Stop then blocked forever in wg.Wait. - Stop during startup could leave subsystems started on a closed DB. Start and Stop are now serialized, each runLoop gets its own stop and done channels, Stop waits for runLoop, runLoop stops its last subsystems before returning, and retry sleeps return early on stop. The plan-loading retry loop no longer returns success on error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A changed monitor whose old instance is still stopping starts when the old instance stops, right after the loader removes the old monitor's status and counters. Test that the new monitor's retry sink counter is reported and incremented once it starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Areson
force-pushed
the
Areson/parallel-monitor-stop
branch
from
October 22, 2026 19:32
f0e1912 to
de3d8d7
Compare
This branch has not been deployed
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.
Why
Loader.Loadstops removed and changed monitors one at a time while it holds the loader mutex.Monitor.Stophas no deadline. Each stop waits for the level collector's next 1 s tick, 0.5 s on average. So removing many monitors at once holds the mutex for n × 0.5 s, and a monitor that never stops holds it forever. Every reload, start, and stop waits on that mutex.What
config.monitor-loader.stop-timeout(default3s).monitor-stop-timeoutand statuserror:stop.POST /monitors/startand/monitors/stopthrough the loader. A stop that misses the timeout returns 202.Monitorthat the new concurrency test found.How
error:stopstatus clears when it stops.Loader.StoporUnloadon a monitor that never started no longer panics.Loader.Monitorreturns nil for an unknown ID instead of panicking.Monitor.StartandStopare now serialized. EachrunLoopgets its own stop and done channels, andStopwaits forrunLoop. This fixes three races:Stopright afterStartpanicked when it closed a nil channel.StopthenStartcould leave tworunLoops on one monitor. The nextStopthen hung forever inwg.Wait.Stopduring startup could start subsystems on a closed DB.Risk
Medium to high. This changes the monitor lifecycle and the loader, which every Blip deployment uses. The API behavior of
/monitors/startand/monitors/stopalso changes: a start can now be pending, and a slow stop returns 202.Testing
A local benchmark ran 400 monitors in one process against 4 MySQL 8.0 servers.
Bigger picture
Part of a stack of 5 fixes found by the same benchmark. Each stop still waits for the level collector's tick; #191 removes that wait.
🤖 Generated with Claude Code