amadan
host: the cleaning parks .corrupt and the litestream shadow only when the install recorded them absent (final re-review residual)
The database install journals pre_install inside its guard, right before the rename: which of the two siblings it does not refuse (the quarantine and the litestream shadow) were absent, Lstat'ed. A replay before the rename looks again. The target's cleaning parks either only when the database was installed and pre_install records it absent; present then, or no record, is left in place and logged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+188
−7
internal/host/movesland.go+1 −0internal/host/moveslandinstall.go+60 −0internal/host/movesrollback.go+26 −7internal/host/movesrollback_finalfix_test.go+101 −0
diff --git a/internal/host/movesland.go b/internal/host/movesland.go| index 7087ab6e5..dd1a46700 100644 |
| --- a/internal/host/movesland.go |
| +++ b/internal/host/movesland.go |
| @@ -112,6 +112,7 @@ const ( |
| landStepVerify = "verify" // the restore-verification entry is gone |
| landStepFilesManifest = "files_manifest" // the prepared files manifest (C8; skipped until it exists) |
| landStepFilesGen = "files_gen" // .files-gen written |
| + landStepPreInstall = "pre_install" // the database's siblings absent before its install (JSON preInstall) |
| ) |
| // landingFilesManifestSkipped is what the files_manifest step records while |
diff --git a/internal/host/moveslandinstall.go b/internal/host/moveslandinstall.go| index 1d8825d09..6f5c56245 100644 |
| --- a/internal/host/moveslandinstall.go |
| +++ b/internal/host/moveslandinstall.go |
| @@ -594,6 +594,9 @@ func (ir *installRun) restore(ctx context.Context, obs hostclaim.Observed) (sha |
| return err |
| } |
| } |
| + if err := ir.notePreInstall(); err != nil { |
| + return err |
| + } |
| if err := os.Rename(built, dbPath); err != nil { |
| return fmt.Errorf("install the database: %w", err) |
| } |
| @@ -616,6 +619,63 @@ func (ir *installRun) restore(ctx context.Context, obs hostclaim.Observed) (sha |
| return want, restoreInstalled, nil |
| } |
| +// preInstall is the landStepPreInstall record: which of the database's |
| +// unchecked siblings (installSiblingPaths) were absent, relative to the |
| +// data directory, when the install was about to put the database in place. |
| +// Only a sibling recorded absent can later count as this op's (the target's |
| +// cleaning, rollbackOwnership; the final re-review's residual): one already |
| +// there was not made by anything of this install's. |
| +type preInstall struct { |
| + Absent []string `json:"absent"` |
| +} |
| + |
| +// installSiblingPaths are the database's siblings the install does not |
| +// refuse beside an absent database (it refuses a WAL and shared memory): |
| +// the quarantine and the litestream shadow, relative to the data directory. |
| +func installSiblingPaths(host string) []string { |
| + db := host + ".db" |
| + return []string{db + ".corrupt", ".activator-" + db + "-litestream"} |
| +} |
| + |
| +// notePreInstall journals preInstall inside the database install's guard, |
| +// right before the rename: each sibling Lstat'ed, never followed. A replay |
| +// before the rename (a crash after the step) looks again and journals what |
| +// it sees, so the record is always the look that immediately preceded the |
| +// rename, under the same guard. A replay that finds the database already |
| +// installed never reaches here, so a step lost that way is missing, and its |
| +// siblings are never counted as the op's. |
| +func (ir *installRun) notePreInstall() error { |
| + j, err := ir.journal() |
| + if err != nil { |
| + return err |
| + } |
| + raw, replay := j.Steps[landStepPreInstall] |
| + rec := preInstall{Absent: []string{}} |
| + for _, p := range installSiblingPaths(ir.rec.Host) { |
| + _, err := os.Lstat(filepath.Join(ir.cfg().DataDir, p)) |
| + switch { |
| + case errors.Is(err, os.ErrNotExist): |
| + rec.Absent = append(rec.Absent, p) |
| + case err != nil: |
| + return err |
| + default: |
| + log.Printf("moves: %s: %s is already there before the database is installed; it is not the %s's", ir.rec.Host, p, ir.src.Name) |
| + } |
| + } |
| + body, err := json.Marshal(rec) |
| + if err != nil { |
| + return err |
| + } |
| + if replay && string(body) == raw { |
| + return nil |
| + } |
| + j.Mark(landStepPreInstall, string(body)) |
| + if err := j.Write(ir.cfg().DataDir); err != nil { |
| + return err |
| + } |
| + return ir.fault("after-" + landStepPreInstall) |
| +} |
| + |
| // refuseInstall writes the install's refusal for a build that does not |
| // verify. A refusal that landed is restoreMarked; one refused (the record |
| // moved on) is restoreWaiting, and nothing is installed either way. |
diff --git a/internal/host/movesrollback.go b/internal/host/movesrollback.go| index 3cef68bcc..c9d5d85d0 100644 |
| --- a/internal/host/movesrollback.go |
| +++ b/internal/host/movesrollback.go |
| @@ -1344,11 +1344,16 @@ func (r *rollbackRun) parkLeftovers(ctx context.Context, j *moves.Journal, mark |
| // - the files directory and <host>.env: the landing's placement receipt |
| // (files_placed), or before it the staged extraction's identities |
| // (staged), which a rename keeps: the live path must have that identity; |
| -// - the database, its marker, WAL, shared memory, quarantine and |
| -// litestream shadow: the landing installed its database, as the record |
| -// says (landing.installed_db_sha256, stamped by the CAS that ends |
| -// configured); the install refused a WAL or shared memory beside an |
| -// absent database, so what sits beside it since is the op's instance's; |
| +// - the database, its marker, WAL and shared memory: the landing installed |
| +// its database, as the record says (landing.installed_db_sha256, stamped |
| +// by the CAS that ends configured); the install refused a WAL or shared |
| +// memory beside an absent database, so what sits beside it since is the |
| +// op's instance's; |
| +// - the quarantine and the litestream shadow, which the install does not |
| +// refuse: the database installed as above, and the install's journal |
| +// records the path absent just before it put the database in place |
| +// (pre_install, notePreInstall). Present then, or no record, is never |
| +// the op's (the final re-review's residual); |
| // - the rendered config, <host>.config.env: the held render's ack on this |
| // box names this op, and the file's sha256 is the one the ack recorded; |
| // - the sidecar's env: never, since a moved host has no sidecar. |
| @@ -1356,7 +1361,8 @@ type rollbackOwnership struct { |
| host, dataDir string |
| placed stagedFiles // the landing's receipt, or its staged identities |
| installed bool |
| - configSHA256 string // the held render's ack, "sha256:<hex>", or "" |
| + absentBefore map[string]bool // the siblings pre_install recorded absent |
| + configSHA256 string // the held render's ack, "sha256:<hex>", or "" |
| } |
| // ownership reads the evidence: the landing's journal, the record and the |
| @@ -1379,6 +1385,17 @@ func (r *rollbackRun) ownership(ctx context.Context, j *moves.Journal) (rollback |
| if l := r.rec.Move.Landing; l != nil && l.InstalledDBSHA256 != "" { |
| ev.installed = true |
| } |
| + if raw, ok := j.Steps[landStepPreInstall]; ok { |
| + var pre preInstall |
| + if err := json.Unmarshal([]byte(raw), &pre); err != nil { |
| + log.Printf("moves: %s: the journal's %s step is unreadable (%v); the database's siblings are not counted as %s's", host, landStepPreInstall, err, id) |
| + } else { |
| + ev.absentBefore = map[string]bool{} |
| + for _, p := range pre.Absent { |
| + ev.absentBefore[p] = true |
| + } |
| + } |
| + } |
| body, _, err := r.s.Get(ctx, appconfig.ConfigAckKey(cfg.Box, host)) |
| switch { |
| case errors.Is(err, os.ErrNotExist): |
| @@ -1416,8 +1433,10 @@ func (ev rollbackOwnership) owns(path, identity string) (bool, error) { |
| return false, err |
| } |
| return got == ev.configSHA256, nil |
| - case db, db + ".gen", db + "-wal", db + "-shm", db + ".corrupt", ".activator-" + db + "-litestream": |
| + case db, db + ".gen", db + "-wal", db + "-shm": |
| return ev.installed, nil |
| + case db + ".corrupt", ".activator-" + db + "-litestream": |
| + return ev.installed && ev.absentBefore[path], nil |
| } |
| return false, nil |
| } |
diff --git a/internal/host/movesrollback_finalfix_test.go b/internal/host/movesrollback_finalfix_test.go| index 24ca5662e..d227e650d 100644 |
| --- a/internal/host/movesrollback_finalfix_test.go |
| +++ b/internal/host/movesrollback_finalfix_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "context" |
| "crypto/sha256" |
| "encoding/hex" |
| + "encoding/json" |
| "errors" |
| "fmt" |
| "io/fs" |
| @@ -433,3 +434,103 @@ func TestLandingWritesPassThePolicy(t *testing.T) { |
| } |
| p.want(t, "local/held-timers", "configured/held-config", "configured/held-release", "configured/restore", "configured/install") |
| } |
| + |
| +// plantSiblings writes a quarantine file and a litestream shadow directory |
| +// beside the database at the flat paths. |
| +func plantSiblings(t *testing.T, dir, body string) []string { |
| + t.Helper() |
| + corrupt := p1Host + ".db.corrupt" |
| + shadow := ".activator-" + p1Host + ".db-litestream" |
| + if err := os.MkdirAll(filepath.Join(dir, shadow), 0o750); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, b := range map[string]string{corrupt: body, filepath.Join(shadow, "generation"): body} { |
| + if err := os.WriteFile(filepath.Join(dir, name), []byte(b), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + return []string{corrupt, shadow} |
| +} |
| + |
| +// Finding 1, the re-review's residual (task-final-rereview-1.md): the |
| +// quarantine file and the litestream shadow are the op's only when the |
| +// install journalled them absent before it installed the database. One |
| +// already there when the landing installed survives the cleaning byte for |
| +// byte; ones made after the install are parked; with the journal entry |
| +// missing (an install that recorded none), they are left. |
| +func TestRollbackCleaningOwnsDatabaseSiblingsOnlyWhenRecordedAbsent(t *testing.T) { |
| + t.Run("present before the install", func(t *testing.T) { |
| + h := &rollbackHarness{landingHarness: newLandingHarness(t)} |
| + h.flip(t) |
| + foreign := plantSiblings(t, h.tdir, "not the landing's") |
| + before := treeSnapshot(t, h.tdir, foreign...) |
| + h.land(t) |
| + h.installed(t) |
| + h.step(t, provision.LandRestored, provision.LandProbing) |
| + h.tpos = h.n |
| + h.rollBack(t) |
| + h.cleanTarget(t) |
| + if after := treeSnapshot(t, h.tdir, foreign...); !sameSnapshot(before, after) { |
| + t.Fatalf("the siblings the landing found changed:\nbefore %v\nafter %v", before, after) |
| + } |
| + }) |
| + t.Run("made after the install", func(t *testing.T) { |
| + h := newRollbackHarness(t) |
| + made := plantSiblings(t, h.tdir, "the landing's instance") |
| + h.rollBack(t) |
| + h.cleanTarget(t) |
| + for _, p := range made { |
| + if _, err := os.Lstat(filepath.Join(h.tdir, p)); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("%s is still live: %v", p, err) |
| + } |
| + } |
| + parked := parkedUnder(t, h.tdir) |
| + if !parkedHas(parked, "/artifacts/"+made[0], "the landing's instance") || !parkedHas(parked, "/artifacts/"+made[1]+"/generation", "the landing's instance") { |
| + t.Fatalf("the op's siblings were not parked: %v", parked) |
| + } |
| + }) |
| + t.Run("no record of their absence", func(t *testing.T) { |
| + h := newRollbackHarness(t) |
| + j := h.landJournal(t) |
| + if _, ok := j.Steps[landStepPreInstall]; !ok { |
| + t.Fatalf("the install journalled no %s step: %v", landStepPreInstall, j.Steps) |
| + } |
| + delete(j.Steps, landStepPreInstall) |
| + if err := j.Write(h.tdir); err != nil { |
| + t.Fatal(err) |
| + } |
| + made := plantSiblings(t, h.tdir, "unrecorded") |
| + before := treeSnapshot(t, h.tdir, made...) |
| + h.rollBack(t) |
| + h.cleanTarget(t) |
| + if after := treeSnapshot(t, h.tdir, made...); !sameSnapshot(before, after) { |
| + t.Fatalf("siblings with no recorded absence changed:\nbefore %v\nafter %v", before, after) |
| + } |
| + }) |
| +} |
| + |
| +// A crash after pre_install is journalled and before the database's rename: |
| +// the replay looks again right before its rename, so a sibling that |
| +// appeared in between is recorded present and survives the cleaning. |
| +func TestPreInstallReplayLooksAgain(t *testing.T) { |
| + h := &rollbackHarness{landingHarness: newLandingHarness(t)} |
| + h.flip(t) |
| + h.landAt = "after-" + landStepPreInstall |
| + h.crashLand(t) |
| + foreign := plantSiblings(t, h.tdir, "appeared after the first look") |
| + before := treeSnapshot(t, h.tdir, foreign...) |
| + h.freshTarget() |
| + h.land(t) |
| + h.installed(t) |
| + var pre preInstall |
| + if err := json.Unmarshal([]byte(h.landJournal(t).Steps[landStepPreInstall]), &pre); err != nil || len(pre.Absent) != 0 { |
| + t.Fatalf("pre_install after the replay: %+v %v", pre, err) |
| + } |
| + h.step(t, provision.LandRestored, provision.LandProbing) |
| + h.tpos = h.n |
| + h.rollBack(t) |
| + h.cleanTarget(t) |
| + if after := treeSnapshot(t, h.tdir, foreign...); !sameSnapshot(before, after) { |
| + t.Fatalf("siblings that appeared before the replay changed:\nbefore %v\nafter %v", before, after) |
| + } |
| +} |