persist: pre-warm the consensus connection pool - #38053
Closed
jubrad wants to merge 2 commits into
Closed
Conversation
Upgrade deadpool 0.9.5 -> 0.12.3 and deadpool-postgres 0.10.3 -> 0.14.1 to get access to Pool::resize, and wire PostgresClient::get_connection to apply a changed connection_pool_max_size knob to the live pool on the next acquire. persist_consensus_connection_pool_max_size and pg_timestamp_oracle_connection_pool_max_size previously required a process restart to take effect, which made the pool cap unusable as an operational lever during CRDB maintenance or incidents. Also export a new *_postgres_connpool_waiting gauge. Note a monitoring behavior change that comes with the deadpool upgrade: available is now reported as a non-negative count, and acquires queued on an exhausted pool (previously visible as negative available) are reported on the new waiting gauge instead. The deadpool API migration itself: the Manager trait is natively async, recycle takes a Metrics argument, and pre_recycle hook errors are constructed with HookError::message. The TTL-culling semantics of the pre_recycle hook are unchanged. deadpool 0.12 requires lazy_static 1.5, which moves the duplicate-spin skip in deny.toml from 0.5.2 to 0.9.9. Adds a pool_resize_applies_on_acquire test in mz-postgres-client that verifies grow and shrink both apply on acquire. It opts in via the same MZ_PERSIST_EXTERNAL_STORAGE_TEST_POSTGRES_URL variable as the persist external-storage tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a connection_pool_min_idle knob to mz_postgres_client, wired to a new persist_consensus_connection_pool_min_idle dyncfg (default 0, disabled). When set, a background task tops the pool up to that many idle connections by briefly holding that many connections at once and returning them, forcing creation of any shortfall off the acquire path. During a CRDB rolling restart, each node drain closes its share of the pooled connections. The pool discards them silently on recycle and recreates on demand, so the first burst of acquires after a drain pays TCP+TLS+auth setup while demand queues behind the pool cap. Keeping an idle floor rebuilds that buffer in the background instead. The top-up task skips any tick where acquires are already queued, so it never competes with real traffic while the pool is saturated, and its acquires use a short timeout so a mistimed tick cannot stall. Also add a pool-exhaustion workflow to test/crdb-restarts that rolls a 4-node CRDB cluster behind an haproxy round-robin balancer (matching how a cloud load balancer spreads pool connections across nodes) under a multi-table insert load, scraping the connection pool metrics and reporting queued waiters per drain window. With a scaled-down pool cap, rolling drains reproduce waiter pileups an order of magnitude above steady-state noise, and enabling min_idle suppressed them in initial A/B runs (small sample, high run-to-run variance). The new test in mz-postgres-client verifies the idle floor is rebuilt without acquire traffic, opted in via the same environment variable as the persist external-storage tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Motivation
Stacked on #38050 (deadpool upgrade, live pool resize,
waitinggauge) — review that first; this PR's diff includes it until it merges.During a CRDB rolling upgrade, each node drain closes its share of the persist consensus pool's connections. Deadpool discards them silently on recycle and recreates on demand, so the first burst of acquires after a drain pays TCP+TLS+auth while demand queues behind the pool cap. We observed multi-second acquire stalls and deep waiter queues during a recent production CRDB upgrade.
Changes
connection_pool_min_idleonPostgresClientKnobs(trait default 0), wired to new dyncfgpersist_consensus_connection_pool_min_idle(default 0 = disabled, applies live).PostgresClient: each second, if idle < floor and no acquires are queued, briefly holdmin_idleconnections concurrently (forcing creation of the shortfall) and return them all as idle. The waiting-check guard plus a 100ms acquire timeout guarantee it never deepens a queue on a saturated pool. Task spawns lazily on first acquire, aborts on client drop.pool-exhaustionworkflow intest/crdb-restarts: 4-node CRDB behind an haproxy round-robin TCP balancer (a shared DNS alias does not spread a pool's connections — the client sticks to one resolved address; the balancer matches cloud LB behavior), multi-table insert load, scripted drain+restart roll, per-drain-window report of queued waiters / connection creation / mean acquire latency from environmentd's metrics.Validation
With
--insert-threads 32 --pool-max-size 10(cap scaled down to make exhaustion locally reachable), max queued waiters per run across three drain windows, steady-state noise ~20–25:min_idle=0)min_idle=5)The baseline tail event (282, >10x noise) did not occur in either pre-warm run. Sample is small and run-to-run variance is high (the tail event hit 1 of 6 baseline drain windows), so this is directional, not conclusive. The harness makes more runs cheap; happy to accumulate samples before undraft.
Tests
pool_prewarm_maintains_idle_floorinmz-postgres-client(opt-in viaMZ_PERSIST_EXTERNAL_STORAGE_TEST_POSTGRES_URL) verifies the idle floor is rebuilt without acquire traffic.UNINTERESTING_SYSTEM_PARAMETERSand the parallel-workload toggle list.Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.🤖 Generated with Claude Code