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 4185077..dc4eb35 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "time" @@ -433,15 +434,40 @@ func (p *passer) staleAsk(key string, trunk map[string]delivery.Module) bool { if !on { return true } - if m.FP2 == "" { - return false + // The assigner asks about a composition at more than the trunk fingerprint: a node held back by the rules is asked about the + // revised composition of the fingerprint it runs (ADR 0545), and a node's first want at the older fingerprint it runs. So a + // record ask is stale only when its digest is the composition of none of the fingerprints the node can still want (the + // trunk's, its want's, its have's), and only when every one of them is read. + for _, fp := range p.fingerprintsOf(k.Node, k.Module, m.FP2) { + d, ok := p.w.composed(k.Node, k.Module, fp) + if !ok || d.Digest == k.Composed { + return false + } } - d, ok := p.w.composed(k.Node, k.Module, m.FP2) - return ok && d.Digest != k.Composed + return len(p.fingerprintsOf(k.Node, k.Module, m.FP2)) > 0 } return false } +// fingerprintsOf are the fingerprints a node may still want of module: the trunk's, the one its want names and the one its have +// names, each once. +func (p *passer) fingerprintsOf(node, module, trunkFP string) []string { + var out []string + add := func(fp string) { + if fp != "" && !slices.Contains(out, fp) { + out = append(out, fp) + } + } + add(trunkFP) + add(p.w.wantsOf()[module][node].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) + } + } + return out +} + // staleAsks takes away, at every pass, each hold, restore and record ask that staleAsk names (issue 1906, note 28): an ask // outlived its fingerprint, and the teller takes its router ask back from the operator when the key goes. With the trunk not // read at all (no module), nothing is taken away. @@ -453,6 +479,14 @@ 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. + 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) + } } } } 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 9f9a5b3..12de196 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 @@ -521,16 +521,20 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { trunk map[string]delivery.Module gone []string // the keys taken away noDigit bool // the controller's composition is not read + heldAt string // the node's want is at this fingerprint, whose composition is digOld }{ - {"the trunk moved on", trunkOf(fpNew), []string{holdOld, holdGone, restOld, recOld}, false}, - {"the trunk is at the old fingerprint", trunkOf(fpOld), []string{holdNew, holdGone, restNew, recOld}, false}, - {"the trunk's fingerprint is not read yet", trunkOf(""), []string{holdGone}, false}, - {"the composition is not read yet", trunkOf(fpNew), []string{holdOld, holdGone, restOld}, true}, - {"the trunk is not read at all", nil, nil, false}, + {"the trunk moved on", trunkOf(fpNew), []string{holdOld, holdGone, restOld, recOld}, false, ""}, + {"the trunk is at the old fingerprint", trunkOf(fpOld), []string{holdNew, holdGone, restNew, recOld}, false, ""}, + {"the trunk's fingerprint is not read yet", trunkOf(""), []string{holdGone}, false, ""}, + {"the composition is not read yet", trunkOf(fpNew), []string{holdOld, holdGone, restOld}, true, ""}, + {"a node held back at the old fingerprint is still asked about its composition", trunkOf(fpNew), []string{holdOld, holdGone, restOld}, false, fpOld}, + {"the trunk is not read at all", nil, nil, false, ""}, } { t.Run(c.name, func(t *testing.T) { - asksKV := &memKV{values: map[string][]byte{}} - p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV}, time.Now()) + asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now()) + var said []asks.Condition + p.say = func(c asks.Condition) error { said = append(said, c); return nil } for _, key := range all { k, _ := delivery.ParseAnyAskKey(key) var a delivery.AssignerAsk @@ -556,9 +560,30 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { v, _ := delivery.EncodeComposedDigest(delivery.ComposedDigest{Digest: digNew, Judgement: delivery.JudgedUnchanged}) p.w.put(delivery.ControllerComposedBucket, ckey, v) okey, _ := delivery.ComposedKey("novox", "web", fpOld) - p.w.put(delivery.ControllerComposedBucket, okey, v) + old, _ := delivery.EncodeComposedDigest(delivery.ComposedDigest{Digest: digNew, Judgement: delivery.JudgedUnchanged}) + if c.heldAt != "" { + old, _ = delivery.EncodeComposedDigest(delivery.ComposedDigest{Digest: digOld, Judgement: delivery.JudgedUnchanged}) + } + p.w.put(delivery.ControllerComposedBucket, okey, old) } + if c.heldAt != "" { + wkey, _ := delivery.WantKey("novox", "web") + w, _ := delivery.EncodeWant(delivery.Want{FP: c.heldAt, Composed: digOld}) + p.w.put(delivery.AssignerWantsBucket, wkey, w) + } + heldOld, _ := delivery.HeldKey("web", fpOld) + wantsKV.values[heldOld] = []byte("x") + p.w.put(delivery.AssignerWantsBucket, heldOld, []byte("x")) 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) + _, heldThere := wantsKV.values[heldOld] + 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) + } for _, key := range all { _, there := asksKV.values[key] want := !slices.Contains(c.gone, key) @@ -593,3 +618,53 @@ func mustKey(key string, err error) string { } return key } + +// 1906 note 28, review: a node held back at an earlier fingerprint is asked about the revised composition of that fingerprint +// (ADR 0545). That record ask is not about the trunk's fingerprint: it survives every pass, so the operator is asked, and on a +// yes the want names the revised digest. +func TestARecordAskOfAHeldBackNodeSurvivesPassesAndItsYesMovesTheWant(t *testing.T) { + fpOld := delivery.SourcePrefix2 + strings.Repeat("a", 64) + revised, was := "sha256:"+strings.Repeat("e", 64), "sha256:"+strings.Repeat("d", 64) + asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now()) + p.logf = t.Logf + put := func(bucket, key string, v []byte) { p.w.put(bucket, key, v) } + mod, _ := delivery.EncodeModule(delivery.Module{FP2: fpTest, Repository: "novox/web", Commit: strings.Repeat("1", 40), Path: delivery.RootPath}) + put(delivery.ModulesBucket, "web", mod) // the trunk's build is not done: rule 1 holds g14 back at fpOld + put(provider.AssignmentsBucket, "g14.web", []byte(`{"node":"g14","module":"web","build":"b-1"}`)) + on, _ := delivery.EncodePresence(delivery.Presence{Online: true}) + put(delivery.PresenceBucket, "g14", on) + have, _ := delivery.EncodeHave(delivery.Have{FP: fpOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) + put(delivery.HaveBucket, "g14.web", have) + wkey, _ := delivery.WantKey("g14", "web") + want, _ := delivery.EncodeWant(delivery.Want{FP: fpOld, Composed: was}) + wantsKV.values[wkey] = want + put(delivery.AssignerWantsBucket, wkey, want) + ckey, _ := delivery.ComposedKey("g14", "web", fpOld) + cd, cerr := delivery.EncodeComposedDigest(delivery.ComposedDigest{Digest: revised, Judgement: delivery.JudgedRecreates, Policy: delivery.PolicyRecord, PolicyFrom: "default", Against: was, Words: "recreates"}) + if cerr != nil { + t.Fatal(cerr) + } + put(delivery.ControllerComposedBucket, ckey, cd) + akey, _ := delivery.RecordAskKey("g14", "web", revised) + for i := 0; i < 2; i++ { + p.wants(context.Background()) + if _, there := asksKV.values[akey]; !there { + t.Fatalf("pass %d: the record ask of the held-back node is not there", i+1) + } + } + a, err := delivery.DecodeAssignerAsk(akey, asksKV.values[akey]) + if err != nil { + t.Fatal(err) + } + yes, _ := delivery.EncodeAnswer(delivery.Answer{Option: assign.OptionYes, Warrant: a.Ask.ID + "@2026-10-26T01:00:00Z", Words: "the operator"}) + put(delivery.AnswersBucket, akey, yes) + p.wants(context.Background()) + got, err := delivery.DecodeWant(wantsKV.values[wkey]) + if err != nil || got.FP != fpOld || got.Composed != revised { + t.Errorf("after the yes the want is %+v (%v), want it at %s naming %s", got, err, fpOld, revised) + } + if _, there := asksKV.values[akey]; !there { + t.Error("the answered record ask was taken away") + } +}