Repository navigation
fix(sqlite): serialize management store writes through the engine write lock - #340
Merged
Merged
Conversation
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
September 11, 2026 14:25
8 tasks done
ssbytedev
pushed a commit
to ssbytedev/extenddb
that referenced
this pull request
Sep 13, 2026
…xtendDB#344) * ci(integration): restore the indentation that stops the workflow loading Commit 8d3d585 (ExtendDB#312) moved four lines in integration.yml one column left: two comment lines and the two "run: sudo journalctl" lines in the syslog dump steps of run-rust-integration and run-rust-integration-postgres-novector. The run key no longer aligns with its sibling if and name keys, so the file is not valid YAML. GitHub cannot load it, and since 10:41 UTC on 2026-09-11 every run of this workflow is a zero-job failure named by the file path instead of "Integration Tests", for pull_request, merge_group and push alike. Branch protection on main requires the integration status. A status that can never be reported leaves every pull request opened since then BLOCKED with all other checks green (ExtendDB#341, ExtendDB#340, ExtendDB#343 at the time of writing), and the merge queue cannot advance. This restores the four lines to their previous indentation. Nothing else changes. Verified by parsing the file before and after: the main version fails with "while parsing a block collection", the fixed version loads with the seven expected jobs. * chore(licenses): regenerate MongoDB notices for workspace version 0.1.11 SOFTWARE-LICENSE-NOTICES-MONGODB.html was generated on the ExtendDB#331 branch, which was cut before ExtendDB#332 bumped the workspace to 0.1.11. The ten drifted lines are the workspace's own crates at 0.1.10; no third-party dependency or license changed. The Licenses workflow's "MongoDB notices must be current" step fails on main since the ExtendDB#331 merge. Signed-off-by: Lee Hannigan <lhnng@amazon.com> (cherry picked from commit 5268480) * ci(licenses): run the notices check on every pull request and merge group The notices-current job becomes a required status check. A required check whose workflow is path-filtered never reports on a pull request outside its paths, and that pull request can never merge, so the pull_request and merge_group triggers lose their path filters. The job takes under three minutes. The push trigger keeps its filter since it gates nothing. Signed-off-by: Lee Hannigan <lhnng@amazon.com> (cherry picked from commit 35f42a2) --------- Signed-off-by: Lee Hannigan <lhnng@amazon.com>
… the engine write lock SQLite permits one writer per database file. The engine serializes its own writers with an in-process write_lock (store.rs, design decision D1) so they never contend at the file level, and #321 moved every remaining engine writer inside it after measuring this failure class in August. The management stores (users, access keys, policies, groups, roles, accounts, admin, settings, metrics) were left outside the lock. They write through the second sqlx pool that lib.rs opens on the same file, as plain autocommit statements. When one of those writes arrives while another connection holds the file write lock for longer than the 5 second busy_timeout, SQLite returns SQLITE_BUSY ("database is locked") and the management handler answers HTTP 500. The integration suite runs pytest with parallel workers against one server, so management writes from one worker overlap data-plane writes from another. The run-integration-sqlite job hit this three times this week (runs 34507896936, 34518061214, and the private mirror), each time as a 500 from PUT user policy in test_auth_integration.py::TestIBAC::test_readonly_user_denied_write, with "put_policy: ... (code: 5) database is locked" in the server journal. This change shares the engine's write_lock into SqliteCatalogStore through with_encryption_key, adds a lock_writes helper, and takes the lock around every write statement or transaction in the catalog, admin, and management stores (lock first, then begin, released with the statement; reads never take it). The credential store has no write statements and is unchanged. The GSI propagation worker already held the lock; it appeared in the journal as a victim of the unlocked management write, not as a second offender. The out-of-process CLI constructors (settings, doctor) keep a fresh lock since no engine exists there. Init-time bootstrap is now the only deliberate exclusion, and the store.rs module doc says so. Tests: seven new cases in d1_write_lock_tests (policy put, user create, access key create, account create, group create, admin create, set_setting) each prove the write blocks while the engine lock is held and completes after release, using the existing assert_serialized harness. Signed-off-by: Lee Hannigan <lhnng@amazon.com>
LeeroyHannigan
force-pushed
the
fix/sqlite-management-write-lock
branch
from
September 14, 2026 13:28
f82ce56 to
f7f14d4
Compare
robinnsc
approved these changes
Sep 19, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Routes every management and credential-plane write in the SQLite backend through the engine's existing
write_lock, so the two connection pools that open the same database file never contend for the SQLite file write lock.SqliteCatalogStore::with_encryption_keytakes the engine'sArc<Mutex<()>>; the server wiring inlib.rspasses the one instance the engine already uses, so engine writers and management writers queue on a single lock.lock_writes()helper acquires it. Every write statement or transaction in the catalog store (set_setting, metrics insert and prune, failed-login bookkeeping), the admin store, and the management stores (users, access keys, policies, groups, roles, accounts, sessions) takes the lock before the statement orbeginand releases it with the statement. Reads never take it.settings,doctor) keep a fresh lock, since no engine exists in that process.store.rsmodule doc now lists init-time bootstrap as the only deliberate exclusion from the lock.d1_write_lock_tests, one per store family, each proving the write blocks while the engine lock is held and completes after release, using theassert_serializedharness introduced by fix(sqlite): serialize all writers through the engine write lock #321.Why
run-integration-sqlitefails intermittently with one test out of about 980 returning HTTP 500 where 204 was expected. Three occurrences this week, all the same assertion intests/test_auth_integration.py::TestIBAC::test_readonly_user_denied_writeonput_user_policy:The server journal from the failed run names the cause at the same second:
SQLite permits one writer per file. The engine serializes its writers with the in-process
write_lock(design decision D1 instore.rs), and #321 moved every remaining engine writer inside it after measuring this same failure in August. It left the management stores outside and said so in the module doc: they write through the second sqlx pool inlib.rs, which for a file-backed database opens the same file, as plain autocommit statements with no lock. When such a write arrives while another connection has held the file write lock past the 5 secondbusy_timeout, SQLite returnsSQLITE_BUSYand the management handler maps it to 500. The integration suite runs pytest with parallel workers against one server, so management writes from one worker overlap data-plane writes from another, which is the window.Both upstream occurrences postdate #321, and neither PR they failed on touches the SQLite backend or the management store.
Testing done
Unit and lint gates on the branch:
Failing-first check: with the lock call removed from
put_policy,management_put_policy_waits_for_the_write_lockfails; restored, it passes.Integration, reproducing the CI job locally (release build with
--features sqlite,extenddb init --backend sqlite,serve --foreground --write-pid-file, then the CI invocationdevtools/run-tests --extenddb --pytest --comprehensive --parallel --filter "not import_export"):tests/test_gsi_async_queue_sqlite.pyemits five "extenddb stop did not exit within 30s" warnings; the same file against a main-built binary emits the same five in the same 274 seconds, so that is existing harness behavior and not introduced here.Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-diskformat, or public CLI surface, an RFC has been accepted or is linked
below. Otherwise, an ADR captures the decision (link below).
ADR / RFC: n/a. Applies the existing design decision D1 recorded in the
store.rsmodule doc (every SQLite writer holds the engine write lock) to the writers #321 left outside it. No wire, trait, auth, on-disk, or CLI change.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.