amadan

carlos / platform Public

hostfiles: a failed re-read after a lost commit response is retried, not treated as superseded

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 4517c98c58e008557b74dcdcfcf9d7acf502197c parent f75a460
2 files changed, +51 −2
  • internal/hostfiles/hostfiles_test.go +45 −1
  • internal/hostfiles/publisher.go +6 −1
diff --git a/internal/hostfiles/hostfiles_test.go b/internal/hostfiles/hostfiles_test.go
index 757e5eda6..f83539e28 100644
--- a/internal/hostfiles/hostfiles_test.go
+++ b/internal/hostfiles/hostfiles_test.go
@@ -492,7 +492,16 @@ func TestRestoreHandlesADeepDirectoryBecomingAFile(t *testing.T) {
// A store whose manifest Put is applied and then reports a lost response, once.
type lostResponseStore struct {
release.Store
- armed bool
+ armed bool
+ getFail bool // the next Get of the manifest fails once
+}
+
+func (l *lostResponseStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if l.getFail && strings.HasSuffix(key, "manifest.json") {
+ l.getFail = false
+ return nil, "", errors.New("read timed out")
+ }
+ return l.Store.Get(ctx, key)
}
func (l *lostResponseStore) Put(ctx context.Context, key string, body []byte, etag string) (string, error) {
@@ -580,3 +589,38 @@ func TestOverlappingLiveEntriesKeepTheNewerOne(t *testing.T) {
t.Fatalf("a/b = %q, superseded %v, written %d", got, st.Superseded, st.Written)
}
}
+
+func TestAFailedReReadAfterALostResponseIsNotASupersede(t *testing.T) {
+ ctx := context.Background()
+ s := &lostResponseStore{Store: release.NewDirStore(t.TempDir())}
+ p := open(t, s)
+ stage(t, p, "x", "1")
+ s.armed = true
+ if _, err := p.Commit(ctx); err == nil {
+ t.Fatal("first commit should fail")
+ }
+ s.getFail = true
+ if _, err := p.Commit(ctx); err == nil || errors.Is(err, ErrSuperseded) {
+ t.Fatalf("second commit err = %v, want the read error, not ErrSuperseded", err)
+ }
+ if seq, err := p.Commit(ctx); err != nil || seq != 1 {
+ t.Fatalf("third commit = %d, %v", seq, err)
+ }
+}
+
+func TestThreeDeepOverlapKeepsTheNewestEntry(t *testing.T) {
+ ctx, s := context.Background(), release.NewDirStore(t.TempDir())
+ p := open(t, s)
+ stage(t, p, "a", "one")
+ stage(t, p, "a/b/c", "three")
+ stage(t, p, "a/b", "two") // the newest
+ commit(t, p)
+ dir := t.TempDir()
+ _, st, err := Restore(ctx, s, pfx, dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := read(t, dir, "a/b"); got != "two" || st.Written != 1 || !reflect.DeepEqual(st.Superseded, []string{"a", "a/b/c"}) {
+ t.Fatalf("a/b = %q, written %d, superseded %v", got, st.Written, st.Superseded)
+ }
+}
diff --git a/internal/hostfiles/publisher.go b/internal/hostfiles/publisher.go
index be508a715..36d9016f2 100644
--- a/internal/hostfiles/publisher.go
+++ b/internal/hostfiles/publisher.go
@@ -161,7 +161,12 @@ func (p *Publisher) Commit(ctx context.Context) (int64, error) {
// the reply never arrived, and this retry's etag is now stale. If the
// manifest holds exactly the bytes we were writing, it is ours.
cur, curEtag, gerr := p.s.Get(ctx, ManifestKey(p.prefix))
- if gerr != nil || !bytes.Equal(cur, body) {
+ if gerr != nil {
+ // Not knowing is not the same as being superseded: leave the
+ // staged state and etag alone so the next Commit repeats the check.
+ return 0, gerr
+ }
+ if !bytes.Equal(cur, body) {
return 0, fmt.Errorf("%w: %s", ErrSuperseded, p.prefix)
}
err, etag = nil, curEtag