Retry only a failed apply that is not held, answer a repeated compose from the last pass, and say the try the refusal named (issue 774, review round 1)
mesh/delivery merged: source.main moved to novox/mesh-host@3d1465606bed
mesh/delivery-group group fix/compose-retries-transient merged: every member is merged
mesh/check being carried to b0d6684d3731 on 91fb323ce0cf | from mesh-sdk dec3bfe7a8cb | novox/mesh-host#185 in group fix/compose-retries-transient
mesh/delivery merged: source.main moved to novox/mesh-host@3d1465606bed
mesh/delivery-group group fix/compose-retries-transient merged: every member is merged
mesh/check being carried to b0d6684d3731 on 91fb323ce0cf | from mesh-sdk dec3bfe7a8cb | novox/mesh-host#185 in group fix/compose-retries-transient
The verb compose has no warrant (ADR 0563 §2): Agent.Retry released a module held for a failed backup and re-applied one already running its want. RetryFailed tries again only a failed apply not held, and refuses in words otherwise. A hand ask no longer restarts the growing bound, and a pass less than askedWithin old is the answer, so a caller in a loop drives no passes against the vault. A composer whose loop ended refuses at once. The retry line said try N+1. The test's first shadow now ends before slowAfter is set, a race the race detector found under the new timing.
This commit is contained in:
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
+40
-1
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user