amadan
offline return: the door's index cleanup is bounded (fix round 2, item 2)
The return stage or start door's index cleanup runs under its own bound (offlineReturnCleanupTimeout, 10 s) instead of an uncancellable context, so a stalled index store never holds the serial bump or the 202 past it; the entry it leaves is the sweep's. Witness: TestReturnIndexCleanupIsBounded (stage and start over a store whose index read waits for cancellation). RED: the handler blocked in the stalled read until the 600 s package timeout.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+52
−2
internal/console/apiinstancemove_offline.go+6 −2internal/console/offlinereturn.go+4 −0internal/console/offlinereturn_test.go+42 −0
diff --git a/internal/console/apiinstancemove_offline.go b/internal/console/apiinstancemove_offline.go| index 9355360e6..e55ce4324 100644 |
| --- a/internal/console/apiinstancemove_offline.go |
| +++ b/internal/console/apiinstancemove_offline.go |
| @@ -999,8 +999,12 @@ func (s *Server) offlineBegin(w http.ResponseWriter, r *http.Request, d offlineD |
| } |
| if sel.ReturnOf != "" { |
| // The return is staged or started: its index entry has done its |
| - // work (final review F6). |
| - s.offlineReturnIndexDone(context.WithoutCancel(ctx), sel.To, host, sel.ReturnOf) |
| + // work (final review F6). Best effort, bounded: a stalled index |
| + // store never holds the serial bump or the answer past the bound, |
| + // and what it leaves the sweep removes (fix round 2). |
| + cctx, ccancel := context.WithTimeout(context.WithoutCancel(ctx), offlineReturnCleanupTimeout) |
| + s.offlineReturnIndexDone(cctx, sel.To, host, sel.ReturnOf) |
| + ccancel() |
| } |
| s.bumpSerial(ctx) |
| writeJSON(w, http.StatusAccepted, s.offlineViewOf(ctx, got)) |
diff --git a/internal/console/offlinereturn.go b/internal/console/offlinereturn.go| index 10c2a8251..e13c2d40d 100644 |
| --- a/internal/console/offlinereturn.go |
| +++ b/internal/console/offlinereturn.go |
| @@ -199,6 +199,10 @@ func (s *Server) offlineReturnIndex(ctx context.Context, code, host, returnOf, r |
| return err |
| } |
| +// offlineReturnCleanupTimeout bounds the return door's index cleanup. A |
| +// var so a test can shorten it. |
| +var offlineReturnCleanupTimeout = 10 * time.Second |
| + |
| // offlineReturnIndexDone deletes the return index entry of returnOf in |
| // region code once the return is staged or started (the console owns the |
| // entry's lifetime, final review F6): if-match on the entry it read, so |
diff --git a/internal/console/offlinereturn_test.go b/internal/console/offlinereturn_test.go| index 9fed74c22..632e83c78 100644 |
| --- a/internal/console/offlinereturn_test.go |
| +++ b/internal/console/offlinereturn_test.go |
| @@ -588,3 +588,45 @@ func TestReturnSweepDeletesOnlyTheEntryItJudged(t *testing.T) { |
| t.Fatalf("the fresh request's index entry did not survive the sweep: %s %v", raw, err) |
| } |
| } |
| + |
| +// TestReturnIndexCleanupIsBounded (fix round 2, item 2): the return stage |
| +// or start door's index cleanup is best effort under its own bound: an |
| +// index store that stalls cannot hold the response or the serial bump |
| +// past offlineReturnCleanupTimeout; the entry it leaves is the sweep's. |
| +func TestReturnIndexCleanupIsBounded(t *testing.T) { |
| + defer func(d time.Duration) { offlineReturnCleanupTimeout = d }(offlineReturnCleanupTimeout) |
| + offlineReturnCleanupTimeout = 300 * time.Millisecond |
| + for _, door := range []string{"/stage", ""} { |
| + t.Run(map[string]string{"/stage": "stage", "": "start"}[door], func(t *testing.T) { |
| + w := newReturnWorld(t, nil) |
| + w.report("uk-box", "uk-box", "", w.request(), time.Now(), w.ready()) |
| + w.fx.srv.cfg.Releases = &stallStore{Store: w.fx.rel, key: moves.ReturnIndexKey("uk", offlineHost, returnFwd)} |
| + before := serialBytes(t, w.fx.rel) |
| + type answer struct { |
| + code int |
| + out string |
| + } |
| + done := make(chan answer, 1) |
| + start := time.Now() |
| + go func() { |
| + code, _, out := rawReq(t, http.MethodPost, w.base+door, w.token, `{"region":"uk","return_of":"`+returnFwd+`"}`) |
| + done <- answer{code, out} |
| + }() |
| + var a answer |
| + select { |
| + case a = <-done: |
| + case <-time.After(5 * time.Second): |
| + t.Fatal("the door did not answer while the index store stalled") |
| + } |
| + if a.code != http.StatusAccepted { |
| + t.Fatalf("%s = %d %s", door, a.code, a.out) |
| + } |
| + if took := time.Since(start); took > 3*time.Second { |
| + t.Fatalf("the door answered after %v, past the cleanup's bound", took) |
| + } |
| + if serialBytes(t, w.fx.rel) == before { |
| + t.Fatal("the door did not bump the serial") |
| + } |
| + }) |
| + } |
| +} |