diff --git a/modules/mesh-delivery/cmd/mesh-delivery/backup_test.go b/modules/mesh-delivery/cmd/mesh-delivery/backup_test.go index e30022bf..1b802538 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/backup_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/backup_test.go @@ -126,3 +126,29 @@ func TestADeliveryHeldForAFailedBackupIsNotLetGoByItsWalkAlone(t *testing.T) { t.Fatal("a walk started on the controller's own path lifted the backup hold") } } + +func TestAReleaseWhoseBackupsTheControllerRefusedToAskAgainLeavesTheDeliveryHeld(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() + w.ctl.refuseBackupsAgain = true + _, err := w.h.Release(id, "freed some space", "jochen") + if err == nil || !strings.Contains(err.Error(), "held again") { + t.Fatalf("the release of a refused word answered %v, want an error saying it is held again", err) + } + w.settleAll() + d := w.delivery(id) + if d.State != Held || d.BackupsAgainAfter != 0 || !strings.Contains(d.HeldWhy, "no space left on device") { + t.Fatalf("after the refusal it is %s (again-after %d, why %q), want held for the backup", d.State, d.BackupsAgainAfter, d.HeldWhy) + } + // The controller takes the word the next time: the delivery goes on. + w.ctl.refuseBackupsAgain = false + if _, err := w.h.Release(id, "freed some space", "jochen"); err != nil { + t.Fatal(err) + } + if s := w.state(id); s != Delivering { + t.Fatalf("released again, it is %s", s) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index bf616f19..c40666e8 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -94,7 +94,9 @@ type fakeController struct { delivers []string // backupsAgain are the walks whose failed backups were asked again. backupsAgain []string - stops []string + // refuseBackupsAgain refuses the word to take a walk's backups again. + refuseBackupsAgain bool + stops []string // withouts are the stops that left merges out (novox/hq ADR 0299): " without ,…". withouts []string // refuseWithout refuses every stop with merges to leave out, as a controller before ADR 0299 does; mergedAt are @@ -130,6 +132,9 @@ func (c *fakeController) BackupsAgain(walk, why string) error { if c.down { return errors.New("down") } + if c.refuseBackupsAgain { + return errors.New("the controller refuses the backups") + } w, known := c.walks[walk] if !known || w.Backups == nil || w.Backups.State != "failed" { return fmt.Errorf("%s holds no failed backup", walk) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 634399bc..06c4d7db 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -370,7 +370,7 @@ func (h *Holder) Release(id, why, by string) (string, error) { } // 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 := "" + backupsOf, againBefore := "", d.BackupsAgainAfter if d.Walk != nil && len(d.Walk.BackupsFailed) > 0 { backupsOf, d.BackupsAgainAfter = d.Walk.ID, d.Walk.Revision } @@ -379,9 +379,16 @@ func (h *Holder) Release(id, why, by string) (string, error) { if backupsOf != "" { h.mu.Unlock() if err := h.Controller.BackupsAgain(backupsOf, "released by "+by+": "+why); err != nil { + // The controller did not clear the failure: its record still holds it, so the delivery is held for it again now. + h.mu.Lock() + if d := h.deliveries[id]; d != nil { + d.BackupsAgainAfter = againBefore + h.settleAndKeep(d) + } + h.mu.Unlock() 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.Errorf("%s was not released: the controller could not be asked to take the backups of %s again "+ + "(%v), so it is held again for the failed backup", id, 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 }