Merge pull request 'Node-engine: act on a backup answer whatever runs, say why a module stays held, and show the want and the hold in the composition verb (issue 1585)' (#208) from fix/1585-held-answer into main
This commit was merged in pull request #208.
This commit is contained in:
+27
-8
@@ -1305,20 +1305,35 @@ func (a *Agent) settleOne(ctx context.Context, module string) {
|
||||
a.mu.Unlock()
|
||||
}
|
||||
a.mu.Lock()
|
||||
if r, ok := a.running[module]; ok && r.FP == w.Want.FP && r.Composed == w.Want.Composed && r.Phase != delivery.PhaseRemoved {
|
||||
r, runs := a.running[module]
|
||||
runsWant := runs && r.FP == w.Want.FP && r.Composed == w.Want.Composed && r.Phase != delivery.PhaseRemoved
|
||||
att, tried := a.attempt[module]
|
||||
same := tried && att.FP == w.Want.FP && att.Composed == w.Want.Composed
|
||||
if same && att.Held {
|
||||
// Held for a failed backup: looked at before what runs, since a hold may name what the want names (its have kept
|
||||
// the composition it held, nats on novox, 2026-11-04), and an answer in a later revision of the want would then
|
||||
// be swallowed by "it runs what its want names" (issue 1585, the second failure). The operator's answer, again or
|
||||
// skip, in a revision after the one the hold took, is acted on whatever runs; any other new revision of the want
|
||||
// is the answer only for a move (ADR 0568 §4), as before.
|
||||
answered := w.Rev > att.AnsweredRev && (w.Want.Backup == delivery.BackupAgain || w.Want.Backup == delivery.BackupSkip)
|
||||
if !answered && (runsWant || (att.AnsweredRev >= w.Rev && att.CompRev == comp.Rev)) {
|
||||
a.sayOnce("held:"+module, fmt.Sprintf("%s is held for a failed backup before %s: it waits for the operator's "+
|
||||
"answer, and its want (revision %d) carries none since the hold (revision %d)", module, w.Want.FP, w.Rev, att.AnsweredRev))
|
||||
a.mu.Unlock()
|
||||
return
|
||||
}
|
||||
if answered {
|
||||
a.sayOnce("answered:"+module+":"+fmt.Sprint(w.Rev), fmt.Sprintf("%s is held for a failed backup, and "+
|
||||
"its want (revision %d) carries the operator's answer %q: acted on now", module, w.Rev, w.Want.Backup))
|
||||
}
|
||||
} else if runsWant {
|
||||
a.mu.Unlock()
|
||||
return
|
||||
}
|
||||
att, tried := a.attempt[module]
|
||||
same := tried && att.FP == w.Want.FP && att.Composed == w.Want.Composed
|
||||
if same && !att.Held && att.WantRev == w.Rev && att.CompRev == comp.Rev {
|
||||
a.mu.Unlock()
|
||||
return // failed already for exactly these keys: retried by the next change of either
|
||||
}
|
||||
if same && att.Held && att.AnsweredRev >= w.Rev && att.CompRev == comp.Rev {
|
||||
a.mu.Unlock()
|
||||
return // held for a failed backup, and no answer since
|
||||
}
|
||||
if module == NodeModule {
|
||||
// Not while a module's composition cannot be read or is refused: what it carries may be what left the node's own,
|
||||
// and the node's own apply would remove it, held out by nothing (issue 1945, the review's gap 2).
|
||||
@@ -1390,8 +1405,12 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com
|
||||
}
|
||||
if h.Phase == string(delivery.PhaseHeld) {
|
||||
// Held for a failed backup, and the operator's answer is still to come: the hold is kept, nothing is
|
||||
// backed up again or applied, and the next revision of the want is the answer (ADR 0568 §4).
|
||||
// backed up again or applied, and the next revision of the want is the answer (ADR 0568 §4). Said, so
|
||||
// why it holds is read from the journal and not guessed (issue 1585: this branch was silent).
|
||||
a.mu.Lock()
|
||||
a.sayOnce("held:"+module, fmt.Sprintf("%s is held for a failed backup (%s) at this start: it keeps what "+
|
||||
"its have names, %s, and waits for the operator's answer; its want (revision %d) carries none",
|
||||
module, h.Detail, short(h.Composed), w.Rev))
|
||||
a.attempt[module] = attempt{FP: w.Want.FP, Composed: w.Want.Composed, WantRev: w.Rev, CompRev: comp.Rev, Held: true,
|
||||
AnsweredRev: w.Rev}
|
||||
if h.Composed != "" {
|
||||
|
||||
@@ -636,6 +636,78 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) {
|
||||
}
|
||||
stopR()
|
||||
}
|
||||
|
||||
// The live shape (nats on novox, 2026-11-04, issue 1585, the second failure): a module the node composes itself, of
|
||||
// policy record, its want Self with the yes for its composition, and its held have naming that very composition (what
|
||||
// runs is what the want names). The operator's answer acted on at the start, and in a revision of the want that
|
||||
// arrives while the module is held; the engine's read tool says the want's answer and the hold.
|
||||
for _, row := range []struct {
|
||||
name string
|
||||
atStart string // the answer the want carries when the engine starts
|
||||
later string // the answer a later revision carries; "" writes none
|
||||
backups int
|
||||
holdsTill bool // it is held, and said held, before the later revision
|
||||
}{
|
||||
{"answered before the start", delivery.BackupAgain, "", 1, false},
|
||||
{"answered while held", "", delivery.BackupAgain, 1, true},
|
||||
{"skipped while held", "", delivery.BackupSkip, 0, true},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
r := newBed(t)
|
||||
r.env.SelfComposed, r.kept = true, nil
|
||||
r.env.Policy = func(context.Context, string) string { return catalogue.PolicyRecord }
|
||||
ck, craw, dig := composition(t, "nats", fp2, true, file("nats", "x", "2"))
|
||||
r.comps.before(ck, craw)
|
||||
wk, _ := delivery.WantKey(node, "nats")
|
||||
selfWant := func(backup string) []byte {
|
||||
raw, err := delivery.EncodeWant(delivery.Want{FP: fp2, Self: true, Yes: dig, Backup: backup})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return raw
|
||||
}
|
||||
r.wants.before(wk, selfWant(row.atStart))
|
||||
r.haves.stored = map[string]Have{"nats": {Module: "nats", FP: fp2, Composed: dig, Phase: string(delivery.PhaseHeld),
|
||||
Detail: "the backup failed: sh: exit status 1"}}
|
||||
stopR := r.run()
|
||||
defer stopR()
|
||||
if row.holdsTill {
|
||||
eventually(t, "the hold at the start is not said", func() bool {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
for _, s := range r.said {
|
||||
if strings.Contains(s, "nats is held for a failed backup") && strings.Contains(s, "carries none") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
})
|
||||
view, err := r.ag.Composition(context.Background(), "nats", "", false)
|
||||
if err != nil || view.Attempt == nil || !view.Attempt.Held || view.Want == nil || view.Want.Backup != "" || !view.Started {
|
||||
t.Fatalf("the read tool does not say the hold: %+v %+v %v", view.Attempt, view.Want, err)
|
||||
}
|
||||
if got := r.appliedList(); len(got) != 0 {
|
||||
t.Fatalf("applied behind a hold with no answer: %v", got)
|
||||
}
|
||||
r.wants.put(wk, selfWant(row.later))
|
||||
eventually(t, "the read tool does not say the answer", func() bool {
|
||||
v, err := r.ag.Composition(context.Background(), "nats", "", false)
|
||||
return err == nil && v.Want != nil && v.Want.Backup == row.later && v.Want.Revision > view.Want.Revision
|
||||
})
|
||||
}
|
||||
eventually(t, "the answer was not acted on", func() bool { return len(r.appliedList()) == 1 })
|
||||
eventually(t, "not running after the answer", func() bool {
|
||||
h := r.haves.last("nats")
|
||||
return h.Phase == string(delivery.PhaseRunning) && h.Composed == dig
|
||||
})
|
||||
r.mu.Lock()
|
||||
n := len(r.backups)
|
||||
r.mu.Unlock()
|
||||
if n != row.backups {
|
||||
t.Errorf("%d backups for the answer, want %d", n, row.backups)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A data module with a good backup is applied after it; one that never ran has nothing to back up.
|
||||
|
||||
@@ -21,6 +21,13 @@ type CompositionView struct {
|
||||
Module string `json:"module"`
|
||||
// Want is the module's want as the agent holds it; nil when it holds none.
|
||||
Want *WantView `json:"want,omitempty"`
|
||||
// Attempt is what the agent holds of the module's last apply that did not end running: failed, failed for a moment
|
||||
// of the bus, or held for a failed backup, with the want's revision that hold took as answered; nil when none
|
||||
// (issue 1585: why a module stays held was not readable from the engine).
|
||||
Attempt *AttemptView `json:"attempt,omitempty"`
|
||||
// Started says the agent has looked at the module once since it started (its baseline): what runs was taken from its
|
||||
// have, or it was applied.
|
||||
Started bool `json:"started"`
|
||||
// Have is what the module's have key says now; nil when it says nothing.
|
||||
Have *HaveView `json:"have,omitempty"`
|
||||
// Composition is the revision read: the latest of the want's build, or the one whose digest was asked.
|
||||
@@ -55,12 +62,34 @@ type OwnView struct {
|
||||
Held map[string]string `json:"held,omitempty"`
|
||||
}
|
||||
|
||||
// WantView is a want as the verb shows it.
|
||||
// WantView is a want as the verb shows it: as the agent holds it, at the revision of its key it last read.
|
||||
type WantView struct {
|
||||
FP string `json:"fp"`
|
||||
Composed string `json:"composed,omitempty"`
|
||||
Yes string `json:"yes,omitempty"`
|
||||
Removed bool `json:"removed,omitempty"`
|
||||
// Self says the node composes the module itself (ADR 0547 §2).
|
||||
Self bool `json:"self,omitempty"`
|
||||
// Backup is the operator's answer to a failed backup the want carries, again or skip (ADR 0539 §16); empty when none.
|
||||
Backup string `json:"backup,omitempty"`
|
||||
Words string `json:"words,omitempty"`
|
||||
// Revision is the revision of the want's key the agent read last.
|
||||
Revision uint64 `json:"revision"`
|
||||
}
|
||||
|
||||
// AttemptView is the agent's record of a module's last apply that did not end running.
|
||||
type AttemptView struct {
|
||||
FP string `json:"fp"`
|
||||
Composed string `json:"composed,omitempty"`
|
||||
// WantRevision and CompositionRevision are the revisions of the want's and the composition's keys it was tried at.
|
||||
WantRevision uint64 `json:"want_revision"`
|
||||
CompositionRevision uint64 `json:"composition_revision"`
|
||||
// Held says it is held for a failed backup; AnsweredRevision is the want's revision the hold took, so an answer
|
||||
// (again or skip) in a later revision releases it.
|
||||
Held bool `json:"held,omitempty"`
|
||||
AnsweredRevision uint64 `json:"answered_revision,omitempty"`
|
||||
// Transient says it failed because the bus did not answer for a moment, and is tried again by the clock.
|
||||
Transient bool `json:"transient,omitempty"`
|
||||
}
|
||||
|
||||
// HaveView is a have as the verb shows it.
|
||||
@@ -127,10 +156,16 @@ func (a *Agent) Composition(ctx context.Context, module, digest string, diff boo
|
||||
fp := ""
|
||||
if w.Present {
|
||||
fp = w.Want.FP
|
||||
view.Want = &WantView{FP: w.Want.FP, Composed: w.Want.Composed, Yes: w.Want.Yes, Removed: w.Want.Removed}
|
||||
view.Want = &WantView{FP: w.Want.FP, Composed: w.Want.Composed, Yes: w.Want.Yes, Removed: w.Want.Removed,
|
||||
Self: w.Want.Self, Backup: w.Want.Backup, Words: w.Want.Words, Revision: w.Rev}
|
||||
} else if have != nil {
|
||||
fp = have.FP
|
||||
}
|
||||
if att, ok := a.attempt[module]; ok {
|
||||
view.Attempt = &AttemptView{FP: att.FP, Composed: att.Composed, WantRevision: att.WantRev, CompositionRevision: att.CompRev,
|
||||
Held: att.Held, AnsweredRevision: att.AnsweredRev, Transient: att.Transient}
|
||||
}
|
||||
view.Started = a.checked[module]
|
||||
latest, haveLatest := a.compositionFor(module, fp)
|
||||
r, ran := a.running[module]
|
||||
from, known := a.runningComposition(module)
|
||||
|
||||
Reference in New Issue
Block a user