amadan

carlos / platform Public

lease, dbdoor: let a stop reach the integrity check

CheckDB ran its integrity scan with QueryRow and Exec and no context, so
a carlos db stop after the downloads waited out a full scan of the copy.
lease gains CheckDBContext (ExecContext and QueryRowContext, errors
wrapped so a cancellation is still context.Canceled), and CheckDB is now
a wrapper over it, so the activator and ops callers do not change.
RestoreTo checks with the restore's context, reports a stop rather than
a failed check, and checks the context once more before handing the
copy over.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude f7490ce17213160f0cc6da3d25c05f70f3c3858b parent 5c53511
4 files changed, +65 −11
  • internal/dbdoor/restore.go +11 −3
  • internal/dbdoor/restore_test.go +26 −2
  • internal/lease/check.go +13 −6
  • internal/lease/check_test.go +15 −0

CI passed — run details

diff --git a/internal/dbdoor/restore.go b/internal/dbdoor/restore.go
index 95f66afad..385e23295 100644
--- a/internal/dbdoor/restore.go
+++ b/internal/dbdoor/restore.go
@@ -45,9 +45,9 @@ type Restored struct {
// restoredName is the copy's file name inside its private directory.
const restoredName = "replica.db"
-// checkDB is lease.CheckDB, the one check a disaster recovery leans on. A
+// checkDB is lease.CheckDBContext, the one check a disaster recovery leans on. A
// variable only so a test can make it refuse.
-var checkDB = lease.CheckDB
+var checkDB = lease.CheckDBContext
// RestoreTo restores the replica c reads into a new 0700 directory under
// parent (os.TempDir() when empty) and proves it with lease.CheckDB.
@@ -103,10 +103,18 @@ func RestoreTo(ctx context.Context, c *Client, parent string) (*Restored, error)
os.RemoveAll(dir)
return nil, err
}
- if err := checkDB(out); err != nil {
+ if err := checkDB(ctx, out); err != nil {
os.RemoveAll(dir)
+ if cerr := ctx.Err(); cerr != nil {
+ return nil, cerr
+ }
return nil, fmt.Errorf("%w: %v", ErrCheckFailed, err)
}
+ // A stop that lands as the check finishes still withdraws the copy.
+ if err := ctx.Err(); err != nil {
+ os.RemoveAll(dir)
+ return nil, err
+ }
txid, at := rec.newest()
return &Restored{Path: out, Dir: dir, TXID: txid, At: at}, nil
}
diff --git a/internal/dbdoor/restore_test.go b/internal/dbdoor/restore_test.go
index 81d91b0aa..7b51ab40a 100644
--- a/internal/dbdoor/restore_test.go
+++ b/internal/dbdoor/restore_test.go
@@ -133,8 +133,8 @@ func TestRestoreToRefusesACopyThatFailsTheCheck(t *testing.T) {
src := t.TempDir()
dbdoortest.Replicate(t, src, 1)
var checked string
- defer func(old func(string) error) { checkDB = old }(checkDB)
- checkDB = func(path string) error {
+ defer func(old func(context.Context, string) error) { checkDB = old }(checkDB)
+ checkDB = func(_ context.Context, path string) error {
checked = path
return errors.New("integrity_check: page 2 is never used")
}
@@ -463,3 +463,27 @@ func TestRestoreToGivesUpOnAPermanentlyBusyDoor(t *testing.T) {
}
assertEmptyDir(t, parent)
}
+
+// A stop that lands while the copy is being checked still withdraws it:
+// RestoreTo checks the context after CheckDB and hands nothing over.
+func TestRestoreToStopsWhenCancelledDuringTheCheck(t *testing.T) {
+ src := t.TempDir()
+ dbdoortest.Replicate(t, src, 1)
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ defer func(old func(context.Context, string) error) { checkDB = old }(checkDB)
+ var sawCtx bool
+ checkDB = func(c context.Context, path string) error {
+ sawCtx = c == ctx
+ cancel() // the stop arrives as the check finishes
+ return nil
+ }
+ parent := t.TempDir()
+ if _, err := RestoreTo(ctx, NewClient(newTestDoor(t, src).do), parent); !errors.Is(err, context.Canceled) {
+ t.Fatalf("RestoreTo cancelled during the check = %v, want context.Canceled", err)
+ }
+ if !sawCtx {
+ t.Fatal("the check did not get the restore's context")
+ }
+ assertEmptyDir(t, parent)
+}
diff --git a/internal/lease/check.go b/internal/lease/check.go
index 8fdac6469..28a516964 100644
--- a/internal/lease/check.go
+++ b/internal/lease/check.go
@@ -33,6 +33,13 @@ var ErrNoReplica = errors.New("no replica in prefix")
// database passes integrity_check and would be useless in the recovery it was
// meant for.
func CheckDB(path string) error {
+ return CheckDBContext(context.Background(), path)
+}
+
+// CheckDBContext is CheckDB that stops when ctx does: the integrity scan
+// reads every page, and a caller that has been asked to stop (carlos db,
+// on Ctrl-C) should not finish a full scan first.
+func CheckDBContext(ctx context.Context, path string) error {
db, err := sql.Open("sqlite", "file:"+path+"?mode=ro")
if err != nil {
return err
@@ -41,19 +48,19 @@ func CheckDB(path string) error {
db.SetMaxOpenConns(1)
// busy_timeout first, always (Global Constraints) — and on a garbage file
// this first statement is what fails loudly instead of a later query.
- if _, err := db.Exec("PRAGMA busy_timeout=5000"); err != nil {
- return fmt.Errorf("open: %v", err)
+ if _, err := db.ExecContext(ctx, "PRAGMA busy_timeout=5000"); err != nil {
+ return fmt.Errorf("open: %w", err)
}
var res string
- if err := db.QueryRow("PRAGMA integrity_check").Scan(&res); err != nil {
- return fmt.Errorf("integrity_check: %v", err)
+ if err := db.QueryRowContext(ctx, "PRAGMA integrity_check").Scan(&res); err != nil {
+ return fmt.Errorf("integrity_check: %w", err)
}
if res != "ok" {
return fmt.Errorf("integrity_check: %s", res)
}
var tables int
- if err := db.QueryRow("SELECT count(*) FROM sqlite_master WHERE type='table'").Scan(&tables); err != nil {
- return fmt.Errorf("schema count: %v", err)
+ if err := db.QueryRowContext(ctx, "SELECT count(*) FROM sqlite_master WHERE type='table'").Scan(&tables); err != nil {
+ return fmt.Errorf("schema count: %w", err)
}
if tables == 0 {
return errors.New("restored database has no tables")
diff --git a/internal/lease/check_test.go b/internal/lease/check_test.go
index 65828388d..8156b30e5 100644
--- a/internal/lease/check_test.go
+++ b/internal/lease/check_test.go
@@ -221,3 +221,18 @@ func TestVerifyReplicaReturnsTheRestoredChangeCounter(t *testing.T) {
t.Fatalf("restored change counter = %d, want %d", got, want)
}
}
+
+// A stop reaches the integrity scan: CheckDBContext returns the context's
+// error instead of reading the whole file first.
+func TestCheckDBContextHonoursCancellation(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "c.db")
+ seedDB(t, path)
+ if err := CheckDBContext(context.Background(), path); err != nil {
+ t.Fatalf("live context: %v", err)
+ }
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+ if err := CheckDBContext(ctx, path); !errors.Is(err, context.Canceled) {
+ t.Fatalf("cancelled context = %v, want context.Canceled", err)
+ }
+}