amadan
activator: a plain restore never serves another seed's leftover database (final fix F2b)
F2's purge runs only on a box that sees an aborted --from seed with no row. In a multi-box region another box can mark the seed rolled back first, and the box that ran it then skips the rolled-back record and never purges, so its database stays at the nested path for a later plain instance to wake on as warm cache. Other routes could leave such a file behind as well. restorePlain now asks keepWarmCache before it accepts a database already on disk. A <db>.seed marker naming a seed other than the row's own current seed (the row has none, or another) means the file is that seed's leftover: it is purged through purgeSeedTarget and the wake goes on as a cold restore (the row's own replica, or fresh where the plain path allows it). A marker naming the row's own seed, or no marker, keeps today's warm cache. A marker that cannot be read fails the wake, with nothing served or purged. F2's purge in the rollback stays as the first line of defence. TestAPlainWakeAfterPurgeSeedLocalFindsNoDatabase pinned the hazard (a leftover with a marker taken as warm cache); it now expects the wake to come up fresh in both of its cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+134
−5
internal/activator/backend_exec.go+3 −1internal/activator/seedplace.go+30 −0internal/activator/seedplace_linux_test.go+96 −0internal/activator/seedpurge_linux_test.go+5 −4
CI failed — run details
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go| index efffad264..01cd225dc 100644 |
| --- a/internal/activator/backend_exec.go |
| +++ b/internal/activator/backend_exec.go |
| @@ -653,7 +653,9 @@ func (b *ExecBackend) Restore(ctx context.Context, rt registry.Route) (bool, err |
| func (b *ExecBackend) restorePlain(ctx context.Context, rt registry.Route) (bool, error) { |
| host, dbPath := rt.Host, b.pathsFor(rt).dbPath |
| if _, err := os.Stat(dbPath); err == nil { |
| - return false, nil // warm cache — see doWake |
| + if kept, err := keepWarmCache(rt, dbPath); err != nil || kept { |
| + return false, err // warm cache — see doWake |
| + } |
| } else if !os.IsNotExist(err) { |
| return false, err |
| } |
diff --git a/internal/activator/seedplace.go b/internal/activator/seedplace.go| index 15bd4cdbb..18b557515 100644 |
| --- a/internal/activator/seedplace.go |
| +++ b/internal/activator/seedplace.go |
| @@ -195,6 +195,36 @@ func (b *ExecBackend) seededRestore(ctx context.Context, rt registry.Route, src |
| return false, nil |
| } |
| +// keepWarmCache decides a database already at dbPath on the plain restore |
| +// path (final review F2b). A <db>.seed marker beside it that names a seed |
| +// other than the row's own current seed (the row has none, or another) |
| +// says the file is a leftover of that seed: an aborted --from seed whose |
| +// purge ran on no box, a rolled-back import, anything else that left a |
| +// seeded database behind. It is never served as this row's warm cache: it |
| +// is purged with everything that stands in for it (purgeSeedTarget), and |
| +// kept is false, so the caller goes on as a cold restore. A marker naming |
| +// the row's own seed, or no marker, keeps the file as before. A marker |
| +// that cannot be read fails the wake, with nothing served or purged. |
| +func keepWarmCache(rt registry.Route, dbPath string) (kept bool, err error) { |
| + marker, err := ReadSeedMarker(dbPath) |
| + if err != nil { |
| + return false, fmt.Errorf("restore %s: the seed marker beside %s cannot be read: %w", rt.Host, dbPath, err) |
| + } |
| + own := rowSeedID(rt.SeedSource) |
| + if marker == "" || marker == own { |
| + return true, nil |
| + } |
| + dir, base, err := splitDBPath(dbPath) |
| + if err != nil { |
| + return false, err |
| + } |
| + if err := purgeSeedTarget(dir, base); err != nil { |
| + return false, fmt.Errorf("restore %s: purge the database seed %s left at %s: %w", rt.Host, marker, dbPath, err) |
| + } |
| + log.Printf("activator: %s: the database at %s was seed %s's, not this route's (seed %q): purged, and restored cold", rt.Host, dbPath, marker, own) |
| + return false, nil |
| +} |
| + |
| // seedPlacedAt reports whether dir/base is a regular file whose marker names |
| // id. A missing, symlinked or oversized marker or database is "not placed", |
| // and is purged; any other error is returned. |
diff --git a/internal/activator/seedplace_linux_test.go b/internal/activator/seedplace_linux_test.go| index 2a26e5ea7..a207eb2ef 100644 |
| --- a/internal/activator/seedplace_linux_test.go |
| +++ b/internal/activator/seedplace_linux_test.go |
| @@ -806,3 +806,99 @@ func TestSeededRestoreOfItsOwnReplicaWritesTheMarker(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Final review F2b: a database left at a route's path by another seed is |
| +// never this route's warm cache. Its <db>.seed marker names seed A; the |
| +// row has no seed, or seed B. The wake purges it and restores cold: the |
| +// row's own replica when there is one, fresh when the plain path allows |
| +// it. A marker naming the row's own seed keeps the file, as does no |
| +// marker; a marker that cannot be read fails the wake and touches nothing. |
| +func TestPlainRestoreNeverServesAnotherSeedsDatabase(t *testing.T) { |
| + leftover := []byte("seed A's data") |
| + for _, c := range []struct { |
| + name string |
| + seed func(t *testing.T, b *ExecBackend, rt registry.Route) (registry.Route, string) // the row, and the marker's seed |
| + replica bool // the row has a replica of its own |
| + unread bool // the marker cannot be read |
| + kept bool |
| + fresh bool |
| + failed bool |
| + }{ |
| + {name: "no seed on the row", seed: func(t *testing.T, _ *ExecBackend, rt registry.Route) (registry.Route, string) { |
| + a, _ := seeds.NewID() |
| + return rt, a |
| + }, fresh: true}, |
| + {name: "the row names seed B", seed: func(t *testing.T, b *ExecBackend, rt registry.Route) (registry.Route, string) { |
| + rt, _, _ = seededRoute(t, b, rt, seeds.StatePlaced) |
| + a, _ := seeds.NewID() |
| + return rt, a |
| + }, replica: true}, |
| + {name: "the row's own seed", seed: func(t *testing.T, b *ExecBackend, rt registry.Route) (registry.Route, string) { |
| + rt, _, id := seededRoute(t, b, rt, seeds.StatePlaced) |
| + return rt, id |
| + }, kept: true}, |
| + {name: "no marker", seed: func(t *testing.T, _ *ExecBackend, rt registry.Route) (registry.Route, string) { |
| + return rt, "" |
| + }, kept: true}, |
| + {name: "unreadable marker", seed: func(t *testing.T, _ *ExecBackend, rt registry.Route) (registry.Route, string) { |
| + a, _ := seeds.NewID() |
| + return rt, a |
| + }, unread: true, failed: true}, |
| + } { |
| + t.Run(c.name, func(t *testing.T) { |
| + countOwnReplicaLists(t, c.replica) |
| + b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{}) |
| + rt, marker := c.seed(t, b, rt) |
| + restored := 0 |
| + oldRestore := restoreReplica |
| + restoreReplica = func(_ context.Context, dbPath, _, _, _ string) (bool, error) { |
| + restored++ |
| + if !c.replica { |
| + return false, nil |
| + } |
| + return true, writeMinimalSQLite(dbPath) |
| + } |
| + defer func() { restoreReplica = oldRestore }() |
| + if err := os.MkdirAll(filepath.Dir(rt.DBPath), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(rt.DBPath, leftover, 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if marker != "" { |
| + if err := os.WriteFile(rt.DBPath+SeedMarkerSuffix, []byte(marker+"\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if c.unread { |
| + if err := os.Chmod(rt.DBPath+SeedMarkerSuffix, 0); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { os.Chmod(rt.DBPath+SeedMarkerSuffix, 0o600) }) |
| + if _, err := ReadSeedMarker(rt.DBPath); err == nil { |
| + t.Skip("this user reads a mode 0 file (root): the read error cannot be staged") |
| + } |
| + } |
| + fresh, err := b.Restore(context.Background(), rt) |
| + if (err != nil) != c.failed || fresh != c.fresh { |
| + t.Fatalf("fresh %v err %v; want fresh %v failed %v", fresh, err, c.fresh, c.failed) |
| + } |
| + got, rerr := os.ReadFile(rt.DBPath) |
| + switch { |
| + case c.kept || c.failed: |
| + if rerr != nil || !bytes.Equal(got, leftover) || restored != 0 { |
| + t.Fatalf("database %q (%v), %d restores; want the file kept as it was and nothing restored", got, rerr, restored) |
| + } |
| + case bytes.Equal(got, leftover): |
| + t.Fatal("the other seed's database was served") |
| + case c.replica && (rerr != nil || restored != 1): |
| + t.Fatalf("database %v, %d restores; want the row's own replica", rerr, restored) |
| + } |
| + if !c.kept && !c.failed { |
| + if m, err := ReadSeedMarker(rt.DBPath); err != nil || m == marker { |
| + t.Fatalf("marker after the purge = %q, %v; want seed A's gone", m, err) |
| + } |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/activator/seedpurge_linux_test.go b/internal/activator/seedpurge_linux_test.go| index 28913a3bb..4bbd21927 100644 |
| --- a/internal/activator/seedpurge_linux_test.go |
| +++ b/internal/activator/seedpurge_linux_test.go |
| @@ -88,8 +88,9 @@ func TestPurgeSeedLocalNeverFollowsASymlink(t *testing.T) { |
| // Fix round 1, I1, the activator half: an aborted import's database left at |
| // a host's nested path is what a later plain instance at the host wakes |
| -// on. Left there, the plain wake takes it as a warm cache (not fresh); |
| -// after PurgeSeedLocal, the wake finds no file and comes up fresh. |
| +// on. After PurgeSeedLocal the wake finds no file and comes up fresh. Left |
| +// there, its <db>.seed marker names a seed the plain row does not, so the |
| +// wake purges it and comes up fresh all the same (final review F2b). |
| func TestAPlainWakeAfterPurgeSeedLocalFindsNoDatabase(t *testing.T) { |
| for _, purge := range []bool{false, true} { |
| b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{}) |
| @@ -110,10 +111,10 @@ func TestAPlainWakeAfterPurgeSeedLocalFindsNoDatabase(t *testing.T) { |
| restoreReplica = func(context.Context, string, string, string, string) (bool, error) { return false, nil } |
| fresh, err := b.Restore(context.Background(), rt) // a plain row: no seed column |
| restoreReplica = oldRestore |
| - if err != nil || fresh != purge { |
| + if err != nil || !fresh { |
| t.Fatalf("purge %v: fresh %v err %v", purge, fresh, err) |
| } |
| - if _, err := os.Lstat(rt.DBPath); purge && !os.IsNotExist(err) { |
| + if _, err := os.Lstat(rt.DBPath); !os.IsNotExist(err) { |
| t.Fatalf("the wake found a database at %s", rt.DBPath) |
| } |
| } |