From fcac03a4f21bf124e94b2fdbca0226c9665521a8 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 24 Oct 2026 18:15:13 +0200 Subject: [PATCH] Review round 1: the bus with no want is compared with its have; a backup ask is bound to its composition and number and each answer is a distinct want value; the view reads wants and haves, with dotted module names (issue 1906) --- .../integration_test.go | 71 +++++++++++--- .../cmd/mesh-delivery-assigner/main.go | 2 +- .../cmd/mesh-delivery-assigner/pass.go | 92 +++++++++++-------- .../cmd/mesh-delivery-assigner/world.go | 10 ++ .../internal/assign/asks.go | 40 ++++++-- .../internal/assign/asks_test.go | 15 ++- .../internal/assign/wants.go | 15 ++- .../internal/assign/wants_test.go | 19 +++- .../mesh-delivery-view/internal/view/view.go | 59 ++++++++++-- .../internal/view/wants_test.go | 39 +++++++- 10 files changed, 283 insertions(+), 79 deletions(-) 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 2f732fd..8212bb4 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 @@ -568,9 +568,12 @@ func (b *bed) wantNow(t *testing.T, node, module, what string, ok func(delivery. } // answer says the teller wrote the operator's answer to the ask at key. -func (b *bed) answer(t *testing.T, key, option string) { +func (b *bed) answer(t *testing.T, key, option string) { b.answerTo(t, key, option, "r1") } + +// answerTo is answer to the ask with this id (the router's warrant names it). +func (b *bed) answerTo(t *testing.T, key, option, askID string) { t.Helper() - v, err := delivery.EncodeAnswer(delivery.Answer{Option: option, Warrant: "r1@2026-10-24T12:00:00Z", Words: "the operator"}) + v, err := delivery.EncodeAnswer(delivery.Answer{Option: option, Warrant: askID + "@2026-10-24T12:00:00Z", Words: "the operator"}) if err != nil { t.Fatal(err) } @@ -619,30 +622,70 @@ func TestAReleaseAnswerWritesTheWantAgain(t *testing.T) { t.Error("the condition of the held fingerprint was not cleared") } +// askNow waits for the backup ask at key to be the one numbered asking and answers it. +func (b *bed) askNow(t *testing.T, key string, asking int) delivery.AssignerAsk { + t.Helper() + for deadline := time.Now().Add(15 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + if e, err := b.buckets[delivery.AssignerAsksBucket].Get(b.ctx, key); err == nil { + if a, err := delivery.DecodeAssignerAsk(key, e.Value()); err == nil && a.Asking == asking { + return a + } + } + } + t.Fatalf("the ask at %s never became asking %d", key, asking) + return delivery.AssignerAsk{} +} + // 1906/D4 (ADR 0539 §16), whole on the bus: a module held on a node because its backup failed is asked about, and the answer -// is written into the node's want. +// is written into the node's want; an answer "again" followed by another failure is asked again, and its answer is another want +// value, so the node retries; an old answer is never taken for a new revision at the same fingerprint. func TestABackupAnswerIsWrittenIntoTheNodesWant(t *testing.T) { b := startBed(t, "") fNew := delivery.SourcePrefix2 + strings.Repeat("b", 64) b.fixture(t, "web", fNew, `{"module":"web","version":"1"}`) - b.compose(t, "ace", "web", fNew, 'b', delivery.JudgedNeither) - b.putWant(t, "ace", "web", delivery.Want{FP: fNew, Composed: "sha256:" + strings.Repeat("b", 64)}) + first := b.compose(t, "ace", "web", fNew, 'b', delivery.JudgedNeither) + b.putWant(t, "ace", "web", delivery.Want{FP: fNew, Composed: first}) held, _ := delivery.EncodeHave(delivery.Have{FP: fNew, Phase: delivery.PhaseHeld, Health: delivery.HealthWaits, Detail: "no space left"}) b.putJSON(t, delivery.HaveBucket, "ace.web", held) b.start(t) akey, _ := delivery.BackupAskKey("ace", "web", fNew) - a, err := delivery.DecodeAssignerAsk(akey, b.get(t, delivery.AssignerAsksBucket, akey)) - if err != nil || !strings.Contains(a.Ask.Explanation, "no space left") { - t.Fatalf("the backup ask = %+v, %v", a, err) + a := b.askNow(t, akey, 0) + if !strings.Contains(a.Ask.Explanation, "no space left") || !strings.Contains(a.Ask.Explanation, first) { + t.Fatalf("the backup ask = %+v", a) } - b.answer(t, akey, delivery.BackupSkip) - w := b.wantNow(t, "ace", "web", "the answer in the want", func(w delivery.Want) bool { return w.Backup == delivery.BackupSkip }) - if w.FP != fNew || w.Composed != "sha256:"+strings.Repeat("b", 64) { - t.Errorf("the want = %+v", w) + // an answer to another ask is no answer to this one + b.answerTo(t, akey, delivery.BackupSkip, "someone-else") + time.Sleep(time.Second) + if w := b.wantNow(t, "ace", "web", "the want", func(delivery.Want) bool { return true }); w.Backup != "" { + t.Fatalf("an answer to another ask was taken: %+v", w) + } + b.answerTo(t, akey, delivery.BackupAgain, a.Ask.ID) + w1 := b.wantNow(t, "ace", "web", "the answer in the want", func(w delivery.Want) bool { return w.Backup == delivery.BackupAgain }) + if w1.FP != fNew || w1.Composed != first { + t.Errorf("the want = %+v", w1) + } + // the node retries and the backup fails again: a new have value, asked again under the next number + again, _ := delivery.EncodeHave(delivery.Have{FP: fNew, Phase: delivery.PhaseHeld, Health: delivery.HealthWaits, Detail: "no space left, again"}) + b.putJSON(t, delivery.HaveBucket, "ace.web", again) + a2 := b.askNow(t, akey, 2) + if a2.Ask.ID == a.Ask.ID { + t.Fatalf("asked again under the same id") + } + b.answerTo(t, akey, delivery.BackupAgain, a2.Ask.ID) + w2 := b.wantNow(t, "ace", "web", "the second answer another want value", func(w delivery.Want) bool { return w.Backup == delivery.BackupAgain && w.Words != w1.Words }) + if w2.Words == w1.Words { + t.Errorf("the second answer left the want as it was: %+v", w2) + } + // a new revision at the same fingerprint, whose backup fails as well, is asked, and the old answer is not taken for it + second := b.compose(t, "ace", "web", fNew, 'c', delivery.JudgedNeither) + b.putWant(t, "ace", "web", delivery.Want{FP: fNew, Composed: second}) + a3 := b.askNow(t, akey, 3) + if !strings.Contains(a3.Ask.Explanation, second) { + t.Errorf("the ask for the new revision = %+v", a3) } time.Sleep(time.Second) - if w := b.wantNow(t, "ace", "web", "the answer kept", func(delivery.Want) bool { return true }); w.Backup != delivery.BackupSkip { - t.Errorf("a later pass erased the answer: %+v", w) + if w := b.wantNow(t, "ace", "web", "the want", func(delivery.Want) bool { return true }); w.Backup != "" { + t.Errorf("the old answer was taken for the new revision: %+v", w) } } 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 e4b5910..54a6f27 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go @@ -213,7 +213,7 @@ func follow(ctx context.Context, kvs buckets, say func(asks.Condition) error, lo case <-settled: } } - p := &passer{kvs: kvs, w: w, logf: logger.Printf, say: say, said: map[string]bool{}} + p := &passer{kvs: kvs, w: w, logf: logger.Printf, say: say, said: map[string]bool{}, answeredAt: map[string]uint64{}} wake() for { select { 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 df54bcd..0b2ab35 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "strings" "time" "git.novox.be/novox/mesh-sdk/go/asks" @@ -24,6 +25,8 @@ type passer struct { // pass says a condition once and a restart says it again (the router keeps one per key). say func(asks.Condition) error said map[string]bool + // answeredAt is, by the id of a backup ask, how many values the have had when its answer was written into the want. + answeredAt map[string]uint64 } func (p *passer) pass(ctx context.Context) { @@ -276,9 +279,14 @@ func (p *passer) recordAsk(ctx context.Context, module, fp string, need assign.R } // backups sees to the modules held on a node because the backup before an apply failed (the have's phase held, ADR 0539 §16): -// it asks, once for each asking, whether to back up again or go on without a backup, raises the condition naming node, module and -// the backup's words, and writes the answer into the node's want for the module, which the node agent reads from it and from -// nothing else. A backup that fails again after the answer again is asked again, as the second asking. +// it asks whether to back up again or go on without a backup, raises the condition naming node, module and the backup's words, +// and writes the answer into the node's want for the module, which the node agent reads from it and from nothing else. +// +// The ask is bound to the composition (it names the digest, and a new revision at the same fingerprint is asked again under a +// new number) and an answer counts only if the router's warrant names the id of the ask now at the key (the warrant's first part, +// before the `@`). Each answer is another want value (the asking's number is in the want's words), so a backup that fails again +// after "back up again" and is answered "again" again changes the key and the node retries. That the backup failed again is seen +// in a new have value after the answer was written. func (p *passer) backups(ctx context.Context) { wanted := p.w.wantsOf() for key, value := range p.w.read(delivery.HaveBucket) { @@ -298,46 +306,54 @@ func (p *passer) backups(ctx context.Context) { if err != nil { continue } + current, there := delivery.DecodeAssignerAsk(akey, p.w.read(delivery.AssignerAsksBucket)[akey]) + if there != nil || assign.BackupAskDigest(current) != want.Composed { + after := -1 + if there == nil { + after = current.Asking + } + p.backupAsk(ctx, akey, node, module, want, have.Detail, assign.NextBackupAsking(after)) + continue + } answer, answered := p.w.answer(akey) - switch { - case answered && (answer.Option == delivery.BackupAgain || answer.Option == delivery.BackupSkip): - if want.Backup != answer.Option { - want.Backup = answer.Option - wkey, err := delivery.WantKey(node, module) - if err == nil { - w := want - p.put(ctx, delivery.AssignerWantsBucket, wkey, func() ([]byte, error) { return delivery.EncodeWant(w) }) - } - continue - } - if answer.Option == delivery.BackupAgain { - // the node tried again and the backup failed again: the second asking, once - p.backupAsk(ctx, akey, node, module, want.FP, have.Detail, 2) - } - case !answered: - p.backupAsk(ctx, akey, node, module, want.FP, have.Detail, 0) - c := assign.BackupCondition(node, module, want.FP, have.Detail, true) - if p.say != nil && !p.said[c.Key] { - if err := p.say(c); err != nil { - p.logf("the condition of the failed backup of %s on %s could not be said: %v", module, node, err) - } else { - p.said[c.Key] = true - } + if router, _, _ := strings.Cut(answer.Warrant, "@"); !answered || router != current.Ask.ID || + (answer.Option != delivery.BackupAgain && answer.Option != delivery.BackupSkip) { + continue // not answered yet: the ask stands + } + words := assign.BackupAnswerWords(answer.Option, current.Asking) + haveChanges := p.w.changed(delivery.HaveBucket, key) + if want.Backup != answer.Option || want.Words != words { + want.Backup, want.Words = answer.Option, words + if wkey, err := delivery.WantKey(node, module); err == nil { + w := want + p.put(ctx, delivery.AssignerWantsBucket, wkey, func() ([]byte, error) { return delivery.EncodeWant(w) }) + p.answeredAt[current.Ask.ID] = haveChanges } + continue + } + if answer.Option != delivery.BackupAgain { + continue + } + // The answer is in the want. The node retried and the backup failed again when the have took another value since. + if seen, known := p.answeredAt[current.Ask.ID]; !known { + p.answeredAt[current.Ask.ID] = haveChanges // after a restart: wait for the next failure + } else if haveChanges > seen { + p.backupAsk(ctx, akey, node, module, want, have.Detail, assign.NextBackupAsking(current.Asking)) } } } -// backupAsk writes the backup ask at its asking, unless that asking stands already. -func (p *passer) backupAsk(ctx context.Context, key, node, module, fp, words string, asking int) { - if p.w.has(delivery.AssignerAsksBucket, key) { - if asking == 0 { - return - } - if a, err := delivery.DecodeAssignerAsk(key, p.w.read(delivery.AssignerAsksBucket)[key]); err == nil && a.Ask.ID == delivery.AskingID(key, asking) { - return - } - } - a := assign.BackupAsk(key, node, module, fp, words, asking, time.Now()) +// backupAsk writes the backup ask at its asking and raises the condition of the failed backup, once. +func (p *passer) backupAsk(ctx context.Context, key, node, module string, want delivery.Want, words string, asking int) { + a := assign.BackupAsk(key, node, module, want.FP, want.Composed, words, asking, time.Now()) p.put(ctx, delivery.AssignerAsksBucket, key, func() ([]byte, error) { return delivery.EncodeAssignerAsk(key, a) }) + c := assign.BackupCondition(node, module, want.FP, words, true) + if p.say == nil || p.said[c.Key] { + return + } + if err := p.say(c); err != nil { + p.logf("the condition of the failed backup of %s on %s could not be said: %v", module, node, err) + return + } + p.said[c.Key] = true } 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 04ee003..ccbf75e 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/world.go @@ -44,6 +44,8 @@ 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 + // changes counts, by bucket and key, the values this process has seen the key take: a value written again is another. + changes map[string]uint64 } // parsed is a manifest read by digest: its module and its manifest, or why it cannot be read. @@ -69,6 +71,7 @@ func (w *world) put(bucket, key string, value []byte) { return } w.values[bucket][key] = value + w.changes[bucket+"|"+key]++ if bucket == delivery.HaveBucket { if h, err := delivery.DecodeHave(value); err == nil && h.FP != "" && h.Phase == delivery.PhaseRunning && h.Health == delivery.HealthOK { list := slices.DeleteFunc(w.healthy[key], func(fp string) bool { return fp == h.FP }) @@ -386,3 +389,10 @@ func (w *world) answer(key string) (delivery.Answer, bool) { a, err := delivery.DecodeAnswer(value) return a, err == nil } + +// changed is how many values the key has taken in this process (see world.changes). +func (w *world) changed(bucket, key string) uint64 { + w.mu.Lock() + defer w.mu.Unlock() + return w.changes[bucket+"|"+key] +} diff --git a/modules/mesh-delivery-assigner/internal/assign/asks.go b/modules/mesh-delivery-assigner/internal/assign/asks.go index 33e48ff..a3ca7ff 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks.go @@ -2,6 +2,7 @@ package assign import ( "fmt" + "regexp" "strings" "time" @@ -110,15 +111,42 @@ func RecordAsk(key, node, module, fp string, need RecordNeed, now time.Time) del }} } +// NextBackupAsking is the number of the asking that follows the one numbered after (a negative number: none yet). Numbers 0 and 1 +// give one id (delivery.AskingID), so the first asking is 0 and the next 2. +func NextBackupAsking(after int) int { + if after < 0 { + return 0 + } + return max(after+1, 2) +} + +var backupDigest = regexp.MustCompile(`\(composition (sha256:[0-9a-f]{64})\)`) + +// BackupAskDigest is the digest of the composition a backup ask was made about, from its explanation; empty for an ask that names +// none. An answer is taken for the composition of the ask it answers, and a new revision at the same fingerprint is asked again +// under a new number (ADR 0539 §16). +func BackupAskDigest(a delivery.AssignerAsk) string { + if m := backupDigest.FindStringSubmatch(a.Ask.Explanation); m != nil { + return m[1] + } + return "" +} + +// BackupAnswerWords are the words of a want carrying the answer to the asking-th ask: each asking's answer is another want +// value, so a backup that fails again after "back up again" and is answered "again" again changes the key and the node retries. +func BackupAnswerWords(option string, asking int) string { + return fmt.Sprintf("the operator answered %q to the backup ask (asking %d)", option, asking+1) +} + // BackupAsk is the ask about a backup of a module on node that failed before an apply (ADR 0539 §16): back up again, or go on -// without a backup. asking is the number of the asking (0 the first, 2 the second: delivery.AskingID), so a backup that fails again after the answer again is asked -// again. words are the backup's own words, from the node's have. -func BackupAsk(key, node, module, fp, words string, asking int, now time.Time) delivery.AssignerAsk { - return delivery.AssignerAsk{Ask: asks.Ask{ +// without a backup. digest is the composition about to be applied and asking the number of the asking, 0 the first; words are the +// backup's own words, from the node's have. +func BackupAsk(key, node, module, fp, digest, words string, asking int, now time.Time) delivery.AssignerAsk { + return delivery.AssignerAsk{Asking: asking, Ask: asks.Ask{ ID: delivery.AskingID(key, asking), Headline: fmt.Sprintf("The backup of %s failed on %s", module, node), - Explanation: fmt.Sprintf("The backup before applying %s at %s on %s failed: %s. %s keeps running what it has until you answer.", - module, short(fp), node, words, module), + Explanation: fmt.Sprintf("The backup before applying %s at %s on %s failed: %s. %s keeps running what it has until you answer. (composition %s)", + module, short(fp), node, words, module, digest), Options: []asks.Option{ {ID: OptionBackupAgain, Label: "Back up again", Does: "the node tries the backup again before it applies", Level: asks.Acknowledge}, {ID: OptionBackupSkip, Label: "Go on without a backup", Does: "the node applies the new version without a backup", Level: asks.Acknowledge}, diff --git a/modules/mesh-delivery-assigner/internal/assign/asks_test.go b/modules/mesh-delivery-assigner/internal/assign/asks_test.go index 0e2fd77..f734ad2 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks_test.go @@ -87,7 +87,8 @@ func TestTheRecordAndBackupAsksAreValid(t *testing.T) { if err != nil { t.Fatal(err) } - b := BackupAsk(bkey, "ace", "mail", fNew, "no space left", 0, now) + dgst := "sha256:" + strings.Repeat("b", 64) + b := BackupAsk(bkey, "ace", "mail", fNew, dgst, "no space left", 0, now) for key, a := range map[string]delivery.AssignerAsk{rkey: r, bkey: b} { if err := a.Ask.Check(now); err != nil { t.Errorf("%s: the router refuses the ask: %v", key, err) @@ -105,9 +106,19 @@ func TestTheRecordAndBackupAsksAreValid(t *testing.T) { if !strings.Contains(b.Ask.Explanation, "no space left") || b.Ask.Options[0].ID != delivery.BackupAgain || b.Ask.Options[1].ID != delivery.BackupSkip { t.Errorf("the backup ask = %+v", b.Ask) } - if again := BackupAsk(bkey, "ace", "mail", fNew, "x", 2, now); again.Ask.ID == b.Ask.ID { + // the id binds the ask to the composition and to its number (review round 1, items 3 and 4) + if again := BackupAsk(bkey, "ace", "mail", fNew, dgst, "x", NextBackupAsking(0), now); again.Ask.ID == b.Ask.ID { t.Errorf("asked again under the same id") } + if NextBackupAsking(-1) != 0 || NextBackupAsking(0) != 2 || NextBackupAsking(2) != 3 { + t.Errorf("the numbers of the askings: %d %d %d", NextBackupAsking(-1), NextBackupAsking(0), NextBackupAsking(2)) + } + if BackupAskDigest(b) != dgst { + t.Errorf("the ask names composition %q, want %q", BackupAskDigest(b), dgst) + } + if BackupAnswerWords("again", 0) == BackupAnswerWords("again", 1) { + t.Errorf("two answers of the same option are one want value") + } c := BackupCondition("ace", "mail", fNew, "no space left", true) if _, err := asks.EncodeCondition(c); err != nil { t.Errorf("the backup condition is refused: %v", err) diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index f95d46e..e5ffbe1 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -192,10 +192,10 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st d, _ := m.composition(node, m.FP) next = delivery.Want{FP: m.FP, Composed: d.Digest} if had && !before.Removed && before.FP == m.FP && before.Composed == d.Digest { - next.Backup = before.Backup // the answer to a failed backup stays while its target stays (ADR 0539 §16) + next.Backup, next.Words = before.Backup, before.Words // the answer to a failed backup stays while its target stays (ADR 0539 §16) } - _, ran := n.Haves[node][m.Name] - if need, wait := m.mustAsk(node, before, had, ran, d); wait { + running, ran := n.Haves[node][m.Name] + if need, wait := m.mustAsk(node, before, had, ran, running.FP, d); wait { if need != nil { out.Record = append(out.Record, *need) } @@ -261,11 +261,16 @@ func short(fp string) string { // for the operator's hand (ADR 0539 §14, §15). A module the node never ran (it has no want and no have), a move judged neither, a module that is not of // policy record and a want that stays are written as any want; so is a revision of the bus module's composition at the // same fingerprint judged neither (a user list only, §7). -func (m Module) mustAsk(node string, before delivery.Want, had, ran bool, d delivery.ComposedDigest) (*RecordNeed, bool) { +func (m Module) mustAsk(node string, before delivery.Want, had, ran bool, running string, d delivery.ComposedDigest) (*RecordNeed, bool) { if (!had && !ran) || before.Removed || (before.FP == m.FP && before.Composed == d.Digest) { return nil, false } - if catalogue.ProvidesBus(m.Manifest) && before.FP != m.FP { + // The fingerprint the bus ran before: its want's, or with no readable want the one its have names (ADR 0539 §15 bars a new one). + prev := before.FP + if !had { + prev = running + } + if catalogue.ProvidesBus(m.Manifest) && prev != m.FP { return nil, true // the bus is never moved by the assigner; the operator writes its want (§15) } if !m.recorded(d) || d.Judgement == delivery.JudgedNeither { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 0bed414..3678fff 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -338,8 +338,8 @@ func TestTheBusIsNotMovedButItsUserListRevisionIsWrittenAtOnce(t *testing.T) { // 1906/D4 (ADR 0539 §16): the answer to a failed backup stays in the want while its target stays. func TestTheBackupAnswerStaysInTheWantWhileItsTargetStays(t *testing.T) { m, n := ready() - running := map[string]delivery.Want{"ace": {FP: fNew, Composed: dig(fNew), Backup: delivery.BackupSkip}} - if w, ok := Wants(m, n, running, nil).Wants["ace"]; ok && w.Backup == "" { + running := map[string]delivery.Want{"ace": {FP: fNew, Composed: dig(fNew), Backup: delivery.BackupSkip, Words: BackupAnswerWords("skip", 0)}} + if w, ok := Wants(m, n, running, nil).Wants["ace"]; ok { t.Errorf("the answer was erased: %+v", w) } } @@ -402,3 +402,18 @@ func TestTheRecordPolicyIsTheOneTheControllerHolds(t *testing.T) { t.Errorf("a hand-made composition fell through the manifest's record policy: %+v", out) } } + +// Review round 1, item 2 (ADR 0539 §15): a node that runs the bus with no readable want compares the new fingerprint with the one +// it runs, from its have: the same fingerprint is not a new one (written by the rules), another is barred. +func TestTheBusWithNoWantIsComparedWithWhatItRuns(t *testing.T) { + m, n, _ := movingTo(delivery.JudgedNeither) + m.Manifest.Provides = []catalogue.Offer{{Name: "mesh-bus"}} + n.Haves["ace"]["mail"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + if w, ok := Wants(m, n, nil, nil).Wants["ace"]; !ok || w.FP != fNew { + t.Errorf("the bus runs F with no want and was not given one: %+v %v", w, ok) + } + n.Haves["ace"]["mail"] = delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + if w, ok := Wants(m, n, nil, nil).Wants["ace"]; ok { + t.Errorf("the bus was moved to a new fingerprint with no want: %+v", w) + } +} diff --git a/modules/mesh-delivery-view/internal/view/view.go b/modules/mesh-delivery-view/internal/view/view.go index 8e4b98f..8691018 100644 --- a/modules/mesh-delivery-view/internal/view/view.go +++ b/modules/mesh-delivery-view/internal/view/view.go @@ -391,7 +391,7 @@ func (m *Model) deliveredNow() *DeliveredEvent { func (m *Model) apply(e Entry) { switch e.Bucket { case delivery.HaveBucket: - node, module, isNode, err := delivery.ParseHaveKey(e.Key) + node, _, isNode, err := delivery.ParseHaveKey(e.Key) if err != nil { return } @@ -404,7 +404,7 @@ func (m *Model) apply(e Entry) { m.nodes[node] = nodeEntry{node: n, err: errText(err), at: e.At} return } - key := node + "." + module + key := e.Key // the bucket's own key: a module name with a dot is escaped in it (issue 1150) if e.Deleted { delete(m.haves, key) return @@ -480,7 +480,10 @@ func (m *Model) apply(e Entry) { if err != nil { return // a hold, and the node's own key, are not a module's want } - key := node + "." + module + key, kerr := delivery.HaveKey(node, module) + if kerr != nil { + return + } if e.Deleted { delete(m.wants, key) return @@ -599,7 +602,17 @@ func (m *Model) snapshot() Snapshot { r.RequiredBy = a.RequiredBy } var waits []string - h, has := m.haves[key] + hk, herr := delivery.HaveKey(a.Node, a.Module) + if herr != nil { + hk = key + } + h, has := m.haves[hk] + // The controller no longer writes the build of an assignment (R6): the row is the want and the have of the node. + w, wanted := m.wants[hk] + wanted = wanted && w.err == "" && !w.value.Removed && w.value.FP != "" + if wanted { + r.Assigned = w.value.FP + } switch { case !has: waits = append(waits, "no have yet") @@ -607,8 +620,15 @@ func (m *Model) snapshot() Snapshot { r.Since, r.Error = h.at, h.err default: r.Have, r.Phase, r.Health = h.have.Build, string(h.have.Phase), string(h.have.Health) + if r.Have == "" { + r.Have = h.have.FP + } r.Since, r.Detail = h.at, h.have.Detail - if a.Build != "" && h.have.Build != a.Build { + if wanted { + if h.have.FP != w.value.FP { + waits = append(waits, "the node has not applied "+w.value.FP) + } + } else if a.Build != "" && h.have.Build != a.Build { waits = append(waits, "the node has not applied "+a.Build) } } @@ -621,10 +641,13 @@ func (m *Model) snapshot() Snapshot { rows = append(rows, r) } for key, h := range m.haves { - if a, ok := m.assigns[key]; ok && a.open() { + node, module, _, perr := delivery.ParseHaveKey(key) + if perr != nil { + continue + } + if a, ok := m.assigns[node+"."+module]; ok && a.open() { continue } - node, module, _, _ := delivery.ParseHaveKey(key) r := Row{Node: node, Module: module, Status: StatusLeftBehind, Since: h.at, Error: h.err} if h.err == "" && h.have.Phase == delivery.PhaseRemoved { // The node-engine says it took the module away: plainly removed, not left behind. @@ -632,6 +655,9 @@ func (m *Model) snapshot() Snapshot { } if h.err == "" { r.Have, r.Phase, r.Health, r.Detail = h.have.Build, string(h.have.Phase), string(h.have.Health), h.have.Detail + if r.Have == "" { + r.Have = h.have.FP + } } rows = append(rows, r) } @@ -959,7 +985,7 @@ func (m *Model) wantRows() []WantRow { if w.err != "" { continue } - node, module, _ := strings.Cut(key, ".") + node, module := splitKey(key) v := w.value row := WantRow{Node: node, Module: module, Want: v.FP, Newest: v.Newest, Rule: v.Rule, Words: v.Words} h, has := m.haves[key] @@ -1011,7 +1037,7 @@ func (m *Model) sourceRow() *SourceRow { } looked := 0 for key, w := range m.wants { - node, module, _ := strings.Cut(key, ".") + node, module := splitKey(key) mod, ok := m.modules[module] if w.err != "" || w.value.Removed || !ok || mod.err != "" || mod.value.FP2 == "" { continue @@ -1033,7 +1059,11 @@ func (m *Model) sourceRow() *SourceRow { if !a.open() { continue } - if _, ok := m.wants[key]; ok { + wk, werr := delivery.HaveKey(a.Node, a.Module) + if werr != nil { + wk = key + } + if _, ok := m.wants[wk]; ok { continue } if mod, ok := m.modules[a.Module]; ok && mod.err == "" && mod.value.FP2 != "" { @@ -1047,3 +1077,12 @@ func (m *Model) sourceRow() *SourceRow { } return row } + +// splitKey is the node and the module (unescaped, issue 1150) of a model key, which is the have bucket's key form. +func splitKey(key string) (node, module string) { + node, module, isNode, err := delivery.ParseHaveKey(key) + if err != nil || isNode { + node, module, _ = strings.Cut(key, ".") + } + return node, module +} diff --git a/modules/mesh-delivery-view/internal/view/wants_test.go b/modules/mesh-delivery-view/internal/view/wants_test.go index 121626a..053b061 100644 --- a/modules/mesh-delivery-view/internal/view/wants_test.go +++ b/modules/mesh-delivery-view/internal/view/wants_test.go @@ -34,7 +34,11 @@ func haveFP(t *testing.T, node, module, fp string, health delivery.Health) Entry if err != nil { t.Fatal(err) } - return Entry{Bucket: delivery.HaveBucket, Key: node + "." + module, Value: b, At: t0} + key, err := delivery.HaveKey(node, module) + if err != nil { + t.Fatal(err) + } + return Entry{Bucket: delivery.HaveBucket, Key: key, Value: b, At: t0} } func moduleE(t *testing.T, module, fp string) Entry { @@ -190,3 +194,36 @@ func TestTheViewSaysOnceThatASourceRevisionIsDelivered(t *testing.T) { t.Errorf("said again after a have failed and recovered: %v", said) } } + +// Review round 1 (view notes): the controller no longer writes the build of an assignment (C6), so a row reads the want and the +// have: the row shows the fingerprint wanted and waits while the node runs another. +func TestARowReadsTheWantWhenTheAssignmentNamesNoBuild(t *testing.T) { + m := New() + fill(m, assign("ace", "web", "", ""), wantE(t, "ace", "web", delivery.Want{FP: fpNew}), haveFP(t, "ace", "web", fpOld, delivery.HealthOK)) + r := find(t, m.Snapshot(), "ace", "web") + if r.Assigned != fpNew || r.Have != fpOld || !strings.Contains(r.Waits, "has not applied") { + t.Fatalf("row = %+v, want it to wait for the want's fingerprint", r) + } + fill(m, haveFP(t, "ace", "web", fpNew, delivery.HealthOK)) + if r := find(t, m.Snapshot(), "ace", "web"); r.Waits != "" || r.Have != fpNew { + t.Errorf("row = %+v, want no wait once the have names the want's fingerprint", r) + } +} + +// Review round 1 (view notes, issue 1150): a module whose name holds a dot has its have under the escaped key; the row finds it, +// and a have nothing is assigned to keeps the module's name. +func TestADottedModuleNameIsFoundAndKept(t *testing.T) { + m := New() + fill(m, assign("ace", "novox.be", "", ""), wantE(t, "ace", "novox.be", delivery.Want{FP: fpNew}), haveFP(t, "ace", "novox.be", fpNew, delivery.HealthOK), + haveFP(t, "ace", "old.name", fpNew, delivery.HealthOK)) + s := m.Snapshot() + if r := find(t, s, "ace", "novox.be"); r.Have != fpNew || r.Waits != "" { + t.Errorf("the dotted module's row = %+v", r) + } + if r := find(t, s, "ace", "old.name"); r.Status != StatusLeftBehind { + t.Errorf("the left-behind dotted module = %+v", r) + } + if rows := s.Wants; len(rows) != 1 || rows[0].Module != "novox.be" || rows[0].State != WantApplied { + t.Errorf("wants = %+v", rows) + } +}