amadan

carlos / platform Public

host: an unknown instance id is no negative in the return scan (final review F7)

With the metadata service not answering at the first scan, mayAnswer
compared an empty instance id with the pinned source participant's and
the scan cached the forward as a negative, so the box never answered that
return until a restart. mayAnswer now says whether it knows: a source
participant pinned by this box's id and an instance id is not judged while
the box's own id is "". The scan keeps the open record (no reread), caches
nothing as a negative, and judges again each scan; a negative is cached
only once known.

Witness: TestReturnScanRetriesUnknownInstanceID (empty id on the first two
scans, the pinned id later, a report without restarting the worker; a box
on another instance still answers nothing).

Census: the internal/host envelope changes (no pinned unit body).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 5e774b06f0dec9caa6eaba0ee1678f91c80e028e parent 78f67fb
2 files changed, +77 −9
  • internal/host/movesoffline_return.go +37 −9
  • internal/host/movesoffline_return_test.go +40 −0
diff --git a/internal/host/movesoffline_return.go b/internal/host/movesoffline_return.go
index dc0fc914c..601b493dc 100644
--- a/internal/host/movesoffline_return.go
+++ b/internal/host/movesoffline_return.go
@@ -10,7 +10,8 @@ package host
// index of its own region (moves.ReturnIndexPrefix: one LIST, whatever the
// number of hosts), which the console's return door writes beside each
// request. For each pending return it reads the forward's open record once
-// (cached, and a record it may not answer for cached as a negative), lists
+// (cached, and a record it knows it may not answer for cached as a
+// negative; with its own instance id not known yet, neither), lists
// that forward's requests, takes the current one
// (moves.CurrentReturnRequest) and, when no report of this box answers it
// yet, writes one create-only; then it deletes the index entry if it is
@@ -44,9 +45,11 @@ import (
const offlineReturnScanEvery = time.Minute
// returnState is the worker's memory of the scan, under pass: the open
-// records of the pending returns, and the pending returns this box may not
-// answer for, both by "<host>/<forward id>" and both pruned to the index
-// each scan.
+// records of the pending returns not known to be another box's (answered,
+// or waiting for this box's instance id), and the pending returns this box
+// may not answer for, both by "<host>/<forward id>" and both pruned to the
+// index each scan. A negative is cached only once it is known: the
+// instance id cannot change under a running process.
type returnState struct {
scanned time.Time
opens map[string]*moves.OpenRecord
@@ -62,14 +65,29 @@ func (w *movesWorker) returnScanDue(now time.Time) bool {
return every < 0 || w.returns.scanned.IsZero() || now.Sub(w.returns.scanned) >= every
}
-// mayAnswer reports whether this box may answer returns of the forward o.
-func (w *movesWorker) mayAnswer(o *moves.OpenRecord) bool {
+// mayAnswer reports whether this box may answer returns of the forward o,
+// and whether it knows: a source participant pinned by this box's id and
+// an instance id cannot be judged while this box's own instance id is not
+// known yet (the metadata service has not answered), and the scan then
+// decides nothing for o until it is.
+func (w *movesWorker) mayAnswer(o *moves.OpenRecord) (may, known bool) {
cfg := w.cfg
if o.Participants != nil {
- return moves.ReturnParticipant(*o, cfg.BoxID, cfg.instance(), "")
+ instance := cfg.instance()
+ if moves.ReturnParticipant(*o, cfg.BoxID, instance, "") {
+ return true, true
+ }
+ if instance == "" {
+ for _, p := range o.Participants.Source {
+ if p.BoxID != "" && p.BoxID == cfg.BoxID && p.InstanceID != "" {
+ return false, false
+ }
+ }
+ }
+ return false, true
}
here := cfg.parkRegion()
- return here != "" && regionname.PlacementMatches(cfg.ParkHomeRegion, o.From, here)
+ return here != "" && regionname.PlacementMatches(cfg.ParkHomeRegion, o.From, here), true
}
// offlineReturns is the pass's return step, a scan when one is due:
@@ -111,12 +129,22 @@ func (w *movesWorker) offlineReturns(ctx context.Context, s release.Store) {
w.notice(k+"#offline-return", "moves: %s: the open record of %s: %v", host, fwd, err)
continue
}
- if o == nil || !w.mayAnswer(o) {
+ if o == nil {
negatives[k] = true
continue
}
}
+ // Judged every scan (the record is cached, the judgement is not):
+ // an answer not known yet is neither a negative nor an answer.
+ may, known := w.mayAnswer(o)
+ if known && !may {
+ negatives[k] = true
+ continue
+ }
opens[k] = o
+ if !known {
+ continue
+ }
if err := w.answerReturn(ctx, s, o, key); err != nil {
w.notice(k+"#offline-return", "moves: %s: the return of %s: %v", host, fwd, err)
}
diff --git a/internal/host/movesoffline_return_test.go b/internal/host/movesoffline_return_test.go
index 427b0137c..2b29426f6 100644
--- a/internal/host/movesoffline_return_test.go
+++ b/internal/host/movesoffline_return_test.go
@@ -273,3 +273,43 @@ func TestReturnRequestReadFailurePreservesIndexAndRejectsOlderReport(t *testing.
})
}
}
+
+// TestReturnScanRetriesUnknownInstanceID (final review F7): a box whose
+// instance id is not known yet cannot tell whether it is the pinned source
+// participant, so its scan decides nothing for that return and caches no
+// negative; once the metadata service answers, the same worker (no
+// restart) answers the request.
+func TestReturnScanRetriesUnknownInstanceID(t *testing.T) {
+ rw := newReturnWorld(t)
+ id := ""
+ rw.ie.w.cfg.InstanceID = ""
+ rw.ie.w.cfg.InstanceIDOf = func() string { return id }
+ r1 := rw.request(1)
+ rw.scan()
+ if rw.answered(r1) {
+ t.Fatal("a box with no instance id answered for the pinned source")
+ }
+ if !rw.indexed() {
+ t.Fatal("the index entry was deleted while the box's identity was not known")
+ }
+ rw.scan()
+ id = ssSourceInstance // the metadata service answers now
+ rw.scan()
+ if !rw.answered(r1) {
+ t.Fatal("no report once the box's instance id is known")
+ }
+ if rw.indexed() {
+ t.Fatal("the index entry outlived the answer")
+ }
+
+ // A box that is known not to be the pinned source (another instance)
+ // still answers nothing.
+ other := newReturnWorld(t)
+ other.ie.w.cfg.InstanceID = "i-replacement"
+ r := other.request(1)
+ other.scan()
+ other.scan()
+ if other.answered(r) {
+ t.Fatal("a replaced box answered for the pinned source")
+ }
+}