From 769dab43a24d71443eff2aa41ee2af011edebedb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 10:33:30 +0100 Subject: [PATCH] Assigner: keep the held record of a fingerprint whose asks went stale (issue 1906) The held record is the memory that a fingerprint failed (ADR 0408 section 4); a revert returns the trunk to exactly it, and without the key rule 3 would move the canary onto the failed build with nothing asked. A stale hold ask now clears only the condition (raised again on a revisit), and the hold is asked again under a new asking. Also read the single want key for a record ask. --- .../cmd/mesh-delivery-assigner/pass.go | 15 +++++++++------ .../cmd/mesh-delivery-assigner/pass_test.go | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index dc4eb35..50e3471 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -459,7 +459,11 @@ func (p *passer) fingerprintsOf(node, module, trunkFP string) []string { } } add(trunkFP) - add(p.w.wantsOf()[module][node].FP) + if key, err := delivery.WantKey(node, module); err == nil { + if w, err := delivery.DecodeWant(p.w.value(delivery.AssignerWantsBucket, key)); err == nil { + add(w.FP) + } + } if key, err := delivery.HaveKey(node, module); err == nil { if h, err := delivery.DecodeHave(p.w.value(delivery.HaveBucket, key)); err == nil { add(h.FP) @@ -479,13 +483,12 @@ func (p *passer) staleAsks(ctx context.Context, trunk map[string]delivery.Module if p.staleAsk(key, trunk) { p.logf("the ask %s is about a fingerprint or composition the trunk has left behind and is taken away", key) p.remove(ctx, delivery.AssignerAsksBucket, key) - // A hold or restore ask of a fingerprint the trunk left goes with the hold itself: its held record, and its condition, - // which would else ask for an answer that can no longer be given. + // The held record stays: it is the memory that this fingerprint failed (ADR 0408 ยง4), and a revert returns the trunk to + // exactly it. Only the condition, which would else ask for an answer that can no longer be given, is cleared, and it is + // raised again if the trunk returns to the fingerprint. if k, err := delivery.ParseAnyAskKey(key); err == nil && (k.Kind == delivery.AskKindHold || k.Kind == delivery.AskKindRestore) { - if held, err := delivery.HeldKey(k.Module, k.FP); err == nil { - p.remove(ctx, delivery.AssignerWantsBucket, held) - } p.clearHeld(k.Module, k.FP) + delete(p.said, assign.HeldCondition(k.Module, k.FP, "", "", false, false).Key+".raised") } } } diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 12de196..537f2e2 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -574,6 +574,7 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { heldOld, _ := delivery.HeldKey("web", fpOld) wantsKV.values[heldOld] = []byte("x") p.w.put(delivery.AssignerWantsBucket, heldOld, []byte("x")) + firstAsk, _ := delivery.DecodeAssignerAsk(holdOld, asksKV.values[holdOld]) p.staleAsks(context.Background(), c.trunk) // the held record and condition of a fingerprint whose hold ask went are taken away with it heldOld, _ = delivery.HeldKey("web", fpOld) @@ -581,8 +582,8 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { cleared := slices.ContainsFunc(said, func(c asks.Condition) bool { return c.Key == assign.HeldCondition("web", fpOld, "", "", false, false).Key && c.State == asks.StateCleared }) - if gone := slices.Contains(c.gone, holdOld); heldThere == gone || cleared != gone { - t.Errorf("hold ask of the old fingerprint gone = %v, but its held record left = %v and its condition cleared = %v", gone, heldThere, cleared) + if gone := slices.Contains(c.gone, holdOld); !heldThere || cleared != gone { + t.Errorf("hold ask of the old fingerprint gone = %v: its held record stays = %v (must), its condition cleared = %v", gone, heldThere, cleared) } for _, key := range all { _, there := asksKV.values[key] @@ -591,6 +592,17 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { t.Errorf("%s: left in the bucket = %v, want %v", key, there, want) } } + if slices.Contains(c.gone, holdOld) { + // the trunk returns to the fingerprint (a revert): the held record still holds it, and it is asked and said again + p.now = func() time.Time { return time.Now().Add(time.Hour) } + p.holdAsk(context.Background(), "web", fpOld, nil) + p.heldCondition(context.Background(), "web", fpOld, nil, false) + again, err := delivery.DecodeAssignerAsk(holdOld, asksKV.values[holdOld]) + raised := slices.ContainsFunc(said, func(c asks.Condition) bool { return c.State == asks.StateRaised }) + if err != nil || again.Ask.ID == firstAsk.Ask.ID || !raised { + t.Errorf("the trunk returned to the held fingerprint: ask %q (first %q, %v), condition raised again = %v", again.Ask.ID, firstAsk.Ask.ID, err, raised) + } + } }) } @@ -646,6 +658,9 @@ func TestARecordAskOfAHeldBackNodeSurvivesPassesAndItsYesMovesTheWant(t *testing t.Fatal(cerr) } put(delivery.ControllerComposedBucket, ckey, cd) + tkey, _ := delivery.ComposedKey("g14", "web", fpTest) // the trunk's own composition, another digest than the revised one + tcd, _ := delivery.EncodeComposedDigest(delivery.ComposedDigest{Digest: "sha256:" + strings.Repeat("f", 64), Judgement: delivery.JudgedUnchanged}) + put(delivery.ControllerComposedBucket, tkey, tcd) akey, _ := delivery.RecordAskKey("g14", "web", revised) for i := 0; i < 2; i++ { p.wants(context.Background())