amadan
offline reservations: two lost races with no holder observed answer the unreadable 502 (final review F14)
After two CAS conflicts in which no read saw a holder, a final read that fails, is malformed or still names nobody answers the existing unreadable 502 (the agreed ruling) instead of a holder invented from the reserver's own host. Witness: TestReserveLosingTwoRacesIsBusy, three no-holder cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+65
−12
internal/console/offlinereserve.go+19 −11internal/console/offlinereserve_test.go+46 −1
diff --git a/internal/console/offlinereserve.go b/internal/console/offlinereserve.go| index 3f6a4c828..44c39c0ba 100644 |
| --- a/internal/console/offlinereserve.go |
| +++ b/internal/console/offlinereserve.go |
| @@ -247,19 +247,27 @@ func (s *Server) reserveBoxes(ctx context.Context, sel offlineSelection, r moves |
| return offlineReserveUnreadable |
| } |
| } |
| - // Two lost races: busy. When neither read saw a holder (the object |
| - // was created under both of them), one more read names it; failing |
| - // that, the sentence names the reserver's own host, which is moving |
| - // nowhere yet but is the one the caller asked about. |
| + // Two lost races: busy, naming the holder. When neither read saw a |
| + // holder (the object was created under both of them), one more read |
| + // names it; a read that fails, is malformed or still names nobody |
| + // cannot say who holds the boxes, so the answer is the unreadable 502, |
| + // never a holder invented from the reserver's own host (final review |
| + // F14). |
| if lastHolder == "" { |
| - if raw, _, err := s.cfg.Releases.Get(ctx, moves.OfflineActiveKey); err == nil { |
| - if table, err := moves.ParseOfflineActive(raw); err == nil { |
| - lastHolder = moves.OfflineActiveBody(table).Host |
| - } |
| + raw, _, err := s.cfg.Releases.Get(ctx, moves.OfflineActiveKey) |
| + if err != nil { |
| + log.Printf("console: offline move %s: reading the reservations after two lost races: %v", r.Host, err) |
| + return offlineReserveUnreadable |
| + } |
| + table, err := moves.ParseOfflineActive(raw) |
| + if err != nil { |
| + log.Printf("console: offline move %s: %v", r.Host, err) |
| + return offlineReserveUnreadable |
| + } |
| + if lastHolder = moves.OfflineActiveBody(table).Host; lastHolder == "" { |
| + log.Printf("console: offline move %s: two lost races and no holder named", r.Host) |
| + return offlineReserveUnreadable |
| } |
| - } |
| - if lastHolder == "" { |
| - lastHolder = r.Host |
| } |
| return offlineBusy(lastHolder) |
| } |
diff --git a/internal/console/offlinereserve_test.go b/internal/console/offlinereserve_test.go| index df66dbad6..7d273d133 100644 |
| --- a/internal/console/offlinereserve_test.go |
| +++ b/internal/console/offlinereserve_test.go |
| @@ -1043,6 +1043,9 @@ func TestReserveLosingTwoRacesIsBusy(t *testing.T) { |
| t.Fatalf("%d writes attempted, want two", n) |
| } |
| }) |
| + // No holder was ever observed (final review F14): the final read that |
| + // stays empty, fails or is malformed is the unreadable 502, never a |
| + // holder invented from the reserver's own host. |
| t.Run("no holder is ever seen", func(t *testing.T) { |
| w := newReserveWorld(t) |
| lt := <Store{Store: w.fx.rel} |
| @@ -1053,11 +1056,53 @@ func TestReserveLosingTwoRacesIsBusy(t *testing.T) { |
| } |
| return nil |
| }) |
| - w.wantReserve(sentinelSel(), mres(mvA, offlineHost, 0), refBusyAcme) |
| + w.wantReserve(sentinelSel(), mres(mvA, offlineHost, 0), refUnread) |
| if w.activeRaw() != "" { |
| t.Fatal("a lost race wrote active.json") |
| } |
| }) |
| + t.Run("no holder seen and the final read fails", func(t *testing.T) { |
| + w := newReserveWorld(t) |
| + lt := <Store{Store: w.fx.rel} |
| + w.fx.srv.cfg.Releases = lt |
| + gets := 0 |
| + lt.setHook(func(op, key string) error { |
| + if key != moves.OfflineActiveKey { |
| + return nil |
| + } |
| + switch op { |
| + case "put": |
| + return release.ErrCASConflict |
| + case "get": |
| + if gets++; gets == 3 { |
| + return errors.New("the store is unreachable") |
| + } |
| + } |
| + return nil |
| + }) |
| + w.wantReserve(sentinelSel(), mres(mvA, offlineHost, 0), refUnread) |
| + if gets != 3 { |
| + t.Fatalf("%d reads, want the two attempts' and the final one", gets) |
| + } |
| + }) |
| + t.Run("no holder seen and the final read is malformed", func(t *testing.T) { |
| + w := newReserveWorld(t) |
| + lt := <Store{Store: w.fx.rel} |
| + w.fx.srv.cfg.Releases = lt |
| + puts := 0 |
| + lt.setHook(func(op, key string) error { |
| + if op != "put" || key != moves.OfflineActiveKey { |
| + return nil |
| + } |
| + if puts++; puts == 2 { |
| + if _, err := w.fx.rel.Put(context.Background(), key, []byte(`{"entries":`), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + return release.ErrCASConflict |
| + }) |
| + w.wantReserve(sentinelSel(), mres(mvA, offlineHost, 0), refUnread) |
| + }) |
| } |
| // landThenConflict lands the first write of key and then answers it as a |