diff --git a/cmd/mesh-host/engineseat.go b/cmd/mesh-host/engineseat.go index a7fcbbe..13afd82 100644 --- a/cmd/mesh-host/engineseat.go +++ b/cmd/mesh-host/engineseat.go @@ -306,11 +306,18 @@ func composeFrom(current *agentNow, composer *atomic.Pointer[selfcompose.Compose return nil, errors.New("this node composes none of its own modules now (it applies the controller's " + "compositions, or its node agent is starting), so there is no pass to make") } - ag := current.get() + out := map[string]any{} if module != "" { - if ag == nil || !ag.Placed("")[module] { - return nil, fmt.Errorf("%s has no want on this machine, so there is no apply of it to try again", module) + // Only a failed apply that is not held is tried again; what runs is never taken away (review round 1: the + // verb has no warrant, ADR 0563 §2, and a hold for a failed backup is the operator's to release, ADR 0568 §4). + ag := current.get() + if ag == nil { + return nil, errors.New("this engine's node agent has not started, so no apply is tried again") } + if err := ag.RetryFailed(module); err != nil { + return nil, err + } + out["retried"] = module } ctx, cancel := context.WithTimeout(context.Background(), composeReadFor) defer cancel() @@ -318,11 +325,7 @@ func composeFrom(current *agentNow, composer *atomic.Pointer[selfcompose.Compose if err != nil { return nil, err } - out := map[string]any{"pass": pass} - if module != "" { - ag.Retry(module) - out["retried"] = module - } + out["pass"] = pass return out, nil } } diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 45bf7d8..7b59348 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -210,6 +210,8 @@ type Agent struct { retry chan []string // reconcile carries the judge's edges: a resource found not active (ADR 0568 §2). reconcile chan []string + // failedAgain carries the modules whose failed apply a person asked to try again (RetryFailed). + failedAgain chan string // tries counts the transient failures in a row of each module; again carries a retry whose wait ran out, and // transient asks for every transiently failed module at once. stopped is closed when Run returns. tries map[string]int @@ -256,7 +258,7 @@ func New(env Env) *Agent { } return &Agent{env: env, wants: map[string]wantNow{}, comps: map[string]compNow{}, running: map[string]delivery.Have{}, applied: map[string]compNow{}, had: map[string]compNow{}, keptData: map[string]bool{}, attempt: map[string]attempt{}, checked: map[string]bool{}, prior: map[string]bool{}, said: map[string]string{}, notComposed: map[string]notComposedMark{}, - retry: make(chan []string, 8), reconcile: make(chan []string, 64), tries: map[string]int{}, again: make(chan againAsk, 64), + retry: make(chan []string, 8), reconcile: make(chan []string, 64), failedAgain: make(chan string, 8), tries: map[string]int{}, again: make(chan againAsk, 64), transient: make(chan struct{}, 1), stopped: make(chan struct{})} } @@ -302,6 +304,15 @@ func (a *Agent) Run(ctx context.Context) error { dirty[m] = true } a.mu.Unlock() + case m := <-a.failedAgain: + // A failed apply a person asked to try again: only one still failed and not held (RetryFailed); what runs is + // left as it is. + a.mu.Lock() + if att, ok := a.attempt[m]; ok && !att.Held { + delete(a.attempt, m) + dirty[m] = true + } + a.mu.Unlock() case ask := <-a.again: // The wait of a transiently failed apply ran out: tried again from the keys it has, if nothing tried it since. a.mu.Lock() @@ -1150,6 +1161,34 @@ func (a *Agent) Placed(except string) map[string]bool { return out } +// RetryFailed asks a module's failed apply to be tried again (the node-engine's verb compose, issue 774): only an apply +// that failed and is not held for a failed backup, whose release is the operator's answer (ADR 0568 §4); what runs is +// never taken away, so nothing is applied that the engine would not apply on its own at the next change. Refused in +// words otherwise. +func (a *Agent) RetryFailed(module string) error { + a.mu.Lock() + w := a.wants[module] + att, tried := a.attempt[module] + r, ran := a.running[module] + a.mu.Unlock() + switch { + case !w.Present || w.Want.Removed: + return fmt.Errorf("%s has no want on this machine, so there is no apply of it to try again", module) + case tried && att.Held: + return fmt.Errorf("%s is held for a failed backup: the operator's answer releases it", module) + case !tried && ran && r.FP == w.Want.FP && r.Phase != delivery.PhaseRemoved: + return fmt.Errorf("%s runs what its want names, nothing to retry", module) + case !tried: + return fmt.Errorf("%s has no failed apply to try again: it waits for its composition, or settles on its own", module) + } + select { + case a.failedAgain <- module: + return nil + default: + return fmt.Errorf("%s's retry is not taken now: the engine has too many asked; ask again in a moment", module) + } +} + // RemovedByWant is the modules whose want says removed (apply.Scope.RemovedByWant, issue 774): in the node's own apply, // only their resources may go. Called without a.mu held. func (a *Agent) RemovedByWant() map[string]bool { diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 4423a9f..e2bb085 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -457,13 +457,27 @@ func TestAFailedApplyIsRetriedOnlyByAChangeOfEitherKey(t *testing.T) { if n := len(b.appliedList()); n != 1 { t.Fatalf("a failed apply was retried by an unrelated key or the clock (%d applies)", n) } - // The want is written again: the retry. + // A person asks it again (the node-engine's verb compose, issue 774): the failed apply is tried again with no key + // changed; once it runs, nothing is left to retry, and a module with no want has none. b.mu.Lock() b.failWith = "" b.mu.Unlock() - b.wants.put(wk, wraw) - eventually(t, "the change of the want did not retry", func() bool { return len(b.appliedList()) == 2 }) + if err := b.ag.RetryFailed("web"); err != nil { + t.Fatalf("a failed apply was refused a retry: %v", err) + } + eventually(t, "the asked retry did not apply", func() bool { return len(b.appliedList()) == 2 }) eventually(t, "not running", func() bool { return b.haves.last("web").Phase == string(delivery.PhaseRunning) }) + for module, words := range map[string]string{"web": "runs what its want names", "ghost": "has no want"} { + if err := b.ag.RetryFailed(module); err == nil || !strings.Contains(err.Error(), words) { + t.Errorf("a retry of %s: %v, want refused saying %q", module, err, words) + } + } + // The want is written again: no apply, it runs what the want names. + b.wants.put(wk, wraw) + time.Sleep(50 * time.Millisecond) + if n := len(b.appliedList()); n != 2 { + t.Errorf("a module running its want was applied again (%d applies)", n) + } }) // The bus did not answer for a moment (mesh-vault on the bus node, 2026-10-25 23:04:24): tried again with no new // revision of either key, by the backoff or by RetryTransient once the engine knows the bus is back; the have says @@ -540,6 +554,17 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) { if got := b.appliedList(); len(got) != 0 { t.Fatalf("something was applied behind a failed backup: %v", got) } + // A person's retry (the verb compose, issue 774) never releases it: the operator's answer does (ADR 0568 §4). + if err := b.ag.RetryFailed("db"); err == nil || !strings.Contains(err.Error(), "the operator's answer releases it") { + t.Errorf("a retry of db held for a failed backup: %v", err) + } + time.Sleep(50 * time.Millisecond) + b.mu.Lock() + asked := len(b.backups) + b.mu.Unlock() + if got := b.appliedList(); len(got) != 0 || asked != 1 { + t.Fatalf("a retry released db held for a failed backup: applied %v, %d backups asked", got, asked) + } // The answer arrives in the module's own want: back up again (it fails again), then skip. wk, again := wantFor(t, "db", fp2, dig2, delivery.BackupAgain) diff --git a/internal/selfcompose/selfcompose.go b/internal/selfcompose/selfcompose.go index 1040c67..aeb2730 100644 --- a/internal/selfcompose/selfcompose.go +++ b/internal/selfcompose/selfcompose.go @@ -135,6 +135,8 @@ type Composer struct { asked chan chan Pass // last is how the last pass went, under mu. last Pass + // watching says the composer's loop runs (Watch), so an ask of a pass can be answered. + watching atomic.Bool } // Pass is how one pass went, as the node-engine's verb compose answers it. @@ -150,8 +152,14 @@ type Pass struct { // Again is when the node composes again on its own, after a seat or a read that did not answer; zero when it does // not need to. Again time.Time `json:"again,omitempty"` + // Earlier says the answer is the last pass, made less than askedWithin ago, and no new one was made for the ask. + Earlier bool `json:"earlier,omitempty"` } +// askedWithin is how recent a pass answers an ask of the verb compose without a new one: the first wait after a seat +// that did not answer (laterAgain's first). +var askedWithin = 5 * time.Second + // passSaysEvery is how often a pass that runs on says, in the journal, how long it has run and what it waits on. var passSaysEvery = time.Minute @@ -174,6 +182,12 @@ func (c *Composer) paid() { c.owed.Store(false) } +// againLine is what a retry pass says as it starts: the try its refusal named. +func (c *Composer) againLine() string { + _, try := c.waitNow() + return fmt.Sprintf("the node composes again now (try %d), after a seat or a read that did not answer", try) +} + // waitNow is the bound before the pass owed, and its try. func (c *Composer) waitNow() (time.Duration, int) { c.mu.Lock() @@ -209,7 +223,20 @@ func New(env Env) *Composer { // ComposeNow asks the composer for a pass now (the node-engine's verb compose, issue 774): the growing bound after a seat // that did not answer starts again, and the answer is how the pass went. Refused in words when no pass is made before // ctx ends: the composer not running, or its inputs not all read yet. +// +// A pass ended less than askedWithin ago is the answer, and no new one is made, so a caller asking in a loop never +// drives passes back to back against the vault; an ask never restarts the growing bound either (review round 1). func (c *Composer) ComposeNow(ctx context.Context) (Pass, error) { + if !c.watching.Load() { + return Pass{}, errors.New("the node's composer runs no loop now: it composes nothing until its agent starts again") + } + c.mu.Lock() + last := c.last + c.mu.Unlock() + if !last.At.IsZero() && time.Since(last.At) < askedWithin { + last.Earlier = true + return last, nil + } reply := make(chan Pass, 1) select { case c.asked <- reply: @@ -333,8 +360,10 @@ func (c *Composer) Watch(ctx context.Context, _ string) (<-chan agent.Update, er }() } out := make(chan agent.Update, 64) + c.watching.Store(true) go func() { defer close(out) + defer c.watching.Store(false) caught := make([]bool, len(ins)) ready := false dirty := false @@ -350,14 +379,11 @@ func (c *Composer) Watch(ctx context.Context, _ string) (<-chan agent.Update, er case <-again: again = nil dirty = true - _, try := c.waitNow() - c.env.Say(fmt.Sprintf("the node composes again now (try %d), after a seat or a read that did not answer", try+1)) + c.env.Say(c.againLine()) case reply := <-c.asked: - // Asked by hand (the verb compose): composed now, the bound starting again. + // Asked by hand (the verb compose): composed now; the growing bound is not restarted, so a pass that + // fails again waits longer, not less. replies = append(replies, reply) - c.mu.Lock() - c.tries = 0 - c.mu.Unlock() again = nil dirty = true case t := <-merged: diff --git a/internal/selfcompose/selfcompose_test.go b/internal/selfcompose/selfcompose_test.go index c4fe830..01647d9 100644 --- a/internal/selfcompose/selfcompose_test.go +++ b/internal/selfcompose/selfcompose_test.go @@ -241,6 +241,9 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { b.mu.Unlock() // Issue 774, 2026-10-31: the verb compose makes a pass now and says how it went; a composer that runs no watch // makes none, and says so rather than waiting. + // The pass made at the start is less than askedWithin old for a moment: the ask waits that out first. + setFor(t, &askedWithin, 200*time.Millisecond) + time.Sleep(250 * time.Millisecond) madeBefore := composed.Load() cctx, ccancel := context.WithTimeout(ctx, 5*time.Second) nowPass, err := c.ComposeNow(cctx) @@ -248,9 +251,15 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { if err != nil || nowPass.At.IsZero() || nowPass.Refused != "" || !nowPass.Again.IsZero() || composed.Load() == madeBefore { t.Errorf("compose now: %+v (%v), %d pass(es) made; want one pass, composed", nowPass, err, composed.Load()-madeBefore) } - idle, icancel := context.WithTimeout(ctx, 20*time.Millisecond) - if _, err := New(env).ComposeNow(idle); err == nil { - t.Error("a composer that watches nothing answered compose now") + // Asked again at once, the last pass is the answer: a caller in a loop drives no passes back to back (review round 1). + madeBefore = composed.Load() + if again, err := c.ComposeNow(ctx); err != nil || !again.Earlier || !again.At.Equal(nowPass.At) || composed.Load() != madeBefore { + t.Errorf("compose now asked again at once: %+v (%v), %d pass(es) made; want the last pass, none made", again, err, composed.Load()-madeBefore) + } + // A composer whose loop runs not refuses at once, not after the verb's whole bound. + idle, icancel := context.WithTimeout(ctx, 5*time.Second) + if _, err := New(env).ComposeNow(idle); err == nil || !strings.Contains(err.Error(), "runs no loop") || idle.Err() != nil { + t.Errorf("a composer that watches nothing answered compose now: %v", err) } icancel() lemursKey, _ := delivery.ComposedKey("n1", "lemurs", fp) @@ -347,7 +356,9 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { t.Errorf("%s: a pass whose run ended refused %q", name, *refusals) } if !row.composes && !row.cancel && len(*refusals) > 0 { - says := strings.Contains((*refusals)[0], "; the node composes again at ") && strings.HasSuffix((*refusals)[0], "(try 1)") + // The retry pass says the same try the refusal named (review round 1: it said try 2 for try 1). + says := strings.Contains((*refusals)[0], "; the node composes again at ") && strings.HasSuffix((*refusals)[0], "(try 1)") && + strings.Contains(c2.againLine(), "(try 1)") if says != row.again || c2.owed.Load() != row.again || (c2.last.Again.IsZero() == row.again) || c2.last.Refused != (*refusals)[0] { t.Errorf("%s: refused %q, owes a pass %v, last pass %+v; want the time it composes again said and owed: %v", name, (*refusals)[0], c2.owed.Load(), c2.last, row.again) @@ -935,7 +946,8 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { // Review of L2b, round 1, finding 2: the candidates' state is not there when the node starts (a delivery before the // indexer made it): the shadow tries again, and answers once it appears. sb.missing(delivery.PlacementCandidatesBucket, true) - go sh.Keep(sctx, func(int) time.Duration { return 5 * time.Millisecond }) + kept := make(chan struct{}) + go func() { sh.Keep(sctx, func(int) time.Duration { return 5 * time.Millisecond }); close(kept) }() time.Sleep(30 * time.Millisecond) sb.missing(delivery.PlacementCandidatesBucket, false) sb.made(7) @@ -1270,6 +1282,9 @@ func TestTheNodeComposesItsWantsFromTheBus(t *testing.T) { t.Errorf("a shadow not ready said %q", nsaid) } nmu.Unlock() + // The first shadow ends before slowAfter is set: it reads it while it answers (a race the race detector names). + scancel() + <-kept setFor(t, &slowAfter, time.Nanosecond) // every answer is slow: said, and noted in words that do not move with the time nb.send(delivery.AssignerWantsBucket, agent.Update{Caught: true}) select {