k8s workers: bound pre-session memory (explicit memory_limit, GOMEMLIMIT, threads at spawn)#895
Merged
Merged
Conversation
…MIT, THREADS at spawn) Worker pods were spawned with no memory configuration, so the worker process sized its base DuckDB off the NODE's /proc/meminfo (sysinfo.AutoMemoryLimit = 75% of node RAM): all pre-session work — DuckLake ATTACH, tenant activation, warmup, the controlDB side connection — ran with a memory_limit potentially far above the pod's cgroup limit, and the Go runtime had no ceiling at all. Combined with memory the DuckDB buffer manager does not track (C++ catalog objects for large DuckLake catalogs, postgres_scanner/libpq full-result buffering of metadata reads), this OOM-killed workers that were designed to run slow but never die (observed twice on 64Gi dev workers during pg_catalog metadata queries over a ~3k-table catalog, and reproducibly during CREATE TABLE streams on 16Gi default workers). spawnWorker now stamps three env vars derived from the pod's resolved resource requests, via workerMemoryHygieneEnv: - DUCKGRES_MEMORY_LIMIT: the same 75%-of-pod value the per-session SET uses (shared helper duckdbMemoryLimitForPodMemory extracted from workerDuckDBLimits so the two cannot disagree), bounding the base DB from process start; - GOMEMLIMIT: 1/8 of the pod, a soft ceiling so Go GC pushes back before the cgroup kill; - DUCKGRES_THREADS: the pod's CPU request (fractional cores round up). The worker consumes the first and third via the existing configresolve env surface; GOMEMLIMIT is read by the Go runtime. Tests: TestWorkerMemoryHygieneEnv (shapes incl. sub-GB formatting and the no-requests fallback), extended TestK8sPool_SpawnWorkerCreatesCorrectPod, and new assert_worker_pod checks in tests/e2e-mw-dev/harness.sh pinned to the e2e pool default (1536Mi -> 1GB / 192MiB / 1).
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: needs review Warnings
|
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
Investigating worker OOMKills (64Gi workers dying mid-
pg_catalogmetadata query over a ~3k-table DuckLake catalog; 16Gi default workers dying reproducibly ~150 statements into a CREATE TABLE stream) surfaced a gap in the worker memory model:ConfigureMainDBfalls back tosysinfo.AutoMemoryLimit()= 75% of the node's/proc/meminfo— so the whole pre-session window (DuckLake ATTACH, tenant activation, warmup,controlDB) runs with a memory_limit sized to the node, not the pod cgroup. The per-sessionSET memory_limit(75% of pod) only lands once a client session exists.GOMEMLIMITanywhere, and DuckDB'smemory_limitdoesn't govern the allocations that actually killed these pods (C++ catalog objects, postgres_scanner/libpq buffering full metadata result sets client-side) — so the implicit 25% headroom absorbs unbounded, non-spillable growth until the cgroup kills the pod.This PR closes the first two gaps (pod-correct sizing from process start + a Go-side ceiling). The untracked catalog/scanner memory itself is upstream work (PostHog/ducklake#25).
What
spawnWorkerstamps three env vars derived from the pod's resolved resource requests (workerMemoryHygieneEnv):DUCKGRES_MEMORY_LIMITduckdbMemoryLimitForPodMemoryhelper extracted fromworkerDuckDBLimits, so spawn-time and session-time sizing cannot disagreeConfigureMainDBGOMEMLIMITDUCKGRES_THREADSNo change to session-time sizing or the one-session-per-worker contract.
Tests
TestWorkerMemoryHygieneEnv— table test over prod-like/dev/sub-GB/no-request shapes (incl. MB formatting and empty-requests fallback).TestK8sPool_SpawnWorkerCreatesCorrectPod— extended with the three env assertions on the default pool shape.assert_worker_podintests/e2e-mw-dev/harness.sh) — asserts the spawned worker pod carries all three, pinned to the e2e pool default (1536Mi →1GB/192MiB/1).Follow-ups (not this PR)
max_temp_directory_size+ emptyDirsizeLimitto bound the spill path.🤖 Generated with Claude Code