Skip to content

Commit 2fb94e8

Browse files
oscarhermosoclaude
andauthored
perf+fix(daemon): cache getKnownRigs() per tick, atomic rigs.json writes (#3684)
Fixes #3463, #3464. Cache getKnownRigs() per heartbeat tick (#3463) d.getKnownRigs() re-read and re-parsed mayor/rigs.json on every call, and ~10 call sites run per heartbeat tick. Memoize the result for the duration of a tick via a new (knownRigsCache, knownRigsCacheValid) pair on Daemon. Invalidated at the start of each heartbeat so changes between ticks are picked up; accessed only from the heartbeat-loop goroutine so no synchronization is needed. The existing callers all already use d.getKnownRigs(), so no call-site changes are required — they automatically benefit from the cache. Atomic rigs.json writes (#3464) SaveRigsConfig used os.WriteFile, which truncates the file before writing. A concurrent reader (getKnownRigs on another tick, doctor checks, LoadRigsConfig elsewhere) could observe a zero-byte or partial file and fail with "unexpected end of JSON input". - SaveRigsConfig now writes to a temp file in the same directory and renames into place; the rename is atomic on POSIX. - LoadRigsConfig retries once on read/parse failures as a belt-and-suspenders recovery if an older non-atomic writer is still in the process tree. - doctor/workspace_check.go Fix paths (RigsRegistryExistsCheck and RigsRegistryValidCheck) switch to the atomic write helper — same atomicity guarantee, same rigs.json target. Extract atomic-write helpers to internal/atomicfile internal/config could not import internal/util's AtomicWriteFile without a cycle (util → lock → tmux → config via orphan.go). Rather than duplicate the helper, move the atomic-write primitives out of util into a dedicated leaf package internal/atomicfile, which has no internal deps and so can be imported from anywhere. 13 existing callers are updated to atomicfile.WriteFile / WriteJSON / WriteJSONWithPerm / EnsureDirAndWriteJSON; util retains its non-atomic helpers. Net -569 lines. Tests - internal/atomicfile: full port of the existing util/atomic_test.go suite (concurrency, integrity, permissions, read-only-dir, large-file cases). - TestGetKnownRigs_CachedBetweenInvalidations: populates cache, deletes rigs.json, asserts d.getKnownRigs() still returns cached results, and that invalidation surfaces disk state (including after rewrites). - TestSaveRigsConfig_AtomicAgainstConcurrentReaders: 4 concurrent writers × 2000 reader iterations; fails if any raw read observes a zero-byte or parse-failing file. - TestLoadRigsConfig_RetriesOnTruncatedRead: asserts the retry path recovers a real payload after a seeded truncated state. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 66a8ff0 commit 2fb94e8

File tree

19 files changed

+542
-219
lines changed

19 files changed

+542
-219
lines changed

internal/agent/state.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"os"
88
"path/filepath"
99

10-
"github.com/steveyegge/gastown/internal/util"
10+
"github.com/steveyegge/gastown/internal/atomicfile"
1111
)
1212

1313
// StateManager handles loading and saving agent state to disk.
@@ -58,5 +58,5 @@ func (m *StateManager[T]) Save(state *T) error {
5858
return err
5959
}
6060

61-
return util.AtomicWriteJSON(m.stateFilePath, state)
61+
return atomicfile.WriteJSON(m.stateFilePath, state)
6262
}

internal/atomicfile/atomicfile.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Package atomicfile provides atomic file-write primitives (write-to-temp +
2+
// rename). Kept as a leaf package with no internal dependencies so any other
3+
// package — including low-level ones that util/ transitively depends on — can
4+
// use it without creating an import cycle.
5+
package atomicfile
6+
7+
import (
8+
"encoding/json"
9+
"os"
10+
"path/filepath"
11+
)
12+
13+
// WriteJSON writes JSON data to a file atomically with mode 0644.
14+
// It first writes to a temporary file in the same directory, then renames it
15+
// to the target path. This prevents data corruption if the process crashes
16+
// during write. The rename operation is atomic on POSIX systems.
17+
func WriteJSON(path string, v interface{}) error {
18+
data, err := json.MarshalIndent(v, "", " ")
19+
if err != nil {
20+
return err
21+
}
22+
return WriteFile(path, data, 0644)
23+
}
24+
25+
// WriteJSONWithPerm is like WriteJSON but uses the given file mode.
26+
func WriteJSONWithPerm(path string, v interface{}, perm os.FileMode) error {
27+
data, err := json.MarshalIndent(v, "", " ")
28+
if err != nil {
29+
return err
30+
}
31+
return WriteFile(path, data, perm)
32+
}
33+
34+
// EnsureDirAndWriteJSON creates parent directories (mode 0755) if needed, then
35+
// atomically writes JSON with mode 0644.
36+
func EnsureDirAndWriteJSON(path string, v interface{}) error {
37+
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
38+
return err
39+
}
40+
return WriteJSON(path, v)
41+
}
42+
43+
// EnsureDirAndWriteJSONWithPerm is like EnsureDirAndWriteJSON but uses the
44+
// given file mode for the output file.
45+
func EnsureDirAndWriteJSONWithPerm(path string, v interface{}, perm os.FileMode) error {
46+
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
47+
return err
48+
}
49+
return WriteJSONWithPerm(path, v, perm)
50+
}
51+
52+
// WriteFile writes data to a file atomically by writing to a unique temp file
53+
// in the same directory and then renaming it over the target. The rename is
54+
// atomic on POSIX systems; concurrent writers each produce self-consistent
55+
// content because each uses a distinct temp file.
56+
func WriteFile(path string, data []byte, perm os.FileMode) error {
57+
dir := filepath.Dir(path)
58+
base := filepath.Base(path)
59+
60+
// "*" in the pattern is replaced with a random suffix by os.CreateTemp,
61+
// preventing concurrent writers from colliding on the same temp file.
62+
f, err := os.CreateTemp(dir, base+".tmp.*")
63+
if err != nil {
64+
return err
65+
}
66+
tmpName := f.Name()
67+
68+
if _, err := f.Write(data); err != nil {
69+
f.Close()
70+
os.Remove(tmpName)
71+
return err
72+
}
73+
if err := f.Close(); err != nil {
74+
os.Remove(tmpName)
75+
return err
76+
}
77+
78+
// CreateTemp uses 0600 by default; apply the caller's permissions.
79+
if err := os.Chmod(tmpName, perm); err != nil {
80+
os.Remove(tmpName)
81+
return err
82+
}
83+
84+
if err := os.Rename(tmpName, path); err != nil {
85+
os.Remove(tmpName)
86+
return err
87+
}
88+
89+
return nil
90+
}

0 commit comments

Comments
 (0)