Repository navigation
examples/flight: add OomGuard allocator-level OOM circuit breaker to sql_server - #22834
avantgardnerio wants to merge 8 commits into
Conversation
Wraps the global allocator with `OomGuard`, a `GlobalAlloc` wrapper that converts a process-wide OOM into a per-query panic *before* the kernel SIGKILLs the process. Demonstrates the pattern against the existing flight sql_server example so a Houzz-shape groupby that previously OOM-killed the server now surfaces as a per-query `ResourceExhausted`. - `oom_guard.rs`: thread-stamped balance bank with amortized atomic settle. Resident probe via `/proc/self/statm` (no allocator-specific dependency). Cgroup v2 / v1 limit discovery. - `sql_server.rs`: wraps MiMalloc with OomGuard, spawns the balance poll, and `catch_unwind`s overdraft panics around `df.collect()`. - `main.rs`: manual tokio runtime with `on_thread_start` to stamp worker threads; installs `alloc_error_hook` as the safe seam to originate `panic_any` from outside the allocator. - `.cargo/config.toml`: `RUSTC_BOOTSTRAP=1` for unstable `alloc_error_hook` (tracking rust-lang/rust#51245). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alamb
left a comment
There was a problem hiding this comment.
Thanks @avantgardnerio -- is there anything DataFusion specific about this example? It looks like it doesn't use a memory pool at all. I thought your solution hooked the allocator into the memory pool infrastructure -- this seems like it is even deeper and hooks the allocator itself
If that is the case, perhaps it would be better as a standalone crate?
| //! - `sql_server` | ||
| //! (file: sql_server.rs, desc: Standalone SQL server for JDBC clients) | ||
| //! | ||
| //! ## OomGuard |
There was a problem hiding this comment.
Rather than extending an existing example, what would you think about making this its own example ? That might keep the basic arrow-flight example slimpler
It does not. And at this point I have an honest question: what is the We've established it doesn't prevent OOMs, and that it's not easy to keep accurate. So cost=high, benefit=low - Is there a reason for tracking in-flight batches I'm not seeing? |
acknowledged |
Which issue does this PR close?
Not closing an issue directly. This is a follow-up to the
MemoryPoolaccounting discussion (#22723, #22758) — demonstrates an alternative approach.Instead of catching
MemoryPooldivergences from allocations, this uses theflight_sql_serverexampleto demonstrate how downstream applications can install a custom allocator to abort queries rather
than OOMing when
MemoryTrackerunder-reports.Rationale for this change
MemoryPoolis voluntary. Operators that don't calltry_grow—in-flight batches,
arrow-rowdecode buffers, Arrow kernel scratchspace, allocator slab overhead — allocate freely past the declared
budget. On a process with a hard memory ceiling (k8s pod cgroup,
ulimit), the result is an OS-level kill that takes down every
concurrent query, not just the offender.
This PR adds
OomGuard, aGlobalAllocwrapper. Implementation details:BALANCEatomic.stats.residentevery 10msand syncs
BALANCE = threshold - resident, catching slab pagesthat the kernel sees but
GlobalAllocdoesn't.BALANCEgoes negative on a stamped query worker, thewrapper returns
NULLfromalloc.alloc_error_hookoriginatespanic_any(OomGuardPanic)froma safe-Rust frame outside the allocator.
df.collect(), gets caught in theexample, and is converted to
Status::resource_exhausted. Thequery dies; the server survives.
A/B demo on the same 1GiB cgroup:
OOM_GUARD=off: client sees broken pipe, server SIGKILLed.OOM_GUARDon (default): client seesResourceExhausted,follow-up
SELECT 1against the same server returns immediately.What changes are included in this PR?
Everything is scoped to
datafusion-examples/examples/flight/:oom_guard.rs(new) — theGlobalAllocwrapper, per-threaddrift settling, jemalloc-stats-based poll,
cgroup-v2/v1 memory-limit discovery.
sql_server.rs— wraps the global allocator withOomGuard<Jemalloc>, arms viaOOM_GUARDenv var withconfigurable headroom (
OOM_GUARD_HEADROOM, default 5%), wrapsdf.collect()incatch_unwindto convertOomGuardPanictoa
Status::resource_exhaustedresponse.main.rs— installs thealloc_error_hook, installs a globalpanic hook that logs
OomGuardPanicdistinctly.cargo/config.toml(new) — setsRUSTC_BOOTSTRAP=1so# rust-lang/rust#51245)builds on stable.
README.md(new) — A/B walkthrough.Are these changes tested?
Manually via the A/B in the README. No automated tests
added: the example's behavior depends on a cgroup limit being in
effect, which doesn't fit the existing test harness.
Are there any user-facing changes?
Only to the
flightexample. No core / library API surface changed.Caveats
alloc_error_hookis currently unstable. The workspace.cargo/config.tomlsetsRUSTC_BOOTSTRAP=1to compile it on astable toolchain.
allocator-agnostic by reading cgroup's
memory.currentinstead isa straightforward follow-up if there's interest.