From 07dee528c508076ce2715a3e3562c2784cae5d59 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 10:25:50 +0100 Subject: [PATCH] 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 +}