diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go index f04d60c..57cc144 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go @@ -227,10 +227,10 @@ func (b *bed) get(t *testing.T, bucket, key string) []byte { return nil } -// 1297/B4, B5 on the bus: the controller sent F to g14 while the canary ace does not run F healthy yet. Started whole, -// the assigner wants F on the canary, writes the difference on g14 explained by rule 3, and on ace one unexplained (the -// controller sends ace the older fingerprint while ace wants F). -func TestWantsAndTheirDifferencesOnTheBus(t *testing.T) { +// 1297/B4 on the bus, 1906/D5: the controller sends ace and g14 builds while the canary ace does not run F healthy yet. +// Started whole, the assigner wants F on the canary, holds g14 back with no want, and writes no difference at all: there +// is no send to compare with (ADR 0539 §8). +func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { b := startBed(t, "") fNew, fOld := delivery.SourcePrefix2+strings.Repeat("b", 64), delivery.SourcePrefix2+strings.Repeat("a", 64) digest := b.manifest(t, "web", `{"module":"web","version":"1"}`) @@ -253,8 +253,6 @@ func TestWantsAndTheirDifferencesOnTheBus(t *testing.T) { } b.putJSON(t, provider.AssignmentsBucket, "ace.web", []byte(`{"node":"ace","module":"web","build":"b-8"}`)) b.putJSON(t, provider.AssignmentsBucket, "g14.web", []byte(`{"node":"g14","module":"web","build":"b-9"}`)) - b.putJSON(t, delivery.BuildsBucket, delivery.BuildKey("web", "b-8"), []byte(`{"id":"b-8","repository":"novox/web","source-fingerprint":"`+fOld+`"}`)) - b.putJSON(t, delivery.BuildsBucket, delivery.BuildKey("web", "b-9"), []byte(`{"id":"b-9","repository":"novox/web","source-fingerprint":"`+fNew+`"}`)) b.start(t) wkey, _ := delivery.WantKey("ace", "web") @@ -262,15 +260,9 @@ func TestWantsAndTheirDifferencesOnTheBus(t *testing.T) { if err != nil || want.FP != fNew { t.Fatalf("the canary ace's want = %+v, %v", want, err) } - dkey, _ := delivery.DifferenceKey("g14", "web") - d, err := delivery.DecodeDifference(b.get(t, delivery.AssignerDifferencesBucket, dkey)) - if err != nil || d.Rule != delivery.RuleCanary || d.Sent != fNew { - t.Fatalf("g14's difference = %+v, %v; want rule 3", d, err) - } - akey, _ := delivery.DifferenceKey("ace", "web") - a, err := delivery.DecodeDifference(b.get(t, delivery.AssignerDifferencesBucket, akey)) - if err != nil || !a.Unexplained || a.Want != fNew || a.Sent != fOld { - t.Fatalf("ace's difference = %+v, %v; want it unexplained", a, err) + time.Sleep(500 * time.Millisecond) + if keys, err := b.buckets[delivery.AssignerDifferencesBucket].Keys(b.ctx); err == nil && len(keys) != 0 { + t.Errorf("differences were written: %v", keys) } if e, err := b.buckets[delivery.AssignerWantsBucket].Get(b.ctx, "want.g14.web"); err == nil { t.Errorf("g14, held back by the canary with no want before, was given one: %s", e.Value()) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go index 8900bd8..eabfebc 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go @@ -39,7 +39,6 @@ func main() { // holds the two lists equal), so the controller's broker grants exactly these. var readBuckets = []string{ provider.AssignmentsBucket, - delivery.BuildsBucket, delivery.ControllerGrantsBucket, delivery.BuilderBuildsBucket, delivery.CandidatesBucket, 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 a626f95..4c7eeee 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -23,71 +23,13 @@ type passer struct { func (p *passer) pass(ctx context.Context) { p.compositions(ctx) - rules := p.wants(ctx) - p.differences(ctx, rules) -} - -// differences writes, for each node and module the assigner wants or the controller sends, the difference between the -// two with the rule that explains it or unexplained (ADR 0528 clause 4), and deletes the key of a difference that -// ended; its history keeps what it was. -func (p *passer) differences(ctx context.Context, rules map[string]map[string]assign.Holding) { - _, assignments := p.w.assigned() - wanted := p.w.wantsOf() - trunk := p.w.trunk() - pairs := map[[2]string]bool{} - for _, a := range assignments { - pairs[[2]string{a.Node, a.Module}] = true - } - for module, byNode := range wanted { - for node := range byNode { - pairs[[2]string{node, module}] = true - } - } - written := p.w.read(delivery.AssignerDifferencesBucket) - keep := map[string]bool{} - for pair := range pairs { - node, module := pair[0], pair[1] - key, err := delivery.DifferenceKey(node, module) - if err != nil { - continue - } - var want *delivery.Want - if w, ok := wanted[module][node]; ok { - want = &w - } - var sent assign.Sent - if a, ok := assignments[node+"."+module]; ok { - sent = assign.Sent{Sends: true, Build: a.Build, FP: p.w.sentFP(ctx, kvGetter(p.kvs[delivery.BuildsBucket]), module, a.Build)} - } - d, ok := assign.Compare(want, sent, trunk[module].FP2, rules[module][node]) - if !ok { - continue - } - keep[key] = true - if before, err := delivery.DecodeDifference(written[key]); err == nil { - before.V = 0 - if delivery.SameDifference(before, d) { - continue - } - } - p.put(ctx, delivery.AssignerDifferencesBucket, key, func() ([]byte, error) { return delivery.EncodeDifference(d) }) - } - for key := range written { - if keep[key] { - continue - } - if err := p.kvs[delivery.AssignerDifferencesBucket].Delete(ctx, key); err != nil { - p.logf("the difference %s that ended could not be deleted: %v", key, err) - continue - } - p.w.put(delivery.AssignerDifferencesBucket, key, nil) - } + p.wants(ctx) } // wants writes, for every module the trunk holds a fingerprint of, each node's want by the six rules (ADR 0429 §6), and -// the hold of a fingerprint whose health check failed. It returns, by module then node, the rule that holds a node -// back, for the comparison. -func (p *passer) wants(ctx context.Context) map[string]map[string]assign.Holding { +// the hold of a fingerprint whose health check failed. The assigner compares its wants with nothing the controller +// sends: the differences bucket is kept read-only with its history (ADR 0539 §8). +func (p *passer) wants(ctx context.Context) { get := kvGetter(p.kvs[delivery.ManifestsBucket]) assigned, _ := p.w.assigned() trunk := p.w.trunk() @@ -108,7 +50,6 @@ func (p *passer) wants(ctx context.Context) map[string]map[string]assign.Holding } } wanted := p.w.wantsOf() - rules := map[string]map[string]assign.Holding{} names := map[string]bool{} for name := range trunk { names[name] = true @@ -129,7 +70,6 @@ func (p *passer) wants(ctx context.Context) map[string]map[string]assign.Holding last[node] = p.w.lastHealthy(node, name, fp) } out := assign.Wants(m, nodes, wanted[name], last) - rules[name] = out.Rules if out.Held != nil { key, err := delivery.HeldKey(name, fp) if err == nil { @@ -150,7 +90,6 @@ func (p *passer) wants(ctx context.Context) map[string]map[string]assign.Holding p.put(ctx, delivery.AssignerWantsBucket, key, func() ([]byte, error) { return delivery.EncodeWant(want) }) } } - return rules } // compositions writes the composition check's verdict of every group the indexer holds a candidate for, under the diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go index 57cbbb4..1c185eb 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go @@ -44,8 +44,6 @@ type world struct { manifests map[string]parsed // healthy is, by have key, every fingerprint the node ran healthy, oldest first, each once: what a set-back goes to. healthy map[string][]string - // sent is, by build id, the fingerprint of the new form the controller's build record names, once it names one. - sent map[string]string } // parsed is a manifest read by digest: its module and its manifest, or why it cannot be read. @@ -56,7 +54,7 @@ type parsed struct { } func newWorld() *world { - return &world{values: map[string]map[string][]byte{}, manifests: map[string]parsed{}, healthy: map[string][]string{}, sent: map[string]string{}} + return &world{values: map[string]map[string][]byte{}, manifests: map[string]parsed{}, healthy: map[string][]string{}} } // put takes a change of a watched bucket in: a value, or nil for a key deleted or purged. @@ -344,29 +342,3 @@ func (w *world) built(module, fp string) bool { b, err := delivery.DecodeBuilderBuild(value) return err == nil && b.State == delivery.BuildDone } - -// sentFP is the fingerprint of the new form of the build the controller's assignment names, read from its build record -// (mesh-controller_builds `.build.`) once and kept; empty when the record names none. -func (w *world) sentFP(ctx context.Context, get getter, module, build string) string { - if build == "" { - return "" - } - w.mu.Lock() - fp, ok := w.sent[build] - w.mu.Unlock() - if ok { - return fp - } - value, err := get(ctx, delivery.BuildKey(module, build)) - if err != nil { - return "" - } - var rec delivery.BuildRecord - if json.Unmarshal(value, &rec) != nil || !strings.HasPrefix(rec.SourceFingerprint, delivery.SourcePrefix2) { - return "" - } - w.mu.Lock() - w.sent[build] = rec.SourceFingerprint - w.mu.Unlock() - return rec.SourceFingerprint -} diff --git a/modules/mesh-delivery-assigner/internal/assign/compare.go b/modules/mesh-delivery-assigner/internal/assign/compare.go deleted file mode 100644 index 952acd3..0000000 --- a/modules/mesh-delivery-assigner/internal/assign/compare.go +++ /dev/null @@ -1,52 +0,0 @@ -package assign - -import ( - "fmt" - - "git.novox.be/novox/mesh-sdk/go/delivery" -) - -// Sent is what the controller sends a node of a module: whether it sends it at all (an open assignment), the build its -// assignment names, and that build's fingerprint of the new form (empty when its record names none). -type Sent struct { - Sends bool - Build string - FP string -} - -// Compare is the difference between a node's want for a module and what the controller sends it (ADR 0528 clause 4), -// and whether there is one. want is nil when the assigner wrote none; newest is the module's newest fingerprint and -// hold the rule that holds the node back from it (Rule 0 when none does). A difference is explained by a rule when the -// controller sent the newest fingerprint while that rule holds the node's want back; every other is unexplained, said -// in words, for acceptance test 17 of design 47 to read. -func Compare(want *delivery.Want, sent Sent, newest string, hold Holding) (delivery.Difference, bool) { - var d delivery.Difference - removed := want != nil && want.Removed - if want != nil && !removed { - d.Want = want.FP - } - d.WantRemoved = removed - if !sent.Sends { - if want == nil || removed { - return delivery.Difference{}, false - } - d.Unexplained, d.Words = true, "the node wants "+short(want.FP)+" and the controller sends it nothing" - return d, true - } - d.Sent, d.SentBuild = sent.FP, sent.Build - switch { - case sent.FP == "": - d.Unexplained, d.Words = true, fmt.Sprintf("the build %s the controller sends names no fingerprint of the new form", sent.Build) - case removed: - d.Unexplained, d.Words = true, fmt.Sprintf("the want is removed and the controller still sends %s", short(sent.FP)) - case sent.FP == d.Want: - return delivery.Difference{}, false - case hold.Rule != 0 && sent.FP == newest: - d.Rule, d.Words = hold.Rule, fmt.Sprintf("the controller sent %s while %s", short(sent.FP), hold.Words) - case d.Want == "": - d.Unexplained, d.Words = true, fmt.Sprintf("the controller sends %s and the node has no want", short(sent.FP)) - default: - d.Unexplained, d.Words = true, fmt.Sprintf("the controller sends %s and the node wants %s", short(sent.FP), short(d.Want)) - } - return d, true -} diff --git a/modules/mesh-delivery-assigner/internal/assign/compare_test.go b/modules/mesh-delivery-assigner/internal/assign/compare_test.go deleted file mode 100644 index da4a14f..0000000 --- a/modules/mesh-delivery-assigner/internal/assign/compare_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package assign - -import ( - "testing" - - "git.novox.be/novox/mesh-sdk/go/delivery" -) - -// 1297/B5: given the controller sent F to a node while the canary is not yet healthy on F, the difference names rule 3. -func TestADifferenceExplainedByTheCanary(t *testing.T) { - m, n := ready() - n.Haves["ace"]["mail"] = delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} - out := Wants(m, n, wantedOld(), nil) - want := delivery.Want{FP: fOld, Newest: fNew, Rule: delivery.RuleCanary} - d, ok := Compare(&want, Sent{Sends: true, Build: "b-9", FP: fNew}, fNew, out.Rules["g14"]) - if !ok || d.Rule != delivery.RuleCanary || d.Unexplained || d.Want != fOld || d.Sent != fNew { - t.Fatalf("difference = %+v, %v; want rule 3", d, ok) - } - if _, err := delivery.EncodeDifference(d); err != nil { - t.Errorf("the difference cannot be written: %v", err) - } -} - -// 1297/B5: a want equal to what is sent is no difference; nothing sent and no want is none; every other is written -// unexplained, with words, and is writable. -func TestCompare(t *testing.T) { - w := delivery.Want{FP: fNew} - if _, ok := Compare(&w, Sent{Sends: true, Build: "b", FP: fNew}, fNew, Holding{}); ok { - t.Errorf("an equal want and send is a difference") - } - if _, ok := Compare(nil, Sent{}, fNew, Holding{}); ok { - t.Errorf("no want and nothing sent is a difference") - } - removed := delivery.Want{Removed: true} - if _, ok := Compare(&removed, Sent{}, fNew, Holding{}); ok { - t.Errorf("a removed want and nothing sent is a difference") - } - old := delivery.Want{FP: fOld} - for name, c := range map[string]struct { - want *delivery.Want - sent Sent - }{ - "sends an older one": {&w, Sent{Sends: true, Build: "b", FP: fOld}}, - "names no fingerprint": {&w, Sent{Sends: true, Build: "b"}}, - "sends a removed module": {&removed, Sent{Sends: true, Build: "b", FP: fNew}}, - "sends nothing": {&w, Sent{}}, - "no want": {nil, Sent{Sends: true, Build: "b", FP: fNew}}, - "no rule holds it back": {&old, Sent{Sends: true, Build: "b", FP: fNew}}, - } { - d, ok := Compare(c.want, c.sent, fNew, Holding{}) - if !ok || !d.Unexplained || d.Words == "" { - t.Errorf("%s: difference = %+v, %v; want it unexplained, with words", name, d, ok) - continue - } - if _, err := delivery.EncodeDifference(d); err != nil { - t.Errorf("%s: the difference cannot be written: %v", name, err) - } - } -} diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 524f823..96e83b4 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -117,7 +117,7 @@ type Outcome struct { Wants map[string]delivery.Want // Held is the hold of m.FP to write, when a node's health check failed on it and it is not held yet. Held *delivery.Held - // Rules is, by node, the rule holding the node back from m.FP and why, for the comparison. + // Rules is, by node, the rule holding the node back from m.FP and why. Rules map[string]Holding } diff --git a/modules/mesh-delivery-assigner/manifest_test.go b/modules/mesh-delivery-assigner/manifest_test.go index 03b879a..f0385fa 100644 --- a/modules/mesh-delivery-assigner/manifest_test.go +++ b/modules/mesh-delivery-assigner/manifest_test.go @@ -24,7 +24,7 @@ func load(t *testing.T) catalogue.Manifest { // 1297/B2a: the controller's reader (mesh-sdk catalogue, which the controller reads manifests with) takes the // manifest; it reads exactly the buckets the assigner opens (the program's list, held equal in its own test), keeps -// its three states, and holds a credential of its own. +// its four states (the differences kept read-only, 1906/D5), and holds a credential of its own. func TestManifest(t *testing.T) { m := load(t) if m.Module != "mesh-delivery-assigner" { @@ -32,7 +32,7 @@ func TestManifest(t *testing.T) { } reads := slices.Clone(m.Reads) slices.Sort(reads) - want := []string{"mesh-controller.assignments", "mesh-controller.builds", "mesh-controller.grants", + want := []string{"mesh-controller.assignments", "mesh-controller.grants", "mesh-delivery-builder.builds", "mesh-delivery-indexer.candidates", "mesh-delivery-indexer.manifests", "mesh-delivery-indexer.modules", "mesh-delivery-watchman.presence", "node-engine.have"} if !reflect.DeepEqual(reads, want) { diff --git a/modules/mesh-delivery-assigner/module.json b/modules/mesh-delivery-assigner/module.json index 418e8fb..0d42866 100644 --- a/modules/mesh-delivery-assigner/module.json +++ b/modules/mesh-delivery-assigner/module.json @@ -4,7 +4,6 @@ "slug": "assigner", "reads": [ "mesh-controller.assignments", - "mesh-controller.builds", "mesh-controller.grants", "mesh-delivery-builder.builds", "mesh-delivery-indexer.candidates",