Hold a delivering delivery for a failed backup before the send; release asks the backups again (issue 1068)

This commit is contained in:
2026-10-17 22:45:03 +02:00
parent 914099774e
commit 8f7aeeeb81
12 changed files with 305 additions and 4 deletions
+10
View File
@@ -7,3 +7,13 @@ The `mesh-delivery` module of the mesh catalogue: claims the seat `mesh-delivery
Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416).
- `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
## A backup that failed before the send
The controller backs up the data of the modules a delivery moves before it asks or sends anything (ADR 0404). When its
walk's record, in `plan-moved`, names a backup that failed or stayed silent and nothing of the plan was sent, the
delivery goes from delivering to held (the observed row `backup failed`); the held reason names the machine, the
module and the backup's words, and the pull request's comment shows it. The hold does not lift by itself. `release`
of such a delivery asks the controller (`delivery-backups-again`) to take the backups again and the delivery goes on
delivering; a record from before the controller cleared the failure does not hold it again. A held delivery already
ends as covered or delivered when a later merge covers it.
@@ -0,0 +1,126 @@
package main
import (
"strings"
"testing"
)
// A failed backup before the send holds the delivery, and a release asks the backups again (novox/hq ADR 0404
// clause 4, issue 1068). The walk's field names are provisional until the controller's contract settles them.
func failedBackup() *WalkBackups {
return &WalkBackups{Failed: []WalkBackupFailure{{Node: "ace", Module: "pg", Error: "no space left on device"}}}
}
// aDeliveringWalk is a delivery whose walk was let go and runs, nothing sent yet.
func aDeliveringWalk(t *testing.T) (*world, string, string) {
t.Helper()
w := newWorld(t)
id := IDOf("novox/app", head)
w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go"))
w.h.Checked(verdict("novox/app", 7, head, "pass"))
w.h.PullMerged(merged("novox/app", 7, head, merge))
w.h.WalkMoved(w.ctl.put(aWalk("plan-1", "novox/app", merge, true)))
w.settleAll()
if s := w.state(id); s != Delivering {
t.Fatalf("setup: %s, want delivering", s)
}
return w, id, "plan-1"
}
func TestABackupFailedTakesADeliveringDeliveryToHeldOnlyWhileNothingWasSent(t *testing.T) {
w, id, plan := aDeliveringWalk(t)
sent := w.ctl.walk(plan)
sent.Backups = failedBackup()
sent.Modules["app"].SentAt = &w.now
w.h.WalkMoved(w.ctl.put(sent))
w.settleAll()
if s := w.state(id); s == Held {
t.Fatal("held for a failed backup after a module of the plan was sent")
}
w, id, plan = aDeliveringWalk(t)
held := w.ctl.walk(plan)
held.Backups = failedBackup()
w.h.WalkMoved(w.ctl.put(held))
w.settleAll()
d := w.delivery(id)
if d.State != Held {
t.Fatalf("with a failed backup and nothing sent it is %s", d.State)
}
for _, want := range []string{"ace", "pg", "no space left on device"} {
if !strings.Contains(d.HeldWhy, want) {
t.Errorf("the held reason %q does not name %q", d.HeldWhy, want)
}
}
// It does not lift by itself: the same facts read again leave it held.
w.h.WalkMoved(w.ctl.put(w.ctl.walk(plan)))
w.settleAll()
if s := w.state(id); s != Held {
t.Fatalf("read again it is %s, want it still held", s)
}
}
func TestAReleaseOfADeliveryHeldForAFailedBackupAsksTheControllerAgainAndGoesToDelivering(t *testing.T) {
w, id, plan := aDeliveringWalk(t)
held := w.ctl.walk(plan)
held.Backups = failedBackup()
w.h.WalkMoved(w.ctl.put(held))
w.settleAll()
if s := w.state(id); s != Held {
t.Fatalf("setup: %s, want held", s)
}
lettingGo := len(w.ctl.delivers)
if _, err := w.h.Release(id, "freed some space", "jochen"); err != nil {
t.Fatal(err)
}
if got := w.ctl.backupsAgain; len(got) != 1 || got[0] != plan {
t.Fatalf("the controller was asked to take the backups again %v, want once for %s", got, plan)
}
if len(w.ctl.delivers) != lettingGo {
t.Errorf("a started walk was let go again: %v", w.ctl.delivers)
}
if s := w.state(id); s != Delivering {
t.Fatalf("released, it is %s", s)
}
// The record the controller cleared is read: it stays delivering.
w.h.WalkMoved(w.ctl.walk(plan))
w.settleAll()
if s := w.state(id); s != Delivering {
t.Fatalf("with the backups cleared it is %s", s)
}
// Another failure, in a later record, holds it again.
again := w.ctl.walk(plan)
again.Backups = failedBackup()
w.h.WalkMoved(w.ctl.put(again))
w.settleAll()
if s := w.state(id); s != Held {
t.Fatalf("a failed backup in a later record left it %s", s)
}
}
func TestARecordThatStillHoldsTheFailureAfterAReleaseDoesNotHoldItAgain(t *testing.T) {
w, id, plan := aDeliveringWalk(t)
held := w.ctl.walk(plan)
held.Backups = failedBackup()
stale := w.ctl.put(held)
w.h.WalkMoved(stale)
w.settleAll()
if _, err := w.h.Release(id, "freed some space", "jochen"); err != nil {
t.Fatal(err)
}
w.h.WalkMoved(stale) // the record as it was before the controller cleared it
w.settleAll()
if s := w.state(id); s != Delivering {
t.Fatalf("a record from before the release held it again: %s", s)
}
}
func TestADeliveryHeldForAFailedBackupIsNotLetGoByItsWalkAlone(t *testing.T) {
// The controller's own path starts a walk with nobody's word: the held-to-delivering row for that must not
// undo the backup hold.
d := &Delivery{ID: "novox/app@c0", State: Held, MergedAs: "aaaa", Walk: &WalkSeen{ID: "plan-1", State: walkBuilding, Revision: 3,
BackupsFailed: failedBackup().Failed}}
if _, err := Apply(d, EvGo, false, Facts{}); err == nil {
t.Fatal("a walk started on the controller's own path lifted the backup hold")
}
}
@@ -562,3 +562,62 @@ func TestAGroupOfTwoWithGreenChecksGetsOneAskAndOnItsApproveBothMergeInOnePassOv
t.Fatalf("a second ask was made: %d", again)
}
}
// **The acceptance of a failed backup before the send, on the mesh-delivery side** (novox/hq ADR 0404 clause 4, design 47
// milestone 4, issue 1068): a merged delivery whose walk runs is held when the walk's record, as the controller's
// `plan-moved` carries it, names a backup that failed and nothing was sent; the hold names the machine, the module and the
// backup's words, and nothing asks the controller to let it go. One test function, one bus. The walk's backups field names
// are provisional until the controller's contract (issue 1065) settles them.
func TestAPlanMovedCarryingAFailedBackupHoldsTheDeliveryWithTheBackupsWordsOverARealBus(t *testing.T) {
b := newTestBus(t)
rt := b.start(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
rt.deliverEvents(ctx)
rt.until("the module to subscribe to the events it follows", 20*time.Second, func() bool {
rt.mu.Lock()
defer rt.mu.Unlock()
return len(rt.subs) >= len(followed)+1
})
const head, merge = "3333333333333333333333333333333333333333", "4444444444444444444444444444444444444444"
announced := pr("novox/app", 9, head, "fix/y", "src/a.go")
announced.BaseSHA = "t1t1t1t1t1t1"
b.publish(t, "mesh.mod.gitea.event.pull.updated", announced)
b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/app", 9, head, "pass", "t1t1t1t1t1t1"))
b.publish(t, "mesh.mod.gitea.event.pull.merged", merged("novox/app", 9, head, merge))
// Its walk runs, let go: the record names no failed backup yet.
now := time.Now()
walk := aWalk("plan-1", "novox/app", merge, true)
walk.Created, walk.State, walk.Revision = now, walkRolling, 1
walk.Delivery.Go, walk.Delivery.By = &now, "mesh-delivery"
b.publish(t, "mesh.seat."+ControllerSeat+".event.plan-moved", walk)
deliveryOf := func() Delivery {
e, err := b.kvs["deliveries"].Get(context.Background(), kvKey(IDOf("novox/app", head)))
var d Delivery
if err == nil {
_ = json.Unmarshal(e.Value(), &d)
}
return d
}
rt.until("the delivery to be delivering", 10*time.Second, func() bool { return deliveryOf().State == Delivering })
// A backup failed before the send: the delivery is held with the backup's words.
walk.Revision, walk.Backups = 2, &WalkBackups{Failed: []WalkBackupFailure{{Node: "ace", Module: "pg", Error: "no space left on device"}}}
b.publish(t, "mesh.seat."+ControllerSeat+".event.plan-moved", walk)
rt.until("the delivery to be held for the failed backup", 10*time.Second, func() bool { return deliveryOf().State == Held })
d := deliveryOf()
for _, want := range []string{"ace", "pg", "no space left on device"} {
if !strings.Contains(d.HeldWhy, want) {
t.Fatalf("the held reason %q does not name %q", d.HeldWhy, want)
}
}
// It stays held, and the controller is asked nothing to let it go.
time.Sleep(500 * time.Millisecond)
if s := deliveryOf().State; s != Held {
t.Fatalf("the hold lifted by itself: %s", s)
}
if n := len(rt.asksOf("seat:"+ControllerSeat+".deliver")) + len(rt.asksOf("seat:"+ControllerSeat+".delivery-backups-again")); n != 0 {
t.Fatalf("the controller was asked %d times to let the held walk go or take its backups again", n)
}
}
@@ -92,7 +92,9 @@ type fakeController struct {
checks []string
asked int
delivers []string
stops []string
// backupsAgain are the walks whose failed backups were asked again.
backupsAgain []string
stops []string
// withouts are the stops that left merges out (novox/hq ADR 0299): "<walk> without <merge>,…".
withouts []string
// refuseWithout refuses every stop with merges to leave out, as a controller before ADR 0299 does; mergedAt are
@@ -121,6 +123,24 @@ type fakeController struct {
forgot []string
}
// BackupsAgain records the word, and clears the walk's failed backups as the controller does.
func (c *fakeController) BackupsAgain(walk, why string) error {
c.mu.Lock()
defer c.mu.Unlock()
if c.down {
return errors.New("down")
}
w, known := c.walks[walk]
if !known || w.Backups == nil || len(w.Backups.Failed) == 0 {
return fmt.Errorf("%s holds no failed backup", walk)
}
w.Backups = nil
w.Revision++
c.walks[walk] = w
c.backupsAgain = append(c.backupsAgain, walk)
return nil
}
func (c *fakeController) ForgetChecks(owner, repo string, number int) error {
c.mu.Lock()
defer c.mu.Unlock()
@@ -329,7 +329,7 @@ func tools(h *Holder, l *listening) []stdio.Tool {
return h.Recheck(strArg(a, "id"), strArg(a, "why"), aPerson)
}},
{Name: seat + "release",
Description: "A person's word that a held delivery goes on: it starts delivering. With why.",
Description: "A person's word that a held delivery goes on: it starts delivering. A delivery held for a failed backup asks the controller to take the backups again first. With why.",
Input: map[string]any{"type": "object", "properties": map[string]any{"id": str("the delivery's id"),
"why": str("why, kept with the transition")}, "required": []string{"id", "why"}},
Run: func(a map[string]any) (any, error) {
@@ -63,7 +63,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) {
}
// It calls exactly what its ports ask: the controller's verbs and the forge's tools.
want := []string{}
for _, v := range []string{"delivery-plan", "delivery-check", "deliver", "delivery-stop", "delivery-walks", "forget-checks"} {
for _, v := range []string{"delivery-plan", "delivery-check", "deliver", "delivery-backups-again", "delivery-stop", "delivery-walks", "forget-checks"} {
want = append(want, "seat:"+ControllerSeat+"."+v)
}
want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status",
@@ -5,6 +5,7 @@ import (
"fmt"
"maps"
"regexp"
"slices"
"sort"
"strings"
"time"
@@ -74,6 +75,10 @@ type Delivery struct {
MergedAs string `json:"merged_as,omitempty"`
MergedAt time.Time `json:"merged_at"`
HeldWhy string `json:"held_why,omitempty"`
// BackupsAgainAfter is the revision of its walk when a person released it for a failed backup: a record at that
// revision or before still holds the failure the controller was asked to clear, and does not hold it again
// (novox/hq ADR 0404, issue 1068).
BackupsAgainAfter int64 `json:"backups_again_after,omitempty"`
// FromForge is a delivery made from the forge's word at its merge: its head was never heard by this owner.
// History is why it is history, once it is (novox/hq issue 313).
FromForge bool `json:"from_forge,omitempty"`
@@ -500,11 +505,26 @@ type Walk struct {
Revision int64 `json:"revision"`
Release *WalkRelease `json:"release,omitempty"`
Delivery *WalkDelivery `json:"delivery,omitempty"`
// Backups is what the backups the controller asks before it sends anything came to (novox/hq ADR 0404, issue
// 1068). PROVISIONAL field name and shape until the controller's contract (issue 1065) settles them.
Backups *WalkBackups `json:"backups,omitempty"`
// Phases is the walk's time from its merge, phase by phase, as the controller works it out (novox/hq ADR
// 0282): what a delivery's time is kept from. Absent from a controller that does not say it.
Phases *WalkPhases `json:"phases,omitempty"`
}
// WalkBackups is the backups of a walk before its send: the failed ones, each with the backup's own words.
type WalkBackups struct {
Failed []WalkBackupFailure `json:"failed,omitempty"`
}
// WalkBackupFailure is one backup that failed or stayed silent past its bound.
type WalkBackupFailure struct {
Node string `json:"node"`
Module string `json:"module"`
Error string `json:"error"`
}
// WalkModule is one module of a walk.
type WalkModule struct {
State string `json:"state,omitempty"`
@@ -712,6 +732,17 @@ type WalkSeen struct {
// Built is, per module, what the walk's build of it did (novox/hq ADR 0350 §4): "reused from <check build>" or
// "built: <why>" — what `show` says beside the build phase.
Built map[string]string `json:"built,omitempty"`
// BackupsFailed are the backups before the send that failed, each with the backup's words (ADR 0404).
BackupsFailed []WalkBackupFailure `json:"backups_failed,omitempty"`
}
// backupWords says the failed backups: the machine, the module and the backup's own words.
func (w *WalkSeen) backupWords() string {
var parts []string
for _, f := range w.BackupsFailed {
parts = append(parts, fmt.Sprintf("%s on %s: %s", f.Module, f.Node, f.Error))
}
return "a backup before the send failed, and nothing was sent: " + strings.Join(parts, "; ")
}
// Seen is a walk as a delivery keeps it.
@@ -745,6 +776,9 @@ func (w Walk) Seen() *WalkSeen {
}
sort.Strings(s.Moved)
s.MovedSaid = true
if w.Backups != nil && len(w.Backups.Failed) > 0 {
s.BackupsFailed = slices.Clone(w.Backups.Failed)
}
return s
}
@@ -53,6 +53,9 @@ type Controller interface {
Check(group string, members []Member) (string, error)
// Deliver lets a waiting walk start.
Deliver(walk, why string) error
// BackupsAgain asks the controller to take a held walk's failed backups again, and to go on when they pass
// (novox/hq ADR 0404 clause 4). PROVISIONAL verb name until the controller's contract settles it.
BackupsAgain(walk, why string) error
// Stop ends a walk, said as stopped by whom.
Stop(walk, why, by string) error
// StopWithout ends a walk and walks its other merges again without the merges named, each
@@ -259,6 +262,11 @@ func (c seatController) Deliver(walk, why string) error {
return err
}
func (c seatController) BackupsAgain(walk, why string) error {
_, err := c.verb("delivery-backups-again", map[string]any{"plan": walk, "why": why})
return err
}
func (c seatController) ForgetChecks(owner, repo string, number int) error {
_, err := c.verb("forget-checks", map[string]any{"owner": owner, "repo": repo, "number": strconv.Itoa(number)})
return err
@@ -85,6 +85,7 @@ const (
EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves
EvLifted Event = "lifted" // what held it for a person is no longer so: its group's composed check passed, or a walk now answers its merge
EvRewalk Event = "rewalk" // the walk that carried a later commit for it failed, or was stopped for another merge (ADR 0299): it waits for the walk that walks it again
EvBackupFailed Event = "backup failed" // a backup before the send failed or stayed silent: nothing of its plan was sent (novox/hq ADR 0404)
EvTrunkMoved Event = "trunk moved" // another merge moved its trunk, and its verdict names the trunk commit before (novox/hq ADR 0323)
)
@@ -335,6 +336,9 @@ var Table = []Row{
Guard: "its walk started on a word that was not this owner's: the controller's own path, which waits for " +
"nobody, or a person's `plans go`",
Holds: func(d *Delivery, f Facts) error {
if d.Walk != nil && len(d.Walk.BackupsFailed) > 0 && d.Walk.Revision > d.BackupsAgainAfter {
return errors.New("a backup before the send failed: only a release asks the backups again")
}
return unless(d.Walk != nil && d.Walk.Started() && (!d.Walk.Waited() || d.Walk.LetGoByAPerson()),
"its walk was not started by a person or the controller's own path")
}},
@@ -351,6 +355,24 @@ var Table = []Row{
return unless(f.Why != "" && f.By != "", "a release is a person's word, with why")
}},
// **A failed backup holds the delivery while nothing was sent** (novox/hq ADR 0404 clause 4): the controller asks
// the backups before it asks or sends anything, and a failed or silent one holds its plan. Observed, from the
// plan's record; it does not lift by itself: a person's release asks the controller to take the backups again.
{From: []State{Delivering}, Event: EvBackupFailed, To: Held,
Guard: "its walk's record names a backup before the send that failed or stayed silent, and nothing of the plan " +
"was sent; not one the controller was already asked to take again",
Holds: func(d *Delivery, f Facts) error {
switch {
case d.Walk == nil || len(d.Walk.BackupsFailed) == 0:
return errors.New("no backup failed")
case len(d.Walk.Moved) > 0:
return errors.New("a module of the plan was sent already")
case d.Walk.Revision <= d.BackupsAgainAfter:
return errors.New("the controller was asked to take the backups again, and has not said so yet")
}
return nil
},
Says: func(d *Delivery) string { return d.Walk.backupWords() }},
{From: []State{Delivering}, Event: EvSuperseded, To: Superseded,
Guard: "its walk was superseded and names no walk that took it over: a walk that took it over answers its merge, and it follows that walk (novox/hq ADR 0276)",
Holds: func(d *Delivery, f Facts) error {
@@ -505,6 +527,9 @@ func Apply(d *Delivery, ev Event, act bool, f Facts) (Transition, error) {
}
t := Transition{At: f.Now, From: d.State, To: r.To, Event: ev, Why: why, By: by}
d.State, d.Since = r.To, f.Now
if r.Event == EvBackupFailed {
d.HeldWhy = why
}
d.Transitions = append(d.Transitions, t)
if len(d.Transitions) > keptTransitions {
d.Transitions = d.Transitions[len(d.Transitions)-keptTransitions:]
@@ -88,6 +88,9 @@ func aDeliveryFor(r Row) (*Delivery, Facts) {
case EvLifted:
d.MergedAs, d.Lifted = "aaaa1111", "its group's composed check passed"
d.Walk = &WalkSeen{ID: "plan-1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"}
case EvBackupFailed:
d.Walk = &WalkSeen{ID: "plan-1", State: walkBuilding, Revision: 4,
BackupsFailed: []WalkBackupFailure{{Node: "ace", Module: "pg", Error: "no space left on device"}}}
case EvFailed:
d.Walk = &WalkSeen{ID: "plan-1", State: walkFailed}
case EvSuperseded:
@@ -132,7 +135,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing.
func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) {
events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead,
EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop,
EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved}
EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed}
for _, from := range append([]State{None}, AllStates...) {
for _, ev := range events {
for _, act := range []bool{false, true} {
@@ -368,8 +368,23 @@ func (h *Holder) Release(id, why, by string) (string, error) {
h.mu.Unlock()
return "", err
}
// Held for a failed backup before the send: the controller is asked to take the backups again (novox/hq ADR
// 0404 clause 4); its record at this revision or before still holds the failure, and does not hold it again.
backupsOf := ""
if d.Walk != nil && len(d.Walk.BackupsFailed) > 0 {
backupsOf, d.BackupsAgainAfter = d.Walk.ID, d.Walk.Revision
}
d.HeldWhy = ""
h.moved(d, []Transition{t})
if backupsOf != "" {
h.mu.Unlock()
if err := h.Controller.BackupsAgain(backupsOf, "released by "+by+": "+why); err != nil {
h.again("a walk's backups the controller refused or did not answer")
return "", fmt.Errorf("%s is delivering on %s's word, and the controller could not be asked to take the backups of "+
"%s again (%v): the failed backup holds it again when its record is read", id, by, backupsOf, err)
}
return fmt.Sprintf("%s is delivering on %s's word; the backups of its walk %s are asked again", id, by, backupsOf), nil
}
var walk string
if d.Walk != nil && d.Walk.Waits {
walk = d.Walk.ID
+1
View File
@@ -40,6 +40,7 @@
"seat:mesh-controller.delivery-plan",
"seat:mesh-controller.delivery-check",
"seat:mesh-controller.deliver",
"seat:mesh-controller.delivery-backups-again",
"seat:mesh-controller.delivery-stop",
"seat:mesh-controller.delivery-walks",
"seat:mesh-controller.forget-checks",