carlos / platform Public

activator, host, hostbox: no database is born beside a foreign seed marker (re-review finding 8)

F2b checked the <db>.seed marker only when a database was already at the
path. A placement that crashed between its marker write and its rename
leaves seed A's marker alone; a plain instance created at the host then
booted fresh beside it, and its next wake took the instance's own
database for A's leftover and purged it, losing writes not yet
replicated.

restorePlain now reconciles the marker first, whether or not a database
is there (reconcileSeedMarker, replacing keepWarmCache): a marker naming
a seed other than the row's own current seed is purged through
purgeSeedTarget (the database and sidecars, the marker, .seed-sd_*.tmp)
before any restore or fresh boot; a marker read error fails the wake.
The other paths that put a database at an instance's path drop the
marker too: hostbox's cold-restore purge, and the moves landing's
database install. The seed paths themselves already purge or rewrite it
(placeSeed, restoreOwn).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago a2f33f66c922d3f8bb2768d882bfdc3409030f05 parent a3d022a

7 files changed, +163 −20

internal/activator/backend_exec.go +4 −3
internal/activator/seedplace.go +26 −16
internal/activator/seedplace_linux_test.go +70 −0
internal/host/movesland_finalfix_test.go +25 −0
internal/host/moveslandinstall.go +7 −0
internal/hostbox/purgedb_test.go +27 −0
internal/hostbox/restore.go +4 −1

diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go

index 01cd225dc..e6c7daec5 100644

--- a/internal/activator/backend_exec.go

+++ b/internal/activator/backend_exec.go

@@ -652,10 +652,11 @@ func (b *ExecBackend) Restore(ctx context.Context, rt registry.Route) (bool, err

// when there is none of the three.

func (b *ExecBackend) restorePlain(ctx context.Context, rt registry.Route) (bool, error) {

host, dbPath := rt.Host, b.pathsFor(rt).dbPath

+ if err := reconcileSeedMarker(rt, dbPath); err != nil {

+ return false, err

+ }

if _, err := os.Stat(dbPath); err == nil {

- if kept, err := keepWarmCache(rt, dbPath); err != nil || kept {

- return false, err // warm cache — see doWake

- }

+ return false, nil // 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 18b557515..0402bb484 100644

--- a/internal/activator/seedplace.go

+++ b/internal/activator/seedplace.go

@@ -195,34 +195,44 @@ 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

+// reconcileSeedMarker runs first on the plain restore path, whether or not

+// a database is at dbPath (final review F2b, re-review finding 8). A

+// <db>.seed marker beside the path that names a seed other than the row's

+// own current seed (the row has none, or another) is a leftover of that

+// seed: an aborted --from seed whose purge ran on no box, a rolled-back

+// import, or a placement that crashed between its marker and its rename

+// and left the marker alone. Whatever is there goes, through

+// purgeSeedTarget (the database and its sidecars, the marker, any

+// .seed-sd_*.tmp), before anything is restored or created, so the file is

+// never served as this row's warm cache and no new database is ever born

+// beside a foreign marker that a later wake would read as the old seed's.

+// A marker naming the row's own seed, or none, changes nothing. 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) {

+//

+// A relative path is no seeded path (seeded rows are always given the

+// nested, absolute one), so it is left to the restore as before.

+func reconcileSeedMarker(rt registry.Route, dbPath string) error {

+ if !filepath.IsAbs(dbPath) {

+ return nil

+ }

+ dbPath = filepath.Clean(dbPath)

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)

+ return 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

+ return nil

}

dir, base, err := splitDBPath(dbPath)

if err != nil {

- return false, err

+ return 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)

+ return fmt.Errorf("restore %s: purge what 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

+ log.Printf("activator: %s: %s held seed %s's marker, not this route's (seed %q): purged before the restore", rt.Host, dbPath, marker, own)

+ return nil

}

// seedPlacedAt reports whether dir/base is a regular file whose marker names

diff --git a/internal/activator/seedplace_linux_test.go b/internal/activator/seedplace_linux_test.go

index a207eb2ef..da0ea98e0 100644

--- a/internal/activator/seedplace_linux_test.go

+++ b/internal/activator/seedplace_linux_test.go

@@ -902,3 +902,73 @@ func TestPlainRestoreNeverServesAnotherSeedsDatabase(t *testing.T) {

})

}

+// Re-review finding 8: a placement that crashed between its marker and its

+// rename leaves seed A's marker with no database. A plain instance later

+// created at the host must never be born beside it: its first wake removes

+// the marker (and any fetch leftover) before it boots fresh, the instance

+// writes, and its second wake keeps those writes as its warm cache.

+func TestPlainWakeOverAMarkerOnlyLeftoverKeepsTheReplacementsData(t *testing.T) {

+ countOwnReplicaLists(t, false)

+ b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{})

+ oldRestore := restoreReplica

+ restoreReplica = func(context.Context, string, string, string, string) (bool, error) { return false, nil }

+ defer func() { restoreReplica = oldRestore }()

+ dir := filepath.Dir(rt.DBPath)

+ if err := os.MkdirAll(dir, 0o700); err != nil {

+ t.Fatal(err)

+ }

+ a, _ := seeds.NewID()

+ tmp := filepath.Join(dir, seedTempName(a))

+ for p, body := range map[string]string{rt.DBPath + SeedMarkerSuffix: a + "\n", tmp: "half a fetch"} {

+ if err := os.WriteFile(p, []byte(body), 0o600); err != nil {

+ t.Fatal(err)

+ }

+ if fresh, err := b.Restore(context.Background(), rt); err != nil || !fresh {

+ t.Fatalf("first wake: fresh %v err %v; want fresh", fresh, err)

+ }

+ for _, p := range []string{rt.DBPath + SeedMarkerSuffix, tmp} {

+ if _, err := os.Lstat(p); !os.IsNotExist(err) {

+ t.Fatalf("%s survived the first wake: %v", p, err)

+ }

+ replacement := []byte("the replacement's own writes")

+ if err := os.WriteFile(rt.DBPath, replacement, 0o600); err != nil {

+ t.Fatal(err)

+ }

+ if fresh, err := b.Restore(context.Background(), rt); err != nil || fresh {

+ t.Fatalf("second wake: fresh %v err %v; want the warm cache", fresh, err)

+ }

+ if got, err := os.ReadFile(rt.DBPath); err != nil || !bytes.Equal(got, replacement) {

+ t.Fatalf("database after the second wake = %q, %v; want the replacement's writes", got, err)

+ }

+// A marker that cannot be read fails the wake whether or not a database is

+// there, and nothing is created beside it.

+func TestPlainWakeFailsOnAnUnreadableMarkerAlone(t *testing.T) {

+ b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{})

+ oldRestore := restoreReplica

+ restoreReplica = func(context.Context, string, string, string, string) (bool, error) {

+ t.Fatal("restored beside an unreadable marker")

+ return false, nil

+ }

+ defer func() { restoreReplica = oldRestore }()

+ os.MkdirAll(filepath.Dir(rt.DBPath), 0o700)

+ a, _ := seeds.NewID()

+ marker := rt.DBPath + SeedMarkerSuffix

+ if err := os.WriteFile(marker, []byte(a+"\n"), 0); err != nil {

+ t.Fatal(err)

+ }

+ t.Cleanup(func() { os.Chmod(marker, 0o600) })

+ if _, err := ReadSeedMarker(rt.DBPath); err == nil {

+ t.Skip("this user reads a mode 0 file (root): the read error cannot be staged")

+ }

+ if fresh, err := b.Restore(context.Background(), rt); err == nil || fresh {

+ t.Fatalf("fresh %v err %v; want a failed wake", fresh, err)

+ }

+ if _, err := os.Lstat(rt.DBPath); !os.IsNotExist(err) {

+ t.Fatalf("a database was created beside an unreadable marker: %v", err)

+ }

diff --git a/internal/host/movesland_finalfix_test.go b/internal/host/movesland_finalfix_test.go

index 5e85ba952..9288bb4f9 100644

--- a/internal/host/movesland_finalfix_test.go

+++ b/internal/host/movesland_finalfix_test.go

@@ -8,6 +8,7 @@ import (

"testing"

"time"

+ "github.com/carlosframework/platform/internal/activator"

"github.com/carlosframework/platform/internal/hostbox"

"github.com/carlosframework/platform/internal/hostclaim"

"github.com/carlosframework/platform/internal/hostguard"

@@ -420,3 +421,27 @@ func TestLandingCutDeadlineMarksRollback(t *testing.T) {

}

})

}

+// Re-review finding 8: a seed marker left beside the destination by some

+// earlier seed never survives the landing's database install, so the

+// installed database is never read as that seed's leftover.

+func TestLandingInstallRemovesAStaleSeedMarker(t *testing.T) {

+ h := newLandingHarness(t)

+ h.flip(t)

+ h.route.Replication = ""

+ db := provision.DefaultDBPath(h.tdir, p1Host)

+ if err := os.MkdirAll(filepath.Dir(db), 0o700); err != nil {

+ t.Fatal(err)

+ }

+ if err := os.WriteFile(db+activator.SeedMarkerSuffix, []byte("sd_aaaaaaaaaaaaaaaaaaaaaaaaaa\n"), 0o600); err != nil {

+ t.Fatal(err)

+ }

+ h.land(t)

+ h.installed(t)

+ if _, err := os.Stat(db); err != nil {

+ t.Fatalf("the database was not installed: %v", err)

+ }

+ if _, err := os.Lstat(db + activator.SeedMarkerSuffix); !errors.Is(err, os.ErrNotExist) {

+ t.Fatalf("the stale seed marker survived the install: %v", err)

+ }

diff --git a/internal/host/moveslandinstall.go b/internal/host/moveslandinstall.go

index 4a1be3cfe..427d840f5 100644

--- a/internal/host/moveslandinstall.go

+++ b/internal/host/moveslandinstall.go

@@ -13,6 +13,7 @@ import (

"path/filepath"

"strings"

+ "github.com/carlosframework/platform/internal/activator"

"github.com/carlosframework/platform/internal/appconfig"

"github.com/carlosframework/platform/internal/hostbox"

"github.com/carlosframework/platform/internal/hostclaim"

@@ -579,6 +580,12 @@ func (l *landingRun) restore(ctx context.Context, obs hostclaim.Observed) (sha s

return err

}

+ // No seed marker may stand beside the database this landing

+ // installs: it would name another seed's leftover for it

+ // (re-review finding 8).

+ if err := os.Remove(dbPath + activator.SeedMarkerSuffix); err != nil && !errors.Is(err, os.ErrNotExist) {

+ return err

+ }

if err := os.Rename(built, dbPath); err != nil {

return fmt.Errorf("install the database: %w", err)

}

diff --git a/internal/hostbox/purgedb_test.go b/internal/hostbox/purgedb_test.go

new file mode 100644

index 000000000..7c634a493

--- /dev/null

+++ b/internal/hostbox/purgedb_test.go

@@ -0,0 +1,27 @@

+package hostbox

+import (

+ "os"

+ "path/filepath"

+ "testing"

+// Re-review finding 8: the cold restore's purge removes the database's

+// seed marker with it, so a database restored here is never born beside a

+// marker naming another seed.

+func TestPurgeDBRemovesTheSeedMarker(t *testing.T) {

+ db := filepath.Join(t.TempDir(), "a.shop.example.com.db")

+ for _, p := range []string{db, db + "-wal", db + ".gen", db + ".seed"} {

+ if err := os.WriteFile(p, []byte("x"), 0o600); err != nil {

+ t.Fatal(err)

+ }

+ if err := purgeDB(db); err != nil {

+ t.Fatal(err)

+ }

+ for _, p := range []string{db, db + "-wal", db + ".gen", db + ".seed"} {

+ if _, err := os.Lstat(p); !os.IsNotExist(err) {

+ t.Errorf("%s survived the purge", p)

+ }

diff --git a/internal/hostbox/restore.go b/internal/hostbox/restore.go

index 903e0ee4a..d9d3cd0fd 100644

--- a/internal/hostbox/restore.go

+++ b/internal/hostbox/restore.go

@@ -188,7 +188,10 @@ func safeGeneration(s string) bool {

}

func purgeDB(path string) error {

- for _, p := range []string{path, path + "-wal", path + "-shm", path + ".gen"} {

+ // ".seed" is the activator's seed marker (activator.SeedMarkerSuffix),

+ // spelled out because the activator imports this package: a marker

+ // must not outlive the database it named (re-review finding 8).

+ for _, p := range []string{path, path + "-wal", path + "-shm", path + ".gen", path + ".seed"} {

if err := os.Remove(p); err != nil && !errors.Is(err, os.ErrNotExist) {

return fmt.Errorf("purge %s: %w", p, err)

}