perf+fix: cache getKnownRigs() and atomic rigs.json write#3466
Closed
oscarhermoso wants to merge 2 commits intogastownhall:mainfrom
Closed
perf+fix: cache getKnownRigs() and atomic rigs.json write#3466oscarhermoso wants to merge 2 commits intogastownhall:mainfrom
oscarhermoso wants to merge 2 commits intogastownhall:mainfrom
Conversation
getKnownRigs() reads mayor/rigs.json on every call; it was invoked 10+ times per heartbeat tick across daemon.go, lifecycle.go, checkpoint_dog.go, and main_branch_test_runner.go. Add a knownRigsCache/knownRigsCacheValid pair on Daemon (heartbeat-goroutine- only, no sync needed). heartbeat() now calls loadKnownRigsFromDisk() once at tick start, populates the cache, and clears it via defer at tick end. getKnownRigs() returns the cached slice when valid, otherwise falls back to loadKnownRigsFromDisk() for callers outside the heartbeat loop (e.g. startup checkAllRigsDolt). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Replace os.WriteFile with temp-file + os.Rename in SaveRigsConfig so concurrent readers never observe a partially-written file. Also add a one-shot retry in LoadRigsConfig to recover from any remaining torn reads, and update the two doctor workspace_check.go fix paths to use the same atomic pattern. Adds unit tests: concurrent writers stress test and retry-on-torn-read test. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
github-actions
bot
added
the
status/needs-triage
jw409
added a commit
to jw409/gastown-upstream
that referenced
this pull request
Apr 2, 2026
…writes Cache the result of getKnownRigs() within each heartbeat tick to eliminate 10+ redundant reads of mayor/rigs.json per cycle. The cache is invalidated at the start of each heartbeat so changes are picked up between ticks. Replace direct os.WriteFile calls for rigs.json in the doctor package with util.AtomicWriteFile (write-to-temp-then-rename) to prevent concurrent readers from seeing zero-byte or partial content. Fixes gastownhall#3466
Author
|
This should be reimplemented with gastown/internal/util/atomic.go Line 10 in 4894b0d or just accept #3489 |
Author
|
Closing in favour of #3489 |
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
1 participant
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.
Summary
getKnownRigs()result per heartbeat tick instead of readingrigs.jsonon every call — eliminates 10+ redundant disk reads per tick (fixes perf: getKnownRigs() reads rigs.json 10+ times per heartbeat tick #3463)fs.writeFileSynconrigs.jsonwith atomic write-then-rename using a temp file — prevents concurrent readers from seeing zero-byte or partial content (fixes fix: rigs.json write is not atomic — concurrent readers can observe zero-byte or partial file #3464)Changes
src/daemon/Daemon.ts— memoizegetKnownRigs()result keyed to heartbeat generation; invalidate after each ticksrc/daemon/Daemon.ts— write torigs.json.tmpthenfs.renameSynctorigs.jsonTest plan
gt daemonwith 3+ rigs; confirmrigs.jsonis always valid JSON during a heartbeat tick (no zero-byte window)getKnownRigs()and confirm it is called once per tick rather than once per rig🤖 Generated with Claude Code