Allow mid-session SET of duckgres.worker_ttl - #1054
Open
fuziontech wants to merge 1 commit into
Open
Conversation
duckgres.worker_ttl (how long a worker stays hot-idle/warm after its last session ends) was connect-time only: clients that cannot set libpq startup options had no way to extend warm retention, so their worker was reaped at the default TTL whenever the connection went idle. Add SET/SHOW/RESET support on all three protocol paths (simple, batch, extended), mirroring duckgres.s3_cache: - the transpiler intercepts and validates the GUC (22023 on a bad duration, never forwarded to DuckDB, never intercepted inside a multi-statement batch) - the connection layer applies it through a control-plane hook that updates the bound worker's pool-side hot-idle TTL, gated on DUCKGRES_K8S_ALLOW_CLIENT_WORKER_PROFILE and clamped to DUCKGRES_K8S_WORKER_MAX_TTL exactly like the startup option - session state only flips after the apply succeeds, so SHOW never reports a TTL the worker won't park with; SET acquires a worker on lazily-activated connections and the override is re-applied across exploratory-tier worker switches - standalone/process backends get session-state-only SET/SHOW (they have no per-worker hot-idle TTL)
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. |
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.
Why
On the remote/K8s backend, a worker whose last session ends is parked
hot_idle(warm, quickly reusable) and retired once its TTL expires — 1 minute by default. The only way to extend that was the-c duckgres.worker_ttl=...connect-time startup option, which many clients (BI tools, poolers) can't set. Their workers are reaped at the default TTL whenever a connection goes idle, so every return visit pays a cold worker spawn.What
SET duckgres.worker_ttl = '20m'/SHOW/RESETnow work mid-session, on all three protocol paths (simple, split-batch, extended):duckgres.s3_cache— validated with 22023 on a bad duration (without echoing client input), never forwarded to DuckDB, never intercepted inside a multi-statement batch.ttl_minutes, which is exactly what both reapers (leader janitor, per-CP fallback) read. Session state flips only after a successful apply, so SHOW never reports a TTL the worker won't park with; a clamped value is stored clamped.DUCKGRES_K8S_ALLOW_CLIENT_WORKER_PROFILE(SET rejected with 22023 when off) and clamped toDUCKGRES_K8S_WORKER_MAX_TTL.SHOW duckgres.worker_ttlreports the session override, else the bound worker's current park TTL, else the connect-time baseline (startup GUC > org default > deployment default > built-in 1m).Notes / limitations
ttl_minutestruncates), so sub-minute overrides round down — documented in the README.Test plan
transpiler/worker_ttl_test.go: SET/SHOW/RESET interception, normalization, 22023 rejections (no echo), multi-statement guard.server/worker_ttl_test.go: simple/batch/extended apply paths, lazy activation on SET, failure-keeps-state, gate 22023 passthrough, clamp reporting, escalation re-apply + failure semantics, state-only without the capability.controlplane/worker_ttl_test.go+k8s_pool_worker_ttl_test.go: pid→worker routing, gate/clamp hook, baseline resolution, pool SetWorkerTTL/WorkerTTL, and override persisted asttl_minutesat park.just lintclean; full unit suites pass in both build flavors (-tags kubernetesand without).controlplane/adminPostgres-container tests fail in this environment (missing docker-compose) — pre-existing on main, unrelated.