amadan

carlos / platform Public

status: WriteBox last-writer-wins — fix create-only Put breaking the 5-min tick

Boxes republish their own snapshot to the same status/boxes/<label>.json
key every 5 minutes, but WriteBox was doing a bare create-only Put
(ifMatch ""), which fails with ErrCASConflict on every write after the
first. Read the current etag (absent = "", i.e. create), Put with it,
and on a lost race re-read once and retry with the fresh etag. A
second conflict is returned rather than retried again: this box is
the only writer that should ever target its own label, so two
conflicts in a row means a real label collision, which should fail
loudly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 7cd8b7c80d953676c29a1f0fff8a6b7af2d4bd98 parent 08b7268
2 files changed, +83 −11
  • internal/status/snapshot.go +49 −11
  • internal/status/status_test.go +34 −0
diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go
index ec7d5d65e..e048cef86 100644
--- a/internal/status/snapshot.go
+++ b/internal/status/snapshot.go
@@ -10,6 +10,8 @@ import (
"context"
"crypto/sha256"
"encoding/json"
+ "errors"
+ "os"
"time"
"github.com/carlosframework/platform/internal/registry"
@@ -208,20 +210,56 @@ func CollectBox(routes []registry.Route, p Probes, now time.Time) Box {
}
}
-// WriteBox marshals a Box indented (human-diffable in the bucket) and Puts it
-// at its label-derived key with ifMatch "", per this task's contract. Note
-// for whoever wires the recurring publish loop (Task 3+): release.Store's
-// ifMatch "" is create-only (If-None-Match: *) per store.go, so a second
-// WriteBox for the same label — expected every 5 minutes per the plan — will
-// return ErrCASConflict once the object exists. Overwriting on a schedule
-// needs either a Get-then-Put-with-etag retry loop or a Delete-then-Put
-// around this call; WriteBox itself stays a thin, single-attempt Put so its
-// behavior is exactly what Task 2's contract specifies.
+// WriteBox marshals a Box indented (human-diffable in the bucket) and writes
+// it to its label-derived key, last-writer-wins. Each box republishes its own
+// snapshot every 5 minutes to the SAME key, so a bare create-only Put (ifMatch
+// "") would fail every tick after the first with ErrCASConflict — that's the
+// bug this retry loop exists to avoid. The read-etag-then-Put dance is the
+// only way to express "replace whatever is there, or create it" on top of
+// release.Store's CAS-only Put: Get the current etag (a not-found just means
+// "absent", so ifMatch stays "" for a create), Put with that etag, and if the
+// Put itself loses the race (something else wrote in between), re-read once
+// and retry with the fresh etag. A second conflict is NOT retried again and
+// is returned to the caller: this box is the only writer that should ever be
+// targeting its own label's key, so two conflicts in a row means something
+// else collided on this label (clock-skewed duplicate machine id, a bug
+// elsewhere) — that deserves a loud failure, not a silent third attempt.
func WriteBox(ctx context.Context, s release.Store, b Box) error {
body, err := json.MarshalIndent(b, "", " ")
if err != nil {
return err
}
- _, err = s.Put(ctx, "status/boxes/"+b.Label+".json", body, "")
- return err
+ key := "status/boxes/" + b.Label + ".json"
+
+ etag, err := currentEtag(ctx, s, key)
+ if err != nil {
+ return err
+ }
+ if _, err := s.Put(ctx, key, body, etag); err != nil {
+ if !errors.Is(err, release.ErrCASConflict) {
+ return err
+ }
+ etag, err = currentEtag(ctx, s, key)
+ if err != nil {
+ return err
+ }
+ if _, err := s.Put(ctx, key, body, etag); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// currentEtag reads a key's etag for a replace-only Put, folding "doesn't
+// exist yet" into "" (create-only) — the same absent-means-empty-ifMatch
+// convention release.Store's own CAS contract uses.
+func currentEtag(ctx context.Context, s release.Store, key string) (string, error) {
+ _, etag, err := s.Get(ctx, key)
+ if err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ return "", nil
+ }
+ return "", err
+ }
+ return etag, nil
}
diff --git a/internal/status/status_test.go b/internal/status/status_test.go
index 795a7a292..75ca2163d 100644
--- a/internal/status/status_test.go
+++ b/internal/status/status_test.go
@@ -83,3 +83,37 @@ func TestWriteBoxPutsUnderLabeledKey(t *testing.T) {
t.Errorf("round-tripped label %q, want %q", round.Label, b.Label)
}
}
+
+// TestWriteBoxOverwritesOnSecondTick proves WriteBox is last-writer-wins for
+// a given label: boxes publish every 5 minutes to the same key, so a
+// create-only Put (which fails once the object exists) would break on the
+// second tick. Both calls must succeed, and the second call's content must
+// be what a reader sees afterward.
+func TestWriteBoxOverwritesOnSecondTick(t *testing.T) {
+ dir := t.TempDir()
+ store := release.NewDirStore(dir)
+ ctx := context.Background()
+
+ first := status.CollectBox(nil, status.Probes{MachineID: "m"}, time.Unix(1000, 0))
+ if err := status.WriteBox(ctx, store, first); err != nil {
+ t.Fatalf("first WriteBox: %v", err)
+ }
+
+ second := status.CollectBox(nil, status.Probes{MachineID: "m"}, time.Unix(2000, 0))
+ if err := status.WriteBox(ctx, store, second); err != nil {
+ t.Fatalf("second WriteBox (must overwrite, not conflict): %v", err)
+ }
+
+ key := "status/boxes/" + second.Label + ".json"
+ got, _, err := store.Get(ctx, key)
+ if err != nil {
+ t.Fatalf("Get %s: %v", key, err)
+ }
+ var round status.Box
+ if err := json.Unmarshal(got, &round); err != nil {
+ t.Fatalf("unmarshal written box: %v", err)
+ }
+ if !round.GeneratedAt.Equal(second.GeneratedAt) {
+ t.Errorf("stored box has GeneratedAt %v, want the second write's %v (second write must win)", round.GeneratedAt, second.GeneratedAt)
+ }
+}