fix(server): raise the default control-plane idle timeout to 15m - #1158
Merged
Conversation
An idle client connection pins a worker, so the control plane closes it after DefaultControlPlaneIdleTimeout. That was 60s, which broke any client pausing between statements; it became 5m. 5m is still short of the gaps clients actually leave. Measured against a production deployment: the connections 5m reaped were not abandoned. The client came back a median of 8s after the reap and at worst 58s, so every observed idle gap fell between 5m00s and 5m58s. Those clients were missing the threshold by seconds and then paying a cold worker respawn to get back -- churn that reclaims nothing. Only ~4% of reaps had no client return at all, so the window is not holding many genuinely abandoned connections either. Raise it to 15m. That clears the observed cluster with margin for a slower run, and costs on the order of one continuously-held worker across the fleet against roughly 150 avoided cold restarts a day. The value deliberately sits at the ceiling the accompanying test enforces: beyond 15m an abandoned connection holds a pinned worker too long to justify as a default. Operators who want tighter density still set --idle-timeout / DUCKGRES_IDLE_TIMEOUT, and a client needing longer asks per connection via duckgres.idle_timeout, bounded by DUCKGRES_CLIENT_IDLE_TIMEOUT_MAX. This reduces how often the reap fires; it is not a correctness fix. A client whose gap can exceed any finite timeout still has to treat the reap as retryable rather than fatal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyKpkM46efTrRcCJNp8Bmd
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
jghoman
approved these changes
Sep 4, 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
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.
Why
An idle client connection pins a worker, so the control plane closes it after
DefaultControlPlaneIdleTimeout. That was 60s (broke any client pausing between statements), then 5m. 5m is still short of the gaps clients actually leave.Measured against a production deployment, the connections 5m was reaping were not abandoned:
Every observed idle gap fell between 5m00s and 5m58s. These clients were missing the threshold by seconds, then paying a cold worker respawn to get back — churn that reclaims nothing. And only ~4% of reaps had no client return at all, so the window isn't holding many genuinely abandoned connections either.
There's a prior data point for the lever working: reaps ran ~850/day at 60s and dropped to ~170/day at 5m.
What
DefaultControlPlaneIdleTimeout5m → 15m.10m would clear the measured cluster, but the worst observed gap is already 5m58s — that leaves ~4 minutes before a slower run puts us back here. 15m buys real margin for about one continuously-held worker fleet-wide, against roughly 150 avoided cold restarts a day.
The value deliberately sits at the ceiling
TestDefaultControlPlaneIdleTimeoutenforces: beyond 15m an abandoned connection holds a pinned worker too long to justify as a default. Raising it further should be a deliberate decision that trips that guard.Escape hatches are unchanged: operators wanting tighter density set
--idle-timeout/DUCKGRES_IDLE_TIMEOUT; a client needing longer asks per connection viaduckgres.idle_timeout, bounded byDUCKGRES_CLIENT_IDLE_TIMEOUT_MAX.Scope
This reduces how often the reap fires — it is not a correctness fix. A client whose gap can exceed any finite timeout still has to treat the reap as retryable rather than fatal. Clients that already request a longer per-connection
duckgres.idle_timeoutare unaffected.Testing
go build ./...andgo vet ./...clean../server/...green, including the updatedTestDefaultControlPlaneIdleTimeout.go test ./...has 4 failing packages (cmd/cache-proxy,tests/controlplane,tests/integration,tests/trino-ducklake-smoke) — verified identical on unmodifiedorigin/mainin this environment, so they're pre-existing/environmental (they need docker/postgres/trino), not introduced here.