Pool acquire deadline, pool-sharing auxiliary stores, migration advisory locks - #7
Merged
Merged
Conversation
…orever An exhausted pool (or a leaked lease) previously blocked every caller indefinitely in acquire's wait loop. Acquisition now waits at most 30 seconds and then fails with a connection error naming the lease count, so saturation surfaces as a diagnosable error instead of a hang. Claude-Session: https://claude.ai/code/session_01Wq5AdnNWVKWRTS34g6A3Bu
… snapshot stores The Postgres and MySQL auxiliary stores each held a dedicated Arc<Mutex<connection>> - one serialized connection apiece, separate from the event store's pool. They now run on ConnectionPool internally (constructed from ConnectionPool::single for the existing constructors, preserving behaviour), and the event stores expose checkpoint_store, idempotency_store, and snapshot_store accessors (plus _with_table_name variants) that share the event store's pool, so one pooled connection set serves the whole store family. Live-DB tests (env-gated) run the checkpoint, idempotency, and snapshot contracts through the pool-sharing accessors on both backends. Claude-Session: https://claude.ai/code/session_01Wq5AdnNWVKWRTS34g6A3Bu
Concurrent migration runs (multiple stores initializing schemas at once) raced on CREATE TABLE IF NOT EXISTS and the applied-migrations bookkeeping; on a fresh database this reliably failed parallel live tests with Postgres pg_type unique violations, on main as well as this branch. run_postgres now holds pg_advisory_lock and run_mysql GET_LOCK for the whole migration pass. SQLite is unchanged: its per-store connection is already serialized behind a mutex. Verified with three consecutive fully parallel test-suite runs against freshly wiped Postgres and MySQL containers. Claude-Session: https://claude.ai/code/session_01Wq5AdnNWVKWRTS34g6A3Bu
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
2 participants
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.
Next batch from the repo review (items #6 and #8), plus a pre-existing concurrency bug the new live tests exposed.
Changes
ConnectionPool::acquirewaited forever on an exhausted pool. It now fails after 30s with a connection error naming the lease count (N of M connections leased), so saturation is diagnosable instead of a hang. Regression test via a test-only timeout override.Postgres/MySql{Checkpoint,Idempotency,Snapshot}Storeeach held a dedicatedArc<Mutex<connection>>. They now run onConnectionPoolinternally (existing constructors keep identical single-connection behaviour viaConnectionPool::single), and the event stores gainedcheckpoint_store(),idempotency_store(),snapshot_store()accessors (+_with_table_namevariants) that share the event store's pool — one pooled connection set for the whole store family. Reads go throughpool.read(stale-connection retry), writes throughpool.write.initialize_schemaruns raced onCREATE TABLE IF NOT EXISTSand the applied-migrations bookkeeping. Against a fresh database this reliably failed parallel live tests — on main too (4 failures), so it's pre-existing, surfaced by the new tests.run_postgresnow wraps the pass inpg_advisory_lock,run_mysqlinGET_LOCK; SQLite unchanged (already serialized per store).Verification
test_postgres_pool_sharing_auxiliary_stores,test_mysql_pool_sharing_auxiliary_stores)--all-targets --all-features -D warnings, 47 lib + 67 framework + 28 doc tests green offlinehttps://claude.ai/code/session_01Wq5AdnNWVKWRTS34g6A3Bu