Merge pull request 'A backup the node agent interrupted runs again, and a held module keeps the build it runs (issue 1289)' (#189) from fix/held-backup into main

This commit was merged in pull request #189.
This commit is contained in:
2026-10-31 16:33:54 +00:00
4 changed files with 106 additions and 4 deletions
+12
View File
@@ -9,6 +9,7 @@ import (
"testing" "testing"
"time" "time"
"github.com/novox/mesh-host/internal/agent"
"github.com/novox/mesh-host/internal/link" "github.com/novox/mesh-host/internal/link"
) )
@@ -43,6 +44,17 @@ func TestABackupEndsWhenItsEventSaysSo(t *testing.T) {
if done, words := run(50 * time.Millisecond); done || !strings.Contains(words, "did not say it was done") { if done, words := run(50 * time.Millisecond); done || !strings.Contains(words, "did not say it was done") {
t.Errorf("silence within the bound: done=%v words=%q", done, words) t.Errorf("silence within the bound: done=%v words=%q", done, words)
} }
// The node agent stopping while it waits is no failure of the backup: said as interrupted, so its next start backs
// the module up again (postgres on novox, 2026-10-31).
stopped, cancel := context.WithCancel(context.Background())
cancel()
if got := awaitBackup(stopped, make(chan []byte), "n1", "db", since, time.Second); got.Done || !got.Interrupted ||
got.Words != agent.BackupInterrupted {
t.Errorf("a stopped node agent: %+v", got)
}
if got := awaitBackup(context.Background(), make(chan []byte), "n1", "db", since, 10*time.Millisecond); got.Interrupted {
t.Errorf("silence within the bound taken as interrupted: %+v", got)
}
// The ask that starts it (sweep 2026-10-30): one that never reached the holder (nothing on this machine answers it // The ask that starts it (sweep 2026-10-30): one that never reached the holder (nothing on this machine answers it
// while the tool runner restarts) is asked again within its bound, and past it fails saying so; one that may have // while the tool runner restarts) is asked again within its bound, and past it fails saying so; one that may have
+1 -1
View File
@@ -222,7 +222,7 @@ func awaitBackup(ctx context.Context, events <-chan []byte, node, module string,
for { for {
select { select {
case <-ctx.Done(): case <-ctx.Done():
return agent.Backup{Words: "the node agent was stopped while the backup ran"} return agent.Backup{Words: agent.BackupInterrupted, Interrupted: true}
case <-bound.C: case <-bound.C:
return agent.Backup{Words: fmt.Sprintf("the backup did not say it was done within %s", within)} return agent.Backup{Words: fmt.Sprintf("the backup did not say it was done within %s", within)}
case raw := <-events: case raw := <-events:
+35 -3
View File
@@ -84,9 +84,15 @@ type Result struct {
} }
// Backup is the answer of the backup asked before a module moves. // Backup is the answer of the backup asked before a module moves.
// BackupInterrupted is the words of a backup the node agent itself stopped waiting for (its context ended): no failure, so
// the next start backs the module up again rather than holding it for the operator.
const BackupInterrupted = "the node agent was stopped while the backup ran"
type Backup struct { type Backup struct {
Done bool Done bool
Words string Words string
// Interrupted says the node agent stopped waiting for it (its context ended): not a failure of the backup.
Interrupted bool
} }
// Env is everything the agent needs of the node. // Env is everything the agent needs of the node.
@@ -1325,6 +1331,15 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com
a.mu.Lock() a.mu.Lock()
a.prior[module] = true a.prior[module] = true
a.mu.Unlock() a.mu.Unlock()
if h.Phase == string(delivery.PhaseHeld) && h.Detail == BackupInterrupted && w.Want.Backup != delivery.BackupSkip {
// Held only because this node agent stopped while the backup ran (postgres on novox, 2026-10-31): no
// failure and nothing for the operator to answer. It is backed up again and applied (a.prior makes the
// apply back it up first); the want still asking a backup is all the answer needed.
a.mu.Lock()
a.sayOnce("interrupted:"+module, fmt.Sprintf("%s's backup was interrupted by this node agent's stop: it is backed up again", module))
a.mu.Unlock()
return false
}
if h.Phase == string(delivery.PhaseHeld) { 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 // 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).
@@ -1414,16 +1429,33 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo
// A data module is backed up before a new composition of it moves (ADR 0429 §14), unless the want says to go on // A data module is backed up before a new composition of it moves (ADR 0429 §14), unless the want says to go on
// without one (ADR 0539 §16). // without one (ADR 0539 §16).
if comp.C.Data && ranBefore && a.env.Backup != nil && w.Want.Backup != delivery.BackupSkip { if comp.C.Data && ranBefore && a.env.Backup != nil && w.Want.Backup != delivery.BackupSkip {
// What runs, read before the have says backing-up: a module known only from its have (a.prior) has no running
// entry, and its held have kept naming nothing (postgres on novox, 2026-10-31), as the not-composed mark keeps it.
a.mu.Lock()
prev, runs := a.running[module]
a.mu.Unlock()
if !runs {
if h, found, _ := a.readHave(ctx, module); found {
prev = delivery.Have{FP: h.FP, Composed: h.Composed}
}
}
a.writeHave(ctx, Have{Module: module, FP: fp, Composed: dig, Phase: string(delivery.PhaseBackingUp)}) a.writeHave(ctx, Have{Module: module, FP: fp, Composed: dig, Phase: string(delivery.PhaseBackingUp)})
if got := a.env.Backup(ctx, module); !got.Done { if got := a.env.Backup(ctx, module); !got.Done {
a.env.Say(fmt.Sprintf("%s is held: its backup before %s failed: %s", module, fp, got.Words)) words := got.Words
if got.Interrupted || ctx.Err() != nil {
// The node agent stopped while it waited: no failure of the backup, so the next start backs it up again
// (baseline) rather than holding it for the operator.
words = BackupInterrupted
a.env.Say(fmt.Sprintf("%s waits: %s; it is backed up again at the next start", module, words))
} else {
a.env.Say(fmt.Sprintf("%s is held: its backup before %s failed: %s", module, fp, words))
}
a.mu.Lock() a.mu.Lock()
a.attempt[module] = attempt{FP: fp, Composed: dig, WantRev: w.Rev, CompRev: comp.Rev, Held: true, AnsweredRev: w.Rev} a.attempt[module] = attempt{FP: fp, Composed: dig, WantRev: w.Rev, CompRev: comp.Rev, Held: true, AnsweredRev: w.Rev}
prev := a.running[module]
a.mu.Unlock() a.mu.Unlock()
// It keeps running what it has: the have says what it holds and why it waits. // It keeps running what it has: the have says what it holds and why it waits.
a.writeHave(ctx, Have{Module: module, FP: prev.FP, Composed: prev.Composed, Phase: string(delivery.PhaseHeld), a.writeHave(ctx, Have{Module: module, FP: prev.FP, Composed: prev.Composed, Phase: string(delivery.PhaseHeld),
Detail: got.Words}) Detail: words})
return return
} }
} }
+58
View File
@@ -590,6 +590,59 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) {
h := b.haves.last("db") h := b.haves.last("db")
return h.Phase == string(delivery.PhaseRunning) && h.FP == fp2 && h.Composed == dig2 return h.Phase == string(delivery.PhaseRunning) && h.FP == fp2 && h.Composed == dig2
}) })
// The node agent stopped while the backup ran, the module known only from its have: the held have names what runs
// and says it was interrupted (postgres on novox, 2026-10-31, held naming nothing).
ib := newBed(t)
ib.comps.before(ck1, craw1)
ib.comps.before(ck2, craw2)
iwk, iwraw := wantFor(t, "db", fp2, dig2, delivery.BackupAgain)
ib.wants.before(iwk, iwraw)
ib.haves.stored = map[string]Have{"db": {Module: "db", FP: fp1, Composed: dig1, Phase: string(delivery.PhaseRunning)}}
began := make(chan struct{})
ib.env.Backup = func(ctx context.Context, _ string) Backup {
close(began)
<-ctx.Done()
return Backup{Words: "the backup's ask was cancelled"}
}
stopI := ib.run()
<-began
stopI()
if h := ib.haves.last("db"); h.Phase != string(delivery.PhaseHeld) || h.FP != fp1 || h.Composed != dig1 || h.Detail != BackupInterrupted {
t.Errorf("held after an interrupted backup: %+v", h)
}
// At a start: a backup the node agent's own stop interrupted (postgres on novox, 2026-10-31) is no failure, so it is
// backed up again and applied while the want asks a backup; a backup that failed stays held for the operator.
for _, row := range []struct {
detail, backup string
backups, applied int
phase delivery.Phase
}{
{BackupInterrupted, delivery.BackupAgain, 1, 1, delivery.PhaseRunning},
{BackupInterrupted, "", 1, 1, delivery.PhaseRunning},
{"restic: the repository is locked", delivery.BackupAgain, 0, 0, delivery.PhaseHeld},
} {
r := newBed(t)
r.comps.before(ck1, craw1)
r.comps.before(ck2, craw2)
wk, wraw := wantFor(t, "db", fp2, dig2, row.backup)
r.wants.before(wk, wraw)
r.haves.stored = map[string]Have{"db": {Module: "db", FP: fp1, Composed: dig1, Phase: string(delivery.PhaseHeld), Detail: row.detail}}
stopR := r.run()
if row.applied > 0 {
eventually(t, row.detail+": not applied after its backup", func() bool { return len(r.appliedList()) == row.applied })
}
time.Sleep(50 * time.Millisecond)
r.mu.Lock()
n := len(r.backups)
r.mu.Unlock()
h := r.haves.last("db")
if n != row.backups || len(r.appliedList()) != row.applied || h.Phase != string(row.phase) {
t.Errorf("%s, want backup %q: %d backups, applied %v, have %+v", row.detail, row.backup, n, r.appliedList(), h)
}
stopR()
}
} }
// A data module with a good backup is applied after it; one that never ran has nothing to back up. // A data module with a good backup is applied after it; one that never ran has nothing to back up.
@@ -1640,6 +1693,11 @@ func testReconcileAppliesARunningModuleAgainAndLeavesTheHeldAndTheUnwantedAlone(
if got := b.appliedList(); len(got) != 0 { if got := b.appliedList(); len(got) != 0 {
t.Fatalf("applied at the start: %v", got) t.Fatalf("applied at the start: %v", got)
} }
// Held, it keeps naming what runs, which this agent knows only from the have (postgres on novox, 2026-10-31): not
// the build it was to move to, and never nothing.
if h := b.haves.last("db"); h.FP != fpOf("0") || h.Composed != "sha256:"+strings.Repeat("0", 64) {
t.Fatalf("the held have does not name what runs: %+v", h)
}
b.ag.Reconcile("db", "ghost") b.ag.Reconcile("db", "ghost")
time.Sleep(50 * time.Millisecond) time.Sleep(50 * time.Millisecond)