store: migrations under BEGIN IMMEDIATE, the version read again inside
test / test (push) Canceled after 0s
test / test (push) Canceled after 0s
D-S3. A deferred BEGIN starts nothing until the first access (SQLite, lang_transaction.html § 2.2), and migrate() read user_version outside the transaction: after an upgrade the panel and dmarc-ingest, opening the file at once, could both read the old version, both begin, and the second applied the next migration again on the migrated file - "already exists", that Open failing (nothing damaged: its transaction rolled back). The DSN now has _txlock=immediate (modernc.org/sqlite v1.60.1, sqlite.go applyQueryParams() sets beginMode, tx.go newTx() issues "begin immediate" for any transaction not read-only), so a migration holds the write lock from its BEGIN, and migrate() reads user_version again inside it and skips one another process applied meanwhile. Test TestTwoOpensMigrateOnce: a raw connection holds BEGIN IMMEDIATE on a new file while two Opens read version 0 and queue on their first BEGIN; released, both succeed and user_version is the number of migrations. Without the fix it fails as the register said: "apply migration 0001_init.sql: SQL logic error: table settings already exists (1)". Stand (2026-10-15): go vet and go test ./... pass on the server (24 packages); make e2e exit 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import (
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// 2.0 starts from one baseline: the 1.x chain is gone from the binary, not
|
||||
@@ -383,3 +384,57 @@ func TestBaselineConstraints(t *testing.T) {
|
||||
t.Errorf("after deleting the user %d inbound assignment(s) stayed (%v)", n, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Two processes opening one file that needs migrating - the panel and
|
||||
// dmarc-ingest after an upgrade - each apply every migration at most once:
|
||||
// both read the old version, both wait for the write lock, and the second
|
||||
// finds the work done (assumptions D-S3). Before _txlock=immediate and the
|
||||
// re-read, the second ran 0001 again and failed with "already exists".
|
||||
func TestTwoOpensMigrateOnce(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "selfpost.db")
|
||||
// The blocker holds the write lock while both opens read user_version 0
|
||||
// and queue on their first BEGIN IMMEDIATE.
|
||||
blocker, err := sql.Open("sqlite", "file:"+path+"?_pragma=journal_mode(WAL)")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer blocker.Close()
|
||||
blocker.SetMaxOpenConns(1)
|
||||
if _, err := blocker.Exec("BEGIN IMMEDIATE"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
errs := make(chan error, 2)
|
||||
for i := 0; i < 2; i++ {
|
||||
go func() {
|
||||
st, err := Open(path)
|
||||
if err == nil {
|
||||
err = st.Close()
|
||||
}
|
||||
errs <- err
|
||||
}()
|
||||
}
|
||||
time.Sleep(500 * time.Millisecond) // both are waiting on the lock by now
|
||||
if _, err := blocker.Exec("COMMIT"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for i := 0; i < 2; i++ {
|
||||
if err := <-errs; err != nil {
|
||||
t.Fatalf("open %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
st, err := Open(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer st.Close()
|
||||
var version int
|
||||
if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
entries, _ := fs.ReadDir(migrationsFS, "migrations")
|
||||
if version != len(entries) {
|
||||
t.Fatalf("user_version %d, want %d", version, len(entries))
|
||||
}
|
||||
}
|
||||
|
||||
+25
-2
@@ -34,7 +34,17 @@ func Open(path string) (*Store, error) {
|
||||
// busy_timeout first. SQLite 3.53.4: foreign_keys is per connection
|
||||
// (https://sqlite.org/foreignkeys.html § 2), journal_mode=WAL stays in the
|
||||
// file (https://sqlite.org/wal.html § 3.3).
|
||||
dsn := fmt.Sprintf("file:%s?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)&_pragma=foreign_keys(ON)", path)
|
||||
//
|
||||
// _txlock=immediate: every transaction that may write takes the write
|
||||
// lock at BEGIN (modernc.org/sqlite v1.60.1, sqlite.go applyQueryParams()
|
||||
// sets beginMode, tx.go newTx() issues "begin immediate"; read-only
|
||||
// transactions keep a plain begin). A deferred BEGIN starts nothing
|
||||
// until the first access (lang_transaction.html § 2.2), so two processes
|
||||
// - the panel and dmarc-ingest after an upgrade - could each read the
|
||||
// schema version and then both run the next migration; an immediate one
|
||||
// waits through busy_timeout, holding no lock that the other needs, and
|
||||
// migrate() reads the version again under it (assumptions D-S3).
|
||||
dsn := fmt.Sprintf("file:%s?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)&_pragma=foreign_keys(ON)&_txlock=immediate", path)
|
||||
db, err := sql.Open("sqlite", dsn)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("open database: %w", err)
|
||||
@@ -70,7 +80,10 @@ const applicationID = 0x53503230
|
||||
var ErrForeignSchema = errors.New("database was not created by SelfPost 2.x; 2.0 starts from an empty data directory (docs/schema-migrations.md)")
|
||||
|
||||
// migrate applies embedded migrations in filename order, tracking progress via
|
||||
// SQLite's PRAGMA user_version so each migration runs at most once.
|
||||
// SQLite's PRAGMA user_version so each migration runs at most once: the
|
||||
// version is read again inside each migration's transaction, which holds the
|
||||
// write lock from its BEGIN (Open, _txlock), and a migration another process
|
||||
// applied in the meantime is skipped.
|
||||
func (s *Store) migrate() error {
|
||||
entries, err := fs.ReadDir(migrationsFS, "migrations")
|
||||
if err != nil {
|
||||
@@ -123,6 +136,16 @@ func (s *Store) migrate() error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("begin migration %s: %w", name, err)
|
||||
}
|
||||
var now int
|
||||
if err := tx.QueryRow("PRAGMA user_version").Scan(&now); err != nil {
|
||||
tx.Rollback()
|
||||
return fmt.Errorf("read schema version for %s: %w", name, err)
|
||||
}
|
||||
if now >= target {
|
||||
// Applied by another process while this one waited for the lock.
|
||||
tx.Rollback()
|
||||
continue
|
||||
}
|
||||
// One Exec runs the whole file: modernc.org/sqlite v1.60.1, stmt.go
|
||||
// stmt.exec(), "FALLBACK PATH: Multi-statement script" prepares and
|
||||
// steps each statement of the tail in turn. It runs with foreign_keys
|
||||
|
||||
Reference in New Issue
Block a user