diff --git a/changelog.d/fixed-write-counter-read-errors.md b/changelog.d/fixed-write-counter-read-errors.md new file mode 100644 index 0000000000..c73b6d127c --- /dev/null +++ b/changelog.d/fixed-write-counter-read-errors.md @@ -0,0 +1 @@ +- Preserve durable write counter totals and retry pending counts after transient file read errors ([#14531](https://github.com/hivecommons/hive/issues/14531)). diff --git a/src/crux/proxy/write_counters_store.go b/src/crux/proxy/write_counters_store.go index c0d4fad9d6..ba9ee43337 100644 --- a/src/crux/proxy/write_counters_store.go +++ b/src/crux/proxy/write_counters_store.go @@ -181,6 +181,8 @@ func (d DurableWriteCounts) empty() bool { return len(d.Totals) == 0 && len(d.Weeks) == 0 } +var errInvalidWriteCounts = errors.New("invalid write counters JSON") + // LoadDurableWriteCounts reads the file at path. A missing file is an empty // result, not an error. func LoadDurableWriteCounts(path string) (DurableWriteCounts, error) { @@ -193,7 +195,7 @@ func LoadDurableWriteCounts(path string) (DurableWriteCounts, error) { return d, err } if err := json.Unmarshal(data, &d); err != nil { - return newDurableWriteCounts(), fmt.Errorf("parse %s: %w", path, err) + return newDurableWriteCounts(), fmt.Errorf("parse %s: %w: %w", path, errInvalidWriteCounts, err) } return d, nil } @@ -220,7 +222,10 @@ func (c *writeCounters) flush(path string, now time.Time) error { func mergeIntoFile(path string, pending DurableWriteCounts, now time.Time) error { cur, err := LoadDurableWriteCounts(path) if err != nil { - // A corrupt file must not stall persistence forever; start over. + // Only corrupt JSON can safely be replaced; preserve unreadable files. + if !errors.Is(err, errInvalidWriteCounts) { + return err + } cur = newDurableWriteCounts() } cur.merge(pending) diff --git a/src/crux/proxy/write_counters_store_test.go b/src/crux/proxy/write_counters_store_test.go index 5374325d1c..bec4831883 100644 --- a/src/crux/proxy/write_counters_store_test.go +++ b/src/crux/proxy/write_counters_store_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "syscall" "testing" "time" ) @@ -88,6 +89,59 @@ func TestWriteCountersFlushFailureKeepsPending(t *testing.T) { } } +func TestWriteCountersReadFailureKeepsFileAndPending(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "counters.json") + now := time.Date(2026, 10, 15, 12, 0, 0, 0, time.UTC) + var c writeCounters + c.now = func() time.Time { return now } + c.record("ADVISORY", "") + if err := c.flush(path, now); err != nil { + t.Fatal(err) + } + original, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := os.Rename(path, path+".saved"); err != nil { + t.Fatal(err) + } + // A symlink loop produces a deterministic read error even when CI runs as root. + if err := os.Symlink(filepath.Base(path), path); err != nil { + t.Fatal(err) + } + c.record("ADVISORY", DenyReasonMode) + if err := c.flush(path, now); !errors.Is(err, syscall.ELOOP) { + t.Fatalf("flush error = %v, want ELOOP", err) + } + if info, err := os.Lstat(path); err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("unreadable path was replaced: %v, %v", info, err) + } + saved, err := os.ReadFile(path + ".saved") + if err != nil || string(saved) != string(original) { + t.Fatalf("durable data changed: %v", err) + } + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + if err := os.Rename(path+".saved", path); err != nil { + t.Fatal(err) + } + if err := c.flush(path, now.Add(time.Minute)); err != nil { + t.Fatal(err) + } + got, err := LoadDurableWriteCounts(path) + if err != nil { + t.Fatal(err) + } + if wc := got.Totals["ADVISORY"]; wc.Attempted != 2 || wc.Blocked != 1 { + t.Errorf("retry totals = %+v", wc) + } + if !got.RecordedSince.Equal(time.Date(2026, 10, 12, 0, 0, 0, 0, time.UTC)) { + t.Errorf("recorded_since changed: %s", got.RecordedSince) + } +} + func TestLoadDurableWriteCountsMissingAndCorrupt(t *testing.T) { dir := t.TempDir() if d, err := LoadDurableWriteCounts(filepath.Join(dir, "none.json")); err != nil || !d.empty() { @@ -95,8 +149,8 @@ func TestLoadDurableWriteCountsMissingAndCorrupt(t *testing.T) { } bad := filepath.Join(dir, "bad.json") _ = os.WriteFile(bad, []byte("{"), 0o644) - if _, err := LoadDurableWriteCounts(bad); err == nil { - t.Error("expected parse error") + if _, err := LoadDurableWriteCounts(bad); !errors.Is(err, errInvalidWriteCounts) { + t.Errorf("expected parse error, got %v", err) } var c writeCounters c.record("ADVISORY", "")