Repository navigation
feat(migrations): one list of catalog migrations with a ledger in every store, for SQLite and PostgreSQL - #666
LeeroyHannigan wants to merge 40 commits into
Conversation
Add crates/storage/src/migrations, one numbered list of catalog migrations for every backend, and the runner that applies it. The migration_ids! list generates MigrationId and each id's name and description, and LATEST is its last entry. A backend declares, in an exhaustive match, the body each of its stores runs for an id, or NotApplicable with a reason. A body is a file that the file_body! macro embeds, or code that implements CodeBody. The runner applies the pending bodies in id order, catalog store first. Each store records the migrations it applied in its own ledger, extenddb_migrations, with a checksum per migration, and a backend records that row and the catalog_version row through record_rows, in the body's transaction where the store has one. The catalog version is the highest id recorded in every store it touches. The status that serve, migrate and verify act on is read from the ledgers, with an exit code per status, and run_locked reads it again under the migration lock and refuses a store the run cannot apply. reconverge runs every convergent migration again on a catalog that a release before 1.0 wrote to, and forget clears an unreleased migration. A run that stops after a committed step returns the steps it committed. A checksum test, behind the test-util feature, pins every released body, its kind and its convergent flag, and checks the rules for file bodies. CatalogMigrations is the interface the commands use to reach the installed backend's migrations. No backend installs it yet, so each backend sets catalog_migrations to None. advice.rs holds the messages the commands print. The resource-tag rewrite moves into migration 2's module, which every backend shares, and policy_migration.rs calls it. OpError::Database and MigrationError::Database keep the SQLSTATE of a failed statement, so the runner can tell a lock timeout from other failures by its code. Bootstrapper::init_target gives init the deployment the bootstrapper resolved, extract_arg reads --flag=value as clap accepts it, and a server factory can return BackendError::Catalog with the exit code serve ends with.
A backend that installs CatalogMigrations is driven through it, and the others keep their bootstrapper path. init takes the migration lock, runs the migrations as a create run, which refuses a catalog that is already initialized, and then writes the bootstrap rows. migrate previews from the status with no lock. With --yes it waits up to --lock-timeout for the lock, reads the status again under it and refuses a store that is not a catalog with exit 1. --lock-timeout is at most 86400 seconds and is refused where no lock is taken. migrate gains --list, --forget and --reconverge, and when a run stops it prints the steps the run committed and the copy it made. The preview and verify print the backend's status notes, and migrate --yes refuses before it takes the lock while a refusing note holds. serve refuses every status but Current and AheadCompatible with the status's exit code and the command that resolves it. Without a config file the advised command names the store, and migrate and verify read the same EXTENDDB__ variables serve does. The resource-tag marker check in serve is skipped for a backend on the runner, and the server removes its PID file when a backend ends the process with exit. The commands' tests run against a fake backend.
… them Each dump is a database that a released dev image created and seeded through the API, with its encryption key replaced by a fixed test key. regenerate.sh, seed.py and dump.py remake them, and the README gives the commands and says why the two datasets differ.
Migration 1 is a code body that runs migrations/0001_baseline.sql, then rebuilds backup_items only when the table has no id column and adds table_restores.source_table_arn only when it is missing. The first time it is applied to a database, the runner compares the database with 0001_baseline.expected.json and commits nothing on a difference. Migration 2 is the shared body, and migrations 3 and 4 are code bodies. The baseline seeds no delay settings, and the engine uses its defaults while those rows are absent. Each body runs on one dedicated connection with foreign keys off inside BEGIN IMMEDIATE. A foreign key violation the body added stops it, and the ledger row and the version row commit with the body. A run that applies anything first copies the database beside it with VACUUM INTO, with the database's mode and a name that no run reuses. The copy is written to a partial file, synced and renamed, and the run stops when less than three times the database's size is free. A run removes the partial files that stopped runs left, and a run that commits nothing removes its copy. The migration lock is the serve lock, retried until a timeout, and it refuses a missing database without creating a file. The server factory reads the status before it opens the engine. A dev build migrates instead, unless the file holds tables and is not a catalog, and an in-memory database is migrated on the engine's connection. The status opens the file read-only, and the runner opens its connection before its pool, so a database it cannot open fails at once with SQLite's cause. The legacy schema constant, its apply and shape steps, the version check and the shape gate are removed. The settings redaction test stores the delay it reads back, since init no longer writes it.
…ke them Each state is a plain dump of the catalog and data databases that a published PostgreSQL image created and seeded, two of them on pgvector, plus the v0.1.0 and v0.1.2 catalogs built from their tags' files. scrub.py replaces the encryption key, the password hashes and the connection string's password. regenerate.sh and regenerate-all.sh remake the states, with container names and ports taken from FIXTURE_PREFIX and FIXTURE_BASE_PORT.
Migration 1 is one file per database: the legacy files' statements with every index guarded, the three columns that catalogs from v0.1.0 and v0.1.1 lack, the shape steps, a stream sequence that only moves forward, the account_id probe before the idempotency table is recreated, the marker row deleted and the legacy ledger rows written. Each body commits with its ledger row. The runner sets lock_timeout to 5 s, retried three times on SQLSTATE 55P03, and client_min_messages with SET LOCAL after each BEGIN. No session sets a parameter at login, so the commands work behind PgBouncer. Migration 4 builds its indexes outside a transaction through the index helper. A build that has waited 5 s for a lock is cancelled, and its invalid index is dropped and built again, three times, before the body stops and names the sessions that hold the lock. The row is recorded only when every index is valid. The migration lock takes the serving key exclusively after its own key. A server holds the serving key shared on a session with TCP keepalives for its life, and a registration with no successful renewal for 30 s is lost, so the server exits 1. A migrator waits for an earlier migrator's statements in either database and then exits 6 naming them. A body that fails with SQLSTATE 42501 stops the run and names the objects the application role does not own, with a DO block of ALTER ... OWNER TO that gives them to it, and the status carries the same note. The runner and the status reach the data database through the catalog's data_database_connection_string row. The factory registers, then reads the status, before it creates the engine, and migrate connects as an admin role only for the pgvector attempt. The legacy runner, its files, the serve gates, CATALOG_VERSION and the catalog version errors nothing raises are removed. The documentation version test checks the binary version only, and the CLI tests run on the ledger. Every test target that needs a server fails without one when EXTENDDB_TEST_PG_REQUIRED is set, and the stream targets pin the control-plane delay with an upsert, since the baseline seeds no delay row. The bin crate's tests refuse a PostgreSQL migration declared additive and run migrate --yes on two empty databases.
No catalog seeds the delay settings now, so the settings page shows the value each backend's engine uses when the row is absent, read through OperationsEngine::setting_default. The page printed 500 for index_propagation_delay_ms, where SQLite and PostgreSQL use 10. The Cassandra and MongoDB engines read their control-plane delay default from a DEFAULT_CONTROL_PLANE_DELAY_SECONDS constant, which setting_default returns too.
devtools/schema-dump prints a fresh install's schema. The SQLite and PostgreSQL integration jobs compare it with docs/schema/<backend>.txt and upload the regenerated file on a difference. Per backend, check-*-catalog-lock runs serve and migrate on one catalog, the PostgreSQL one also through PgBouncer, and check-*-older-binary runs the 0.1.14 image against migrated catalogs and checks that this binary reports WrittenByOlderRelease and that --reconverge restores Current. The four scripts are Python on one helper module, catalog_checks.py, whose tests the Scripts test workflow runs. The storage-level job runs every PostgreSQL test target with EXTENDDB_TEST_PG_REQUIRED set, and the job without pgvector runs the migration targets on postgres:16. The release gate jobs, the npm one included, run check-released-through on the tagged commit, which fails while RELEASED_THROUGH is below the last migration. packaging/README.md names that step of a release.
ADR-0023 records the catalog migration design and supersedes ADR-0003. CONTRIBUTING.md gains "Adding a catalog migration" with a worked example, and the upgrade manual links to it. The upgrade manual describes the ledger and the catalog status with each command's exit code. It says that the application role must own every table, view and sequence of both databases, what the preview prints when it does not, and how the commands run behind PgBouncer. The manuals' samples, and the statements about the catalog version, the SQLite lock and migrate's credentials, describe what this binary does, and build-docs prints the last migration on the manuals' cover. Refs #495.
pid_exit.rs called libc::atexit, and this crate depends on libc only on Unix, so the Windows build of the dev binary failed. The module now declares the C runtime's atexit itself; glibc and the Windows CRT both provide it.
In .github/workflows/integration.yml, the storage, storage-postgres and storage-sqlite Cargo.toml files, crates/storage/src/error.rs and crates/storage-postgres/src/lib.rs both sides are kept, so the PostgreSQL storage step still runs every target with --tests (table_snapshot included), each crate has both the test-util and test-hooks features, StorageError has SnapshotExpired and not the catalog version variants, and PostgresEngine holds the serving registration beside the snapshot fields. crates/storage-postgres/src/migrations.rs stays deleted, apply_all_migrations moves to migrations/mod.rs and runs the migration runner, the runner's SQLSTATE 55P03 checks use crate::data, and the PostgreSQL and SQLite snapshot tests write the settings rows that migration 1 does not seed, the SQLite ones migrating with apply_for_tests.
# Conflicts: # README.md
robinnsc
left a comment
robinnsc
left a comment
There was a problem hiding this comment.
This holds up well on the upgrade paths, which is the hard part. We used three separate reviewers: the framework and SQLite, PostgreSQL, and operator behaviour.
What was checked:
- All 9 SQLite and 12 PostgreSQL released fixtures migrate to a schema identical to a fresh
init. - Real deployments written by the base binary, the 0.1.14 dev image, and the 0.1.10 and 0.1.14 PostgreSQL images came through with a wire-level snapshot unchanged: items, GSI/LSI queries, stream records, TTL, tags, backups and restores, vector search, users, groups, roles and keys. The only change was the intended policy rewrites.
kill -9at many points during SQLite and PostgreSQL runs never left a body without its ledger row on SQLite. The PostgreSQL catalog/data split resumed cleanly.- A drifted PostgreSQL catalog commits nothing.
- The exit-code matrix matches the description on both backends, and the preview changes nothing (byte- and dump-identical).
- 0.1.14
serve/migrate --yeson a migrated catalog give WrittenByOlderRelease, and--reconvergerestores the schema. - The ownership advice after
pg_dump --no-ownerrestores was correct, and--yesrefused before the lock. - Serve/migrate exclusion works directly and through PgBouncer in both modes.
- All four devtools check scripts pass with these binaries, and the SQLite lock check fails against main.
The ones I'd hold this for are the first two inline comments. --reconverge reruns migration 2 and can widen access. And PostgreSQL servers now exit when their registration session goes away (a database restart, pg_terminate_backend, or a short idle_in_transaction_session_timeout). The SQLite copy cost on large catalogs is the third one worth looking at before release.
Smaller notes:
verifyreturns the status code whenever it's non-zero (cmd_verify.rs:150), so pending migrations plus an unreachable data database exit 3, which reads as "run migrate --yes".- The total PostgreSQL wait can reach about 3×
--lock-timeout:lock_timeoutapplies to each lock statement, andwait_for_earlier_runnersthen adds another full timeout. - The backup advice says "the catalog database X and its data database" without naming the data database, though
resolvealready readsdata_database_name. forgetdeletes the row and writes the version row as two autocommit statements on SQLite. That's cosmetic, since nothing reads that row for the version now.- A refused
serve, ormigrate --yeson a foreign file, leaves an empty<db>.lockbeside it. docs/manuals/07-upgrade-manual.md:174-177still quotes the 0.0.4 output (Pending: catalog 0.0.3 -> 0.0.4 ...) next to the updated copy paragraph.- Manual line 539 says the rewrite "preserves each policy's decision". That isn't true for policies 0.1.14 stored after its own rewrite had run: those were inert on 0.1.14 and take effect after migration 2. It's the same as main, but maybe worth a sentence.
- The pre-migration copy takes the database's mode, so it is 0644 on a catalog that 0.1.x
initcreated and no server ever started. The description says the runner's files are owner-only. - Messages currently say "ExtendDB 0.1.14 needs migration 4". That's fine once the version is bumped.
--forget 0answers "migration 0 has been released". - The devtools scripts need Python ≥ 3.10 and
curl, which CONTRIBUTING doesn't mention. - Windows:
std::process::exitgoes throughExitProcess, which I don't think runs the executable's CRTatexittable, so the PID file may stay after a registration loss there. Low confidence.
| }, | ||
| MigrationId::ResourceTagConditionKeys => Migration::Applies { | ||
| kind: Kind::Breaking, | ||
| convergent: true, |
There was a problem hiding this comment.
Migration 2 being convergent means --reconverge reruns it over policies written through the 1.0 API, where a dynamodb:ResourceTag key is inert. The rewrite then gives them effect. Reproduced on SQLite with the head binary, each case on a migrated catalog:
- an
Allowgated onStringEquals dynamodb:ResourceTag/env=prodwent from denied to allowed on an env=prod table; Allow GetItem *plus aDenygated onStringNotEquals dynamodb:ResourceTag/env=prodwent from denied to allowed;- a
Denygated onStringEquals ...went from allowed to denied.
The run prints Ran again 2 resource_tag_condition_keys / rewrote ... policy .... Nothing previews it, because --reconverge needs --yes. It's also the command the WrittenByOlderRelease advice (advice.rs:153) tells operators to run, so it touches every policy written since the upgrade, not just whatever the 0.1.x binary wrote. The same applies at storage-postgres/src/migrations/mod.rs:89.
Could consider making migration 2 non-convergent and having --reconverge skip, rather than refuse, a non-convergent id that's already recorded. The "safe to run again at any later id" wording in table.rs:133, ADR rule 7 and CONTRIBUTING step 4 might also want a caveat for bodies that rewrite data written through the API.
There was a problem hiding this comment.
Fixed in 5576692. Migration 2 is now declared not convergent on SQLite and PostgreSQL. --reconverge runs the convergent migrations again, applies a migration to a store that has no row for it as migrate --yes does, and leaves a recorded migration that is not convergent as it is, with a line in its output:
Skipped 2 resource_tag_condition_keys: it is not convergent on sqlite, so it does not run again over data written since it was applied
It still refuses a ledger with a migration the binary does not carry.
Your first case is now part of tests/test_resource_tag_migration.py: after the upgrade, a user policy written through the API, Allow GetItem gated on StringEquals dynamodb:ResourceTag/env = prod, then the marker row written back and --reconverge run. With migration 2 declared convergent again, the reconverge rewrote that policy and its GetItem on the env=prod table went from denied to allowed. With the change the policy keeps its key and the GetItem stays denied, while the policy the upgrade itself rewrote still allows its GetItem. New tests on both backends check that a reconverge leaves policies written after migration 2, in every place a policy is stored, with the text they were written with, and that a policy with both forms on one key does not stop it.
The convergent doc comment, ADR-0023 points 1 and 7, CONTRIBUTING step 4 and the upgrade manual now say that a body that rewrites data clients wrote through the API is not convergent. The manual also states what remains: a policy that a SQLite dev build from 0.1.7 to 0.1.13 writes to the catalog after the upgrade keeps its dynamodb: key, which 1.0 leaves absent, so it has to be rewritten by hand; and 0.1.14's migrate --yes rewrites such keys whenever it runs, so it should not be run against a migrated catalog.
| ); | ||
| tracing::error!("{message}"); | ||
| eprintln!("{message}"); | ||
| std::process::exit(1); |
There was a problem hiding this comment.
Any error on the registration connection marks it lost at once (held.rs:76), and this exits every server within one renewal, 15 s. The 30 s in the description only covers renewals that get no answer. Measured with head vs main on the same catalog:
docker restartof PostgreSQL: head exits 1 after 15 s; main is healthy after 45 s.pg_terminate_backendon the registration session: head exits after 13-15 s.- A PgBouncer restart in transaction mode: head exits after 9 s.
ALTER ROLE ... SET idle_in_transaction_session_timeout = '10s': head exits 15 s after every start, so a supervisor loops. Main serves normally.
From the docs, PG 17's transaction_timeout (not reset by SELECT 1) and PgBouncer's idle_transaction_timeout would do the same, and a managed failover would stop every server. The exit also skips the serve shutdown path, so in-flight requests are cut and the workers' token never fires.
A few options:
- On loss, stop accepting requests, try to take the shared key again (that fails fast while a migration holds the exclusive key), re-check the status, and resume if it's unchanged.
- At least cancel the shutdown token first.
- List this under "differs from 0.1.14" and in the upgrade manual.
The idle-timeout part has a small fix of its own; see locks.rs:440.
There was a problem hiding this comment.
Fixed in 9ba4b62, with your first option, and in 5e5b316. A backend now gives the server a ServingRegistrar through its runtime hooks; PostgreSQL's opens a new session, takes the shared serving key with a try, and reads the status under the new key. Once the registration is lost, the server answers every new request, /health included, with ServiceUnavailable (HTTP 503), which the SDKs retry, and lets requests in flight finish. It tries to register again at once and then every second, and serves again once it holds the key and the catalog's status is the one it started with. When a migration holds the exclusive key or waits for the servers, when the status changed, or when 30 s pass, the server fires its background workers' shutdown token at once, and then stops through the serve shutdown path, as SIGTERM stops it: the listener drains and lets the requests already running finish, the PID file is removed, and the process exits 1. A shutdown signal that came first decides the exit status, so a stop asked for during a SIGTERM's drain still ends with 0. pid_exit.rs and its atexit handler are removed.
Measured with the release binary, polling /health every 100 ms:
docker restartof PostgreSQL 16.15 (back after 2.7 s): the server found the session gone at its next renewal, 13.9 s in, answered 503 for about 0.1 s, registered again on its first attempt after 64 ms, and was still serving 77 s in.- A PgBouncer restart in transaction mode (5.6 s): 503 for about 0.1 s at 13.7 s, registered again on its first attempt after 122 ms, still serving 81 s in.
- PostgreSQL stopped for 40 s: 503 from 13.8 s, 30 failed attempts, then the shutdown path and exit 1 at 44 s.
pg_terminate_backendon the registration session is in the new tests (tests/migrations_serving.rs) and indevtools/check-postgres-catalog-lock, which also checks that a server that cannot register again while a migration holds both keys exits 1 and removes its PID file. bcaabf9 adds a test that drops an attempt while its status read waits behind a lock on the ledger table, and checks that no session holds the serving key afterwards.
A server still learns of the loss at its next renewal, up to 15 s after it, and goes on serving until then, so a migrate --yes that was waiting for the servers can start beside it in those seconds, as at d4187ea. The manual now says so, next to its step that stops every server before migrating.
The role-level idle_in_transaction_session_timeout and PostgreSQL 17's transaction_timeout are fixed with your locks.rs:440 comment. PgBouncer's idle_transaction_timeout is a setting of PgBouncer that no transaction can change; the upgrade manual says to leave it at 0 or set it to 30 s or more, since the holders renew every 15 s. The manual and the description's "differs from 0.1.14" list now say what a server does when its registration is lost.
| // Inside the transaction that holds the key, so a transaction-mode pooler | ||
| // applies them to the backend the key lives on; they last as long as the | ||
| // transaction, which is the registration's life. | ||
| for (setting, value) in TCP_KEEPALIVES { |
There was a problem hiding this comment.
Since both the registration and the migration lock are transactions held open for the life of the server or run, it might be worth adding SET LOCAL idle_in_transaction_session_timeout = 0 here (and in the lock transaction near :225), and SET LOCAL transaction_timeout = 0 when server_version_num >= 170000. These SET LOCALs already reach the right backend behind a transaction pooler. Today a role or database setting under 15 s ends the session before the next SELECT 1. the_lock_keepalive_outlasts_the_idle_in_transaction_timeout uses a 300 ms test interval, so it doesn't cover the fixed 15 s production interval.
There was a problem hiding this comment.
Fixed in 3e3c3e0. Both key transactions, the registration (register_on) and the migration lock (take_migration_keys), now send SET LOCAL idle_in_transaction_session_timeout = 0 after BEGIN, and SET LOCAL transaction_timeout = 0 when server_version_num is 170000 or more. The 300 ms test is replaced by one at the production interval: a role with idle_in_transaction_session_timeout = '5s', and transaction_timeout = '20s' on PostgreSQL 17 and later, holds the registration and the migration lock for 32 s with renewals every 15 s, and both are still held, while a plain transaction of the same role left idle is ended by the setting. Without the two SET LOCALs the registration is lost. On PostgreSQL 17.11 the test passed in 32.8 s, and with the transaction_timeout statement left out it failed with "terminating connection due to transaction timeout"; CI's PostgreSQL is 16.
| } | ||
| let partial = partial_path(copy); | ||
| let target = format!("file:{}?modeof={}", uri_path(&partial), uri_path(db)); | ||
| let copied = sqlx::query("VACUUM INTO ?") |
There was a problem hiding this comment.
VACUUM INTO replays every CREATE, and each one costs time proportional to the schema already there, so the copy grows faster than linearly with the number of tables (about 11 schema objects per table with a GSI and an LSI). These catalogs were built by cloning a fixture, with 10 items per table:
- 2,001 tables:
migrate --yestook 33 s. - 4,001 tables (198 MB): 159.5 s on head vs 9.2 s on main.
VACUUM INTOalone took 120 s, whileConnection.backup()of the same file took 0.46 s.
With only one migration pending, the run still took 77-182 s, almost all of it the copy. Every --reconverge and every dev-server start pays it too. The run prints nothing between "Taking the migration lock..." and the end, so it looks hung for minutes with the lock held.
Could consider the online backup API (sqlite3_backup_*, via lock_handle()), or wal_checkpoint(TRUNCATE) under the lock followed by a plain file copy. A progress line before the copy, and one per step as it commits, would also help.
There was a problem hiding this comment.
Fixed in 5dbfdf2 and 62d1351. The copy is now taken with the online backup API, the first of your two suggestions, on two connections opened for the copy on a blocking thread: one to the database, and one to <copy>.partial, which is created 0600 first. The backup reads the database file and the committed frames of its WAL in one read transaction, so the copy holds every write committed when that transaction began, and its time grows with the database's size. The backup also copies the database's WAL mark, so the copy is then switched to rollback-journal mode, as VACUUM INTO left it, and can be opened without a -shm file beside it, also from a directory its reader may not write to. The run then syncs the partial file through a handle opened for writing, which FlushFileBuffers needs on Windows, renames it and syncs the directory. A line before the copy gives its path and the database's size, and a line after it gives the time it took:
--- Copying the database (82444288 bytes) to /srv/catalog.sqlite.pre-migration-3-to-4-20261101T113951Z...
--- Copied the database in 0.3 s
A copy that waits 30 s for a lock another connection holds stops the run before its first write with "another connection held a lock on the database for 30 s".
The backup does not go through lock_handle(), because the locked handle borrows the pool's connection and so cannot be moved to a blocking thread. Its connections and sqlx's use one SQLite library (storage-sqlite now depends on libsqlite3-sys 0.30.1, the version sqlx-sqlite links), so when the backup's connections close, SQLite keeps the runner connection's locks.
The checkpoint and plain file copy lost data when I tried it. POSIX locks belong to a process and a file, so closing the descriptor the copy opened on the database released the shared lock of the runner's connection. A sqlite3 client in another process that opened, read and closed the database during the run then took itself for the last connection, checkpointed the WAL and deleted it, and migration 4's later commit went to the deleted file: migrate --yes exited 0 and the catalog stayed at migration 3. A new test compares this process's locks on the database in /proc/locks before and after the copy; it fails with the file copy.
Measured with release binaries on a catalog of 2,000 tables, each with one GSI, one LSI and 3 items, built through the API (82 MB, 18,077 schema objects), with migration 4 pending after --forget 4. At a load average of about 30 on 16 cores, migrate --yes took 32.3 s at d4187ea and 17.2 s at this head, of which the copy took 0.7 s; the rest is migration 4's body and the status reads. An earlier build of the change, at a load average of about 12, took 7.8 s against 19.0 s at d4187ea, with 0.3 s for the copy. From Python's sqlite3 module on the same file, at a load average of about 17, VACUUM INTO took 38.7 s and the backup API 0.29 s. Both copies pass PRAGMA quick_check and hold the same row counts in every table.
A line per step as it commits is not in this change: the run prints the steps it applied when it ends. On this catalog migration 4's body, 7 to 16 s at those loads, is now the longest stretch without output.
| ) -> Result<(), MigrationError> { | ||
| let fail = |e: sqlx::Error| MigrationError::store("take the migration lock", e); | ||
| statement(conn, "BEGIN").await.map_err(fail)?; | ||
| let _: String = sqlx::query_scalar("SELECT set_config('lock_timeout', $1, true)") |
There was a problem hiding this comment.
PostgreSQL reads lock_timeout = 0 as no timeout, so migrate --yes --lock-timeout 0 waits forever, printing "Waiting up to 0 s". The clap range at cmd_migrate.rs:89 allows 0. While it waits it holds the migration key, so every server started in the meantime exits 6. On SQLite, 0 means one attempt and then exit 6. Mapping 0 to the try-lock result alone, or starting the range at 1, would line the two up.
There was a problem hiding this comment.
Fixed in 16d30d7, with 0 mapped to one attempt, as on SQLite. A held key with no time left is not waited for: the run rolls back its lock transaction and exits 6 naming the holders, so migrate --yes --lock-timeout 0 exits 6 at once while another migrator or a running server holds its key, and no server started meanwhile is refused. The same change gives a run one deadline (N2): the wait for the migration key, the wait for the servers and the wait for an earlier migrator's statements each get the time the earlier ones left, and each wait line prints the seconds left, rounded up, so "Waiting up to 0 s" no longer appears while time is left. The refusal for an earlier migrator's statements says that its seconds are the part of the lock timeout the earlier waits left (65ab3dd). A test with a 6 s timeout, a server holding its key until 4 s, a session holding the migration key until 2 s and an earlier migrator's pg_sleep(60) exits 6 after 5 to 7 s, where it took 12.6 s before.
| .assignment | ||
| .map_or_else(String::new, |a| format!("{a} ")); | ||
| match self.config { | ||
| Some(config) => format!(" {assignment}extenddb migrate --config {config} {flags}"), |
There was a problem hiding this comment.
The advised command isn't quite copy-safe:
- The config path isn't shell-quoted. In a directory named
sp ace's dir, pasting it into bash fails, and a space alone splits it into two arguments. - The config path is made absolute, but a relative store path inside it (
path = "extenddb.sqlite"is theinitdefault) still resolves against the cwd. Run from another directory, the advised--reconvergeanswered "not initialized; create it with 'extenddb init'", which would lead to a second deployment.
Quoting each path, and adding the store assignment or a cd <dir> && when the configured path is relative, would cover both. The env-variable form already gets this right.
There was a problem hiding this comment.
Fixed in a1b6aed. The advised command quotes the config file's path for a POSIX shell, and when the config file names the store by a relative path, as init writes it, the command starts with the store's assignment with its absolute path (EXTENDDB__STORAGE__SQLITE__PATH=...). A test runs the advised command through sh from another directory, with the config file in a directory named sp ace's dir. Run by hand the same way, the --reconverge that serve and migrate advised repaired the catalog the config file names.
| value INTEGER NOT NULL DEFAULT 0 | ||
| ); | ||
|
|
||
| INSERT OR IGNORE INTO seq_counters (name, value) |
There was a problem hiding this comment.
A fresh install no longer seeds control_plane_delay_seconds / index_propagation_delay_ms. The engines and the console fall back to the defaults, so requests are fine. But extenddb settings get control_plane_delay_seconds now prints "not found" with exit 1 on a new install (main prints 0.25), and settings list omits both. Could consider having cmd_settings use operations::setting_default like the console does, or listing it under "differs from 0.1.14".
There was a problem hiding this comment.
Fixed in 14201ce and de90538. settings get prints the engine's default (operations::setting_default) for a key with no row, as the console does, and settings list adds the defaults in key order. For the renamed key, get under either name prints the row the engine reads: the row of the current name, index_propagation_delay_ms, else the row of the old name, gsi_propagation_delay_ms. list adds no default beside a row under the old name.
| } | ||
| if let Ok(rows) = migrations.catalog_ledger(&target).await { | ||
| for row in rows.iter().filter(|r| !r.notes.is_empty()) { | ||
| println!(" Notes of migration {} {}:", row.id, row.name); |
There was a problem hiding this comment.
verify prints every ledger note on every run. On a 2,001-table catalog with 50,000 policies that's 14,019 lines (10,000 "rewrote" lines and 4,002 "created idx_base_key" lines), for the life of the catalog. A count and the first few lines, with a flag for the rest, would keep it readable.
There was a problem hiding this comment.
Fixed in 9ee2435. verify prints, for each migration with notes, the number of notes and the first 5, and verify --all-notes prints every note.
| return Ok(()); | ||
| } | ||
| let failed = format!( | ||
| "resource-tag rewrite: {} policy lookup(s) or write(s) failed", |
There was a problem hiding this comment.
With one policy_document set to {not json, preview, verify and migrate --yes all stop with exit 1 and only this count. They name neither the account, principal nor policy, nor the decode error, and the other policies of that principal aren't checked. Main also gave only a count, but now serve won't start until this passes, so the operator has to bisect the catalog by hand. Keeping the principal and error text in the report would fix that, and exit 5 ("a person must act") might fit better than 1.
There was a problem hiding this comment.
Fixed in 3e62ce6 and 697eb23. ManagementStore has two new methods, list_policy_documents and boundary_document, which return each stored document or its decode error, and the SQLite store implements them by decoding each document on its own. Migration 2 reports a document it cannot decode as an item that needs attention, with the account, principal, policy name and decode error, so the preview, verify, migrate --yes and serve exit 5 and name it. The principal's other policies and its boundary are still read, so the run names every policy it cannot read, and rewrites them all once those are replaced or deleted. A lookup that fails for another reason is named with its account, principal and error text, the first 5 of them, with exit 1. PostgreSQL stores the documents as jsonb, which refuses a document that does not decode, so its store keeps the trait's defaults.
| its rows may hold NULL" | ||
| )), | ||
| Some(found) => { | ||
| differences.push(format!("column {key} is {found:?}, expected {column:?}")) |
There was a problem hiding this comment.
nit: the drift refusal prints Rust Debug structs (Column { data_type: "double precision", not_null: true, default: Some("1"), ... }) and doesn't say what to do next, e.g. drop the extra column or restore the default and run again.
There was a problem hiding this comment.
Fixed in 8c56f27. The refusal now lists the differences one per line, with their count and what to do, for example:
after migration 1 the database acme_catalog differs from the v1.0 schema in 2 place(s), so nothing was committed:
column t.id has type bigint where v1.0 has integer; change it to integer NOT NULL
sequence t_seq is missing; create it
Make each change in the database acme_catalog, then run migrate --yes again; migration 1 checks the schema again before it commits.
A changed column names each attribute that differs (type, NULL or NOT NULL, default, collation) and the v1.0 definition to change it to; a missing column or constraint comes with the definition to add; a missing index with the CREATE INDEX that creates it; an index with another definition with both definitions; an index that is not valid with the REINDEX INDEX that rebuilds it; and a column v1.0 does not have says to drop it, after saving any data in it that is needed.
forget checked the released bound before it checked that the binary carries the id, so `extenddb migrate --forget 0` answered "migration 0 has been released". It now checks the id first and answers "this binary carries no migration 0; it carries migrations 1 to 4", whatever the released bound.
Migration 2 rewrites dynamodb:ResourceTag condition keys in stored policies to the aws:ResourceTag form. It was declared convergent, so --reconverge ran it again over every stored policy, including policies written through the API after the upgrade. From 1.0 a dynamodb:ResourceTag key written that way has no effect, as on the service, and the rewrite gave it one. --reconverge is the repair that WrittenByOlderRelease advises, so the repair could change the decision of any policy written since the upgrade. Migration 2 is now not convergent on SQLite and PostgreSQL. A reconverge runs the convergent migrations again, applies an id to a store that has no row for it, as migrate does, and leaves a recorded id that is not convergent as it is. The report names each one it left: Skipped 2 resource_tag_condition_keys: it is not convergent on sqlite, so it does not run again over data written since it was applied A reconverge no longer refuses a catalog with such an id; it still refuses a ledger with an id this binary does not carry. It does not run migration 2's detect either, so a policy written after the upgrade with both forms on one key does not stop the repair. migrate --list widens its convergent column to fit "not convergent". The convergent doc comment, ADR-0023, CONTRIBUTING step 4 and the upgrade manual say that a body that rewrites data that clients wrote through the API is not convergent. The manual adds that a policy a SQLite dev build from v0.1.7 to v0.1.13 writes after the upgrade keeps its dynamodb: keys, which v1.0 leaves absent, and that a v0.1.14 migrate --yes rewrites them whenever it runs. Tests: on the fake backend a reconverge skips a recorded id that is not convergent and applies it where a store has no row. On SQLite and PostgreSQL a reconverge leaves policies written after migration 2, in every place a policy is stored, as they were written, and a policy with both forms on one key does not stop it. The SQLite wire test checks that a GetItem such a policy would allow is still denied after the reconverge, and the PostgreSQL CLI test expects the skip line.
A fresh install seeds no settings rows, so `extenddb settings get control_plane_delay_seconds` printed "not found" and exited 1, and `settings list` left out control_plane_delay_seconds and index_propagation_delay_ms, while the engines and the console use their defaults. A key with no row now prints the storage engine's default (`operations::setting_default`), as the console does. A row under the other name of a renamed key (gsi_propagation_delay_ms) is the row the engine reads, so `get` prints it in place of the default, and `list` adds no default beside it.
The four devtools check scripts use the `X | None` annotations that Python 3.10 added, and three of them call curl to read a server's /health endpoint. CONTRIBUTING now says so where it describes them.
The `extenddb migrate` command that serve, migrate and verify advise carried the config file's path unquoted, so a directory named with a space split it into two arguments and one named with a single quote broke the shell's parse. A config file whose `[storage.sqlite] path` is relative, as `init` writes it, resolves the database against the directory the command runs in, so the advised `--reconverge` run from another directory answered that the catalog is not initialized. The advised command now quotes the config file's path for a POSIX shell. It starts with the backend's location assignment (`EXTENDDB__STORAGE__SQLITE__PATH=<absolute path>`) unless the config file names the store by its absolute location. A test runs the advised command through sh from another directory, with the config file in a directory whose name holds a space and a single quote.
…cannot read The preview showed one line for migration 2 and named only collisions, though migration 2 changes authorization decisions. While migration 2 is pending, the preview and verify now print the number of stored policies it rewrites, the first 20 with their account, principal, policy name and number of keys, and the number of the others. The runner's status keeps no such list, so they read the policies a second time through `CatalogMigrations::pending_changes`, whose default opens the installed backend's management store. The SQLite backend reads them on a read-only connection, as its status does. Its management store opens the database for writing: it sets WAL mode on a rollback-journal database, and its connection, when it is the last one to close, checkpoints the WAL into the database file. A policy document the store could not decode stopped the preview, verify and `migrate --yes` with exit 1 and a count of failed lookups. The error now names the first 5 failed lookups with the account, the principal and the error's text. `ManagementStore` gains `list_policy_documents` and `boundary_document`, which return each document or its decode error. For a backend that implements them, migration 2 reports each document it cannot decode as a NeedsAttention item (exit 5) with its account, principal, policy and decode error, and reads the principal's other policies. Their defaults read `list_policies` and the boundary getters, which fail on such a document as before. The bootstrapper path of `migrate` withholds the resource-tag marker for such a policy as it does for a collision.
verify printed every ledger note on every run, one line per policy that migration 2 rewrote and per index that migration 4 created, for the life of the catalog. It now prints, for each migration with notes, the number of notes and the first 5, and `verify --all-notes` prints every note.
verify returned the status's code whenever it was not 0, so pending migrations with an unreachable data database exited 3, which reads as "run migrate --yes", and that run cannot apply them while the database is unreachable. A failed check now exits 1 on a Behind catalog. The statuses that need a person whatever the checks found keep their codes, 4 and 5.
…isions The SQLite section of catalog 0.0.4 quoted the 0.1.x preview and log lines next to the v1.0 copy paragraph. It now shows the note that v1.0 prints for the backup_items rebuild under migration 1, and labels the 0.1.x output as the 0.1.x binaries'. The resource-tag section said the rewrite keeps every policy's decision. A policy with a dynamodb:ResourceTag key that 0.1.14 stored after its last `migrate --yes` had no effect on 0.1.14, and takes effect after migration 2.
The copy taken before a run's first write used VACUUM INTO, which creates every table and index again, and its time grows faster than the number of tables. The run printed nothing while it copied and held the lock. The run now takes the copy with SQLite's online backup API, on a connection to the database and one to <copy>.partial, both opened for the copy on a blocking thread, then syncs <copy>.partial, renames it and syncs the directory. The backup reads the database file and the committed frames of its WAL in one read transaction, so the copy holds every write committed when that transaction began, whatever another process writes meanwhile, and its time grows with the database's size. A line before the copy names its path and the database's size, and a line after it gives the time it took. sqlx does not expose the backup API, so storage-sqlite depends on libsqlite3-sys directly, at the version sqlx-sqlite already links (0.30.1). Cargo allows one package per links value in a build, so the backup's connections and sqlx's use the same SQLite library. The backup opens connections of its own instead of using the runner's through sqlx's lock_handle(), because the locked handle borrows the connection and cannot be moved to a blocking thread. Only SQLite opens the database file. POSIX advisory locks belong to a process and a file, so closing a descriptor of the database that other code in the process opened releases every lock the process holds on it, the shared lock of the runner's connection included. A process that then closed its own connection would take itself for the last one, checkpoint the WAL and delete it, and the run's later commits would go to the deleted file. SQLite defers closing its own descriptors of a file while another connection of the process holds a lock on it. A checkpoint followed by a copy of the main file opens the database outside SQLite: with a sqlite3 client that opened, read and closed the database during the run, migrate --yes exited 0 and migration 4's commit was lost. Measured on a catalog of 2,000 tables, each with one GSI, one LSI and 3 items, built through the API (82 MB, 18,077 schema objects), with migration 4 pending after --forget 4, on a host with a load average of about 12 on 16 cores: migrate --yes took 19.0 s with VACUUM INTO and 7.8 s with the backup API, which took 0.3 s of it. Both copies pass PRAGMA quick_check and hold the same row counts in every table. On the same catalog, from Python's sqlite3 module at a load average of about 17, VACUUM INTO alone took 38.7 s and the backup API 0.29 s. The copy is created owner-only (0600) before SQLite writes to it, whatever the database's mode. It took the database's mode before, which is 0644 on a catalog that 0.1.x init created and no server opened. A run that was killed after its copy was complete and before it committed anything left the copy, and every later run took another, so a dev server that crashed in a loop piled them up until the free-space check refused. A run now removes each earlier complete copy with the same <from>, the same size and the same bytes as its own copy, and no -wal or -journal file beside it, and names it. The same <from> alone does not show that nothing changed: a reconverge runs bodies again without changing the catalog version, and --forget brings a catalog back to a version it had before. Tests: a copy taken while another connection's read transaction keeps a write in the WAL holds the write, and on Linux the process's POSIX locks on the database, read from /proc/locks, are the same after the copy as before it. The second test fails when the copy opens the database with std::fs::File.
No test pinned that a body's changes and its ledger row commit in one transaction: with COMMIT; BEGIN IMMEDIATE before the ledger insert, every migration test passed. A test hook now fails the ledger row's insert after the body ran. The test stores a policy with a dynamodb:ResourceTag key on a catalog at migration 1, runs migrate with the hook set, and checks that migration 2's rewrite of the policy is gone, that the ledger has migration 1 alone, and that migration 2 is pending.
… one transaction migrate --forget deleted the ledger row and wrote the version row as two autocommit statements, so a failure between them left the row deleted and the version row unchanged. It now runs both in one BEGIN IMMEDIATE transaction, and a failure leaves the database as it was. The test makes the version row's write fail with a trigger and checks that the ledger row is still there.
A refused serve, or migrate --yes on a file that is not a catalog, leaves an empty <db>.lock beside the database. The upgrade manual and ADR-0023 now say that the file stays after every run and server, and why nothing removes it: flock locks the open file, so if one process removed the file while another held it, a third could create and lock a new file of the same name, and the two holders would not exclude each other.
SQLite stores policy documents as text, so one that does not decode as JSON made list_policies fail for the whole principal, and migration 2's preview, verify and migrate --yes stopped with exit 1. The SQLite management store now implements list_policy_documents and boundary_document, which decode each document on its own and return the decode error in its place. Migration 2 then names such a policy with its account, principal, policy name and decode error as an item that needs attention (exit 5), and still reads the principal's other policies and its boundary, so the run names every document it cannot read; it rewrites them all once those are replaced or deleted. PostgreSQL stores the documents as jsonb, which refuses a document that does not decode, so its store keeps the trait's defaults.
A PostgreSQL server ended its process with exit status 1 within one renewal, 15 s, of any error on its registration's connection: a database restart, a failover, pg_terminate_backend, or a pooler that closed the connection. The exit skipped the serve shutdown path, so requests in flight were cut, the workers' shutdown token never fired, and only an atexit handler removed the PID file. A backend now gives the server a ServingRegistrar through its runtime hooks. The registrar reports a lost registration and makes one attempt to register again when asked, and the server handles the rest. While it holds no registration it answers every new request, /health included, with ServiceUnavailable (HTTP 503), which the AWS SDKs retry, and requests in flight finish. It asks the backend to register again at once and then every second, and serves again once it holds a new registration and the catalog's status is the one it started with. When a migration holds the migration lock or waits for the servers to stop, when the status changed, or when 30 s pass without a registration, the server stops as SIGTERM stops it: the listener drains, the workers stop, the PID file is removed, and the process exits 1. On PostgreSQL an attempt opens a new session, takes the serving key with a try, and reads the status under the new key. A migration that holds or waits for the exclusive key makes the try fail at once. The atexit handler that removed the PID file after the old exit is removed.
…ransactions The serving registration and the migration lock are transactions that stay open while their key is held, renewed by SELECT 1 every 15 s. A role or database setting of idle_in_transaction_session_timeout under 15 s ended the session between two renewals, so a server lost its registration 15 s after every start. PostgreSQL 17's transaction_timeout counts from the start of the transaction, and no renewal resets it. Each of the two transactions now sends SET LOCAL idle_in_transaction_session_timeout = 0 after BEGIN, and SET LOCAL transaction_timeout = 0 when server_version_num is 170000 or more. A transaction-mode pooler applies both to the backend the key lives on. The test holds both keys for 32 s, renewed at the production interval, as a role whose idle_in_transaction_session_timeout is 5 s (and whose transaction_timeout is 20 s on PostgreSQL 17 and later), while a plain transaction of the same role left idle is ended by the setting.
…ry once at zero A lock timeout now bounds every wait of a run together. The migration lock computes one deadline when it is asked for: the wait for the migration key, the wait for the serving key and the runner's wait for an earlier migrator's statements each get the time left, and lock_timeout is set to the time left before each key wait. Before this change each of the three waits got the whole timeout, so a run could wait up to three times --lock-timeout. A key that is held when no time is left is not waited for: the run rolls back its lock transaction and exits 6 naming the holders. PostgreSQL reads a lock_timeout of 0 as no limit, so migrate --yes --lock-timeout 0 waited for as long as another migrator or a running server kept its key, and held the migration key meanwhile, so each server started in that time exited 6. SQLite already tries once at zero. The lines that announce a wait print the seconds left, rounded up.
Every runner transaction sets client_connection_check_interval to 1 s where the server supports it, so that a statement whose client is gone ends instead of running on with the transaction's locks held. No test failed without that setting: the migrator tests end the runner's session from the server side, with pg_terminate_backend. The new test runs a code body through a TCP proxy. The body writes a settings row and then runs pg_sleep(60), and the proxy closes both connections while the sleep runs, which is what the database sees when the migrator's process dies. The database ends the session within 10 s, and neither the row nor the ledger row is committed. The proxy moves into the tests' support module, and a test-util accessor gives a test body the runner's catalog connection.
… what to do
When migration 1's post-check found a difference from the v1.0 schema,
the refusal printed the column and index definitions as Rust structures
(Column { data_type: "double precision", not_null: true, default:
Some("1"), ... }), all on one line, and did not say what to do next.
The refusal now gives one line per difference. Each line names the
object and its table, says what the database has and what v1.0 has, and
says what to do: a column's type, nullability, default and collation are
named one by one, followed by the v1.0 definition to change it to; a
missing column or constraint comes with the definition to add, a missing
index with the statement that creates it, an index that is not valid
with the REINDEX that rebuilds it, and a column v1.0 does not have is to
be dropped. The last line says to make each change and run migrate --yes
again.
…timeout The upgrade manual's PostgreSQL section now says what a server does when the database ends the session that holds its key: it answers new requests with ServiceUnavailable while it registers again, serves again once it holds the key with the catalog's status unchanged, and otherwise stops as on SIGTERM and exits 1. The failover note says the server registers again on the new primary. The section also says that the key transactions turn off idle_in_transaction_session_timeout and, on PostgreSQL 17 and later, transaction_timeout, and that PgBouncer's idle_transaction_timeout must be 0 or at least 30 s. It lists the drift refusal migrate --yes can print, and says that one --lock-timeout bounds every wait of a run and that 0 makes one attempt.
On PostgreSQL the advice said "back up both PostgreSQL databases, the catalog database <name> and its data database" whenever the command was given no --data-db, though the catalog records the data database's name in its data_database_name row. CatalogMigrations gains data_store_name, the name of the data store a target resolves to; by default the one the target names. The PostgreSQL backend returns --data-db when given, else the data database resolve reads: the data_database_name row, or the catalog's name without its _catalog suffix. migrate, verify and a serve that the catalog refuses put that name in the target they give backup_advice, which then ends "and the data database <name>". The lookup opens one read-only connection to the catalog and is bounded by 2 s: the name only completes the advice, and an unreachable catalog is reported by the status read that follows, within the 10 s connect timeout.
…ne that cannot check-postgres-catalog-lock ended the registration session and expected the server to exit 1. It now ends the session and expects the server to register again on a new session named "extenddb serve" and go on serving, and a migrate --yes then to exit 6 naming it. A second server, started with --write-pid-file, has its session ended less than 1 s after a renewal while the script takes both keys: it must exit 1, say that it could not register again, and remove its PID file. The script also checks that migrate --yes --lock-timeout 0 exits 6 within 10 s while another session holds the migration lock.
…replaced copies on a stop The backup copies page 1 with the database's WAL mark, so every copy was a WAL database, which cannot be read without a -shm file beside it: a copy moved to read-only storage could not be opened. The copy is now switched to rollback-journal mode before its connection closes, as VACUUM INTO left it. The search for the earlier copies a run replaces opened each candidate under a copy's name. A hard link or a symbolic link to the database under such a name, which restoring a copy with ln can leave, made it open the database outside SQLite, and closing that descriptor released the process's POSIX locks: with a sqlite3 client that opened, read and closed the database after the copy, migrate --yes exited 0 and migration 4's commit was lost. The search now skips a candidate that is a symbolic link, or a hard link to the database, its WAL or its index (same device and inode), before it opens anything. A run that stopped before its first commit removed its own copy and returned its error alone, without the lines naming the earlier copies its copy had replaced: it left no copy, and nothing said that an earlier one had been there. The error now comes with those lines. The earlier copies are still removed as soon as the run's copy is complete, so runs killed before their first commit, as in a crash loop, leave one copy. sqlite3_backup_finish returns the last step's SQLITE_BUSY, so a copy that waited out another connection's lock failed with "database is locked". The error now says that another connection held a lock on the database for 30 s. The partial copy is synced through a handle opened for writing, which FlushFileBuffers needs on Windows; with a read-only handle every copy failed there. Tests: a hard link and a symbolic link under copies' names are neither opened (this process's locks on the database are the same after the search) nor removed; a run that commits nothing names the earlier copy it replaced and leaves no copy; a lock held past the wait gives the new message; the copy's header says rollback journal; and a write that is only in the WAL, which an immutable read of the main file does not see, is in the copy. Each of the first four fails without its change. Runs killed after their copy, one after the other, leave one copy.
…xit status When an attempt to register again was refused because a migration holds the catalog, the supervisor asked for a stop, and the background workers (control-plane transitions, index builds, the metrics flush) went on until the listener had drained. The supervisor now fires the workers' shutdown token with the stop; the listener's drain still lets the requests already running finish. A stop asked for during the drain of a shutdown signal made serve end with exit 1 and a fatal line, though the signal had stopped it. A signal that ends the server's wait first now decides the exit status. The upgrade manual also says that a server serves until the renewal that finds its session gone, so a migrate --yes that was waiting for the servers can start beside it in those seconds.
stored_or_default looked up the name it was asked for first and then the other one, while the engine reads index_propagation_delay_ms first and gsi_propagation_delay_ms only when that row is missing. On a catalog holding both rows, settings get gsi_propagation_delay_ms printed a value the engine does not use. It now looks up the current name first, whichever name it is asked for.
…s leaves no key The server drops an attempt to register again at the bound of its window and at shutdown, and the attempt then relies on ServingRegistration's Drop to close the session that holds the serving key. No test reached that path: without the Drop the serving tests passed. The new test ends the serving session, holds the ledger table so that the next attempt's status read waits, drops the attempt after 3 s, and checks that no "extenddb serve" session holds the key.
…migrator outlasted The refusal for an earlier migrator's statements gave the seconds its own wait had, the part of the run's lock timeout that the waits for the keys left, as if they were the whole timeout: a run with --lock-timeout 6 could report "still after 2 s". It now says that those seconds are the part of the lock timeout its earlier waits left.
The check's line said that every convergent body ran again, and it read only "Ran again 1 baseline". It now also requires migrations 3 and 4 to run again and migration 2 to be skipped.
|
Each inline comment has a reply in its thread. On the smaller notes:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| " NEEDS ATTENTION: account {} {} policy {} left unchanged (its stored document \ | ||
| cannot be decoded: {})", | ||
| u.account_id, u.principal, u.policy_name, u.error |
Summary
Every backend now shares one numbered list of catalog migrations (
crates/storage/src/migrations/registry.rs). For each id, a backend declares the body it runs in each of its stores, a SQL file or code, or a reason the id does not apply to it. Each store records the migrations it applied in its own ledger table,extenddb_migrations, with a checksum per body. The catalog version is now an integer read from the ledgers, andserve,migrateandverifyact on the status the ledgers give.This PR moves SQLite and PostgreSQL onto it. Cassandra and MongoDB keep their current mechanism until the next PR, and the startup policy (an opt-in upgrade at
serve) comes in the one after. The design is recorded indocs/adr/0023-catalog-migrations.md, which supersedes ADR-0003.Refs #346, #495 (moving migration writing to
CONTRIBUTING.md), #636 (serverefuses a catalog that does not match the binary). It covers what #221 proposed for PostgreSQL.The migrations
dynamodb:ResourceTag/<key>toaws:ResourceTag/<key>in stored policies.UpdateTabledeleted before the #259 fix.The first time migration 1 is applied to a store, the runner compares the result with the expected schema inside the same transaction and commits nothing on a difference. The test fixtures hold a catalog from every released schema state of both backends (9 for SQLite, 12 for PostgreSQL), and the tests migrate each one and compare it with a fresh install.
What changes for operators
extenddb migratepreviews what is pending and changes nothing.extenddb migrate --yesapplies it.--list,--reconverge,--forget <id>and--lock-timeoutare new, andverifyreports the same status.servein a production build refuses a catalog that is not current, before it opens the engine, and prints the command that resolves it. Each status has its own exit code, listed in the upgrade manual: 0 current, 1 not a catalog or an error, 3 migrations pending, 4 written by a newer release with a breaking change, 5 a person must act, 6 the lock or a running server was still held. A dev build applies pending migrations at startup.migrate --yesrefuses a store thatinitdid not create, and namesextenddb init.<db>.pre-migration-<from>-to-<to>-<time>) with SQLite's online backup API, writing to a.partialname and renaming it when complete, and checks there is room for it first. It prints the copy's path and the database's size before it copies, and the time the copy took after. The copy is in rollback-journal mode, so it opens without a-shmfile beside it. A run that commits nothing removes its copy, and a run removes an earlier copy with the same bytes as its own, which a run killed before it committed anything leaves, and names it. Files the runner creates, the copy included, are readable by their owner only.vectorextension when--pg-useris given. A run takes an advisory lock, waits up to--lock-timeoutwhile a server is serving the catalog, and then exits 6; one--lock-timeoutbounds every wait of a run, and 0 makes one attempt. Each server holds a shared registration. When the database ends its session (a restart, a failover,pg_terminate_backend, a pooler that closes it), the server answers new requests withServiceUnavailable, which the SDKs retry, while it registers again, and serves again once it holds the registration and the catalog's status is unchanged; when a migration holds the catalog, the status changed, or 30 s pass, it stops its background workers at once, then stops as on SIGTERM, letting the requests already running finish, and exits 1. The transactions that hold the keys turn offidle_in_transaction_session_timeout, andtransaction_timeouton PostgreSQL 17 and later. The commands andservework through PgBouncer in transaction and session mode. When the application role does not own some tables (for example after a restore frompg_dump --no-owner), the preview andverifylist them with the statements that fix it, numbered and with the role that runs each, andmigrate --yesrefuses before it takes the lock.Behaviour that differs from 0.1.14:
dynamodb:ResourceTagkey is no longer rewritten by eachmigrate, nor by--reconverge, which skips migration 2 and says so.migrate --yeswaits for; 0.1.14 servers took none. While the registration is lost, the server answers requests withServiceUnavailable, and it stops with exit 1 when it cannot register again within 30 s.migrate --yesor SQLite dev server writes to one anyway, the status reports it, andmigrate --yes --reconvergerepairs it.servestill exits 1 on a refusal and writes the reason to the log. That changes with the startup policy.devtools/check-released-throughnow runs in the release workflows and fails until the release raisesRELEASED_THROUGHto the last released id (4 for v1.0) and records the released checksums.What changes for contributors
CONTRIBUTING.mdhas a new section, "Adding a catalog migration", with a worked example. Adding migration 5 touches three files: the list, and one body per backend. A checksum test pins every released body, andEXTENDDB_BLESS_EXPECTED=1regenerates the golden dumps.docs/schema/{sqlite,postgres}.txthold the current schema, and CI fails when a fresh install differs from them.CI also gains:
serveandmigrateexclude each other, run once directly and once through PgBouncer;EXTENDDB_TEST_PG_REQUIRED, so the PostgreSQL tests fail instead of skipping when no server is configured.Testing
At the head, with PostgreSQL 16.15 required:
cargo test --workspace, without the backup store crate, which CI tests against its S3 endpoint, and with only the unit tests of the Cassandra crate, whose integration tests need a Cassandra node: 2,487 passed, 0 failed. fmt and clippy pass for the workspace and for each backend's feature set, and the workspace builds on 1.88.0. Each of the 28 commits since the review compiles on its own withcargo check --all-targets.test_resource_tag_migration.pyon SQLite (2 passed), and the CLI lifecycle, container readiness, migrate concurrency and vector catalog migration files on PostgreSQL (33 passed, 1 skipped).Before the review: every job of
integration.ymllocally (pytest 1466 on PostgreSQL and 1473 on SQLite, the--comprehensiverun 335 on each, the Rust integration tests 512 on each of the four backends), and the PostgreSQL migration tests on PostgreSQL 14 and 18.