From 07dee528c508076ce2715a3e3562c2784cae5d59 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 10:25:50 +0100 Subject: [PATCH 1/3] Assigner: take away hold, restore and record asks whose fingerprint the trunk left behind (issue 1906) An ask outlived its fingerprint: a lift left the restore asks of its fingerprint on every node, and no pass removed an ask of a fingerprint that is no longer the module's trunk fingerprint, so the operator kept being asked about a module that runs the trunk healthy. Each pass now takes those asks away (the teller then withdraws the router ask), and a lift takes the restore asks of its fingerprint. Nothing is taken away while the trunk or a composition is unread. --- .../cmd/mesh-delivery-assigner/pass.go | 51 ++++++++++ .../cmd/mesh-delivery-assigner/pass_test.go | 98 +++++++++++++++++++ 2 files changed, 149 insertions(+) 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 32a1dcc..4185077 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -98,6 +98,7 @@ func (p *passer) wants(ctx context.Context) { } wanted := p.w.wantsOf() puts := p.putsByModule() + p.staleAsks(ctx, trunk) names := map[string]bool{} for name := range trunk { names[name] = true @@ -404,6 +405,56 @@ func (p *passer) lift(ctx context.Context, module, fp string) { } p.remove(ctx, delivery.AssignerWantsBucket, held) p.remove(ctx, delivery.AssignerAsksBucket, ask) + // The restore asks of that fingerprint, on every node, go with the hold: every node runs it healthy, so no node waits to be + // set back from it (issue 1906, note 28). + for key := range p.w.read(delivery.AssignerAsksBucket) { + if k, err := delivery.ParseAnyAskKey(key); err == nil && k.Kind == delivery.AskKindRestore && k.Module == module && k.FP == fp { + p.remove(ctx, delivery.AssignerAsksBucket, key) + } + } +} + +// staleAsk says whether the hold, restore or record ask at key (read as the SDK reads it) is about something the trunk has left +// behind: a module no longer on the trunk, a fingerprint that is no longer the module's trunk fingerprint, or a composition the +// controller no longer composes for the node at that fingerprint. A backup ask and an ask of another kind are never stale here. +// A module of the trunk whose fingerprint is not read yet, and a composition not read yet, are not stale: nothing known is +// nothing taken away. +func (p *passer) staleAsk(key string, trunk map[string]delivery.Module) bool { + k, err := delivery.ParseAnyAskKey(key) + if err != nil { + return false + } + switch k.Kind { + case delivery.AskKindHold, delivery.AskKindRestore: + m, on := trunk[k.Module] + return !on || (m.FP2 != "" && m.FP2 != k.FP) + case delivery.AskKindRecord: + m, on := trunk[k.Module] + if !on { + return true + } + if m.FP2 == "" { + return false + } + d, ok := p.w.composed(k.Node, k.Module, m.FP2) + return ok && d.Digest != k.Composed + } + return false +} + +// 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. +func (p *passer) staleAsks(ctx context.Context, trunk map[string]delivery.Module) { + if len(trunk) == 0 { + return + } + for key := range p.w.read(delivery.AssignerAsksBucket) { + 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) + } + } } // remove deletes one key of the assigner's own bucket when it holds it; a delete that fails is said and made again at the next 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 ff438f1..9f9a5b3 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 @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strings" "testing" "time" @@ -495,3 +496,100 @@ func TestAHeldModuleRaisesNoNeitherAndClearsAStandingOne(t *testing.T) { t.Errorf("after a start: %d raised, %d cleared, want 0 and 1: %+v", r, c, *said) } } + +// 1906 note 28: an ask outlives its fingerprint. Each pass takes away the hold, restore and record asks of a module whose +// fingerprint (or composition) is not the trunk's now, or whose module left the trunk; a trunk not read takes nothing away; a +// backup ask is left; and a lift takes the restore asks of its fingerprint on every node. +func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { + fpOld, fpNew := delivery.SourcePrefix2+strings.Repeat("a", 64), delivery.SourcePrefix2+strings.Repeat("c", 64) + digOld, digNew := "sha256:"+strings.Repeat("d", 64), "sha256:"+strings.Repeat("e", 64) + trunkOf := func(fp string) map[string]delivery.Module { + return map[string]delivery.Module{"web": {FP2: fp, Manifest: "sha256:" + strings.Repeat("1", 64), Repository: "novox/web", Commit: strings.Repeat("1", 40), Path: delivery.RootPath}} + } + type ask struct{ name, key string } + holdOld, _ := delivery.HoldAskKey("web", fpOld) + holdNew, _ := delivery.HoldAskKey("web", fpNew) + holdGone, _ := delivery.HoldAskKey("gone", fpOld) + restOld, _ := delivery.RestoreAskKey("novox", "web", fpOld) + restNew, _ := delivery.RestoreAskKey("novox", "web", fpNew) + recOld, _ := delivery.RecordAskKey("novox", "web", digOld) + recNew, _ := delivery.RecordAskKey("novox", "web", digNew) + backOld, _ := delivery.BackupAskKey("novox", "web", fpOld) + all := []string{holdOld, holdNew, holdGone, restOld, restNew, recOld, recNew, backOld} + for _, c := range []struct { + name string + trunk map[string]delivery.Module + gone []string // the keys taken away + noDigit bool // the controller's composition is not read + }{ + {"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}, + } { + t.Run(c.name, func(t *testing.T) { + asksKV := &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV}, time.Now()) + for _, key := range all { + k, _ := delivery.ParseAnyAskKey(key) + var a delivery.AssignerAsk + switch k.Kind { + case delivery.AskKindHold: + a = assign.HoldAsk(key, k.Module, k.FP, "novox", "w", 1, time.Now()) + case delivery.AskKindRestore: + a = assign.RestoreAsk(key, k.Node, k.Module, k.FP, "w", time.Now()) + case delivery.AskKindRecord: + a = assign.RecordAsk(key, k.Node, k.Module, fpNew, assign.RecordNeed{Node: k.Node, Digest: k.Composed}, time.Now()) + default: + a = assign.BackupAsk(key, k.Node, k.Module, k.FP, digOld, "w", 2, time.Now()) + } + v, err := delivery.EncodeAssignerAsk(key, a) + if err != nil { + t.Fatal(err) + } + asksKV.values[key] = v + p.w.put(delivery.AssignerAsksBucket, key, v) + } + if !c.noDigit { + ckey, _ := delivery.ComposedKey("novox", "web", fpNew) + 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) + } + p.staleAsks(context.Background(), c.trunk) + for _, key := range all { + _, there := asksKV.values[key] + want := !slices.Contains(c.gone, key) + if there != want { + t.Errorf("%s: left in the bucket = %v, want %v", key, there, want) + } + } + }) + } + + t.Run("a lift takes the restore asks of its fingerprint on every node", func(t *testing.T) { + asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now()) + keep, _ := delivery.RestoreAskKey("ace", "web", fpNew) + keepOther, _ := delivery.RestoreAskKey("ace", "other", fpOld) + for _, key := range []string{restOld, mustKey(delivery.RestoreAskKey("ace", "web", fpOld)), keep, keepOther} { + asksKV.values[key] = []byte("x") + p.w.put(delivery.AssignerAsksBucket, key, []byte("x")) + } + p.lift(context.Background(), "web", fpOld) + for key, gone := range map[string]bool{restOld: true, mustKey(delivery.RestoreAskKey("ace", "web", fpOld)): true, keep: false, keepOther: false} { + if _, there := asksKV.values[key]; there == gone { + t.Errorf("%s: left in the bucket = %v after the lift", key, there) + } + } + }) +} + +func mustKey(key string, err error) string { + if err != nil { + panic(err) + } + return key +} -- 2.54.0 From 98a05997aeb84be78c07e123056c9e6646cccad4 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 10:31:01 +0100 Subject: [PATCH 2/3] Assigner: keep a held-back node's record ask, and clear the held record and condition of a stale hold (issue 1906) Review round 1: a record ask is about the composition of any fingerprint a node may still want (trunk, want, have), not only the trunk's, so deleting it looped with the wants rule that asks it again and the operator was never asked. It is now stale only when the module left the trunk or no read composition of those fingerprints has its digest. A hold or restore ask taken away as stale also takes its held record and clears its held condition. --- .../cmd/mesh-delivery-assigner/pass.go | 42 ++++++++- .../cmd/mesh-delivery-assigner/pass_test.go | 91 +++++++++++++++++-- 2 files changed, 121 insertions(+), 12 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 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") + } +} -- 2.54.0 From 769dab43a24d71443eff2aa41ee2af011edebedb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 10:33:30 +0100 Subject: [PATCH 3/3] 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()) -- 2.54.0