andygrove commented on issue #4576:
URL:
https://github.com/apache/datafusion-comet/issues/4576#issuecomment-5707080086
This answers the precondition set when #4582 was closed:
> Whether to enforce on that number at all is a decision I'd rather make
after seeing how well it tracks RSS on real workloads, rather than before.
Measured, and the answer is no: `current_balance()` is not a signal an OOM
guard can be built on.
## Method
TPC-H SF100, 2 executors x 8 cores, Spark 4.1.1, 16g off-heap, **default
allocator** (`COMET_FEATURES=alloc-accounting`, no jemalloc, since that is what
stock Comet ships). I added a kernel RSS counter (`/proc/self/status` VmRSS)
emitted at the same trace anchor as `native_allocated`, 79 microseconds apart,
so each sample pairs the two.
## Result
| | executor 0 | executor 1 |
| --- | --- | --- |
| paired samples | 4508 | 4574 |
| balance p50 / max | 274 MB / 2100 MB | 278 MB / 2116 MB |
| RSS p50 / max | 7148 MB / 8612 MB | 6658 MB / 7849 MB |
| level correlation | +0.187 | +0.226 |
| delta correlation | +0.436 | +0.423 |
| balance leads RSS by 1 / 2 / 5 / 10 samples | +0.00 / -0.04 / -0.02 /
-0.01 | -0.01 / -0.03 / -0.00 / -0.01 |
| RSS - balance, p10 to p90 spread | 3212 MB | 2097 MB |
Two conclusions that do not depend on how much of RSS is Comet's:
1. **The balance never leads RSS.** Correlation at every lead horizon is
indistinguishable from zero. A guard exists to fire before the kernel does, and
this signal carries no warning.
2. **The offset wanders by more than the signal's entire range.** The gap
between RSS and the balance moves 2 to 3 GB, while the balance itself only ever
spans 0 to 2.1 GB. Any threshold placed on the balance is swamped by drift in
the quantity it is meant to predict.
This is the same thing the closing review of #4582 identified analytically
("the tracked balance is layout bytes, not RSS ... that gap is unbounded and
always in the dangerous direction"), now with numbers.
**Caveat, stated plainly:** RSS here is dominated by the 16 GB JVM heap,
which moves for GC reasons Comet does not control. The balance may well track
*native* memory closely. But a guard has to predict the *container* total,
because that is what gets killed, and it cannot.
Related: in the same session I ran a Comet-free control at identical
configuration. It measured *higher* RSS (p50 9.28 GB) than the Comet run (p50
6.7 to 7.1 GB), which is why I retracted an earlier claim in this thread that
RSS corroborated a large Comet-attributable gap. It does not.
## What I think follows
A guard should read the kernel's number directly rather than infer it:
- cgroup v2 `memory.current` against `memory.max`, which is what the OOM
killer compares, and inside a Kubernetes pod the container's cgroup is
namespaced so these appear at `/sys/fs/cgroup/`.
- cgroup v1 `memory.usage_in_bytes` / `memory.limit_in_bytes` as a fallback.
- `/proc/self/status` VmRSS where no cgroup limit is readable.
That removes the allocator wrapper from the guard's critical path entirely,
which also removes the `LOCAL_DRIFT` leak and the layout-bytes objection from
the #4582 review. `AccountingAllocator` remains valuable as observability,
which is what that review proposed and what #5934 shipped.
One implementation note for whoever builds this. I hardcoded
`/sys/fs/cgroup/memory.current` in my instrumentation and it silently produced
zero samples, because on a non-containerised host the process is in a subtree
(`/user.slice/user-1000.slice/session-852.scope`). The path has to be resolved
from `/proc/self/cgroup`, and the failure mode is silence rather than an error,
so it needs an explicit liveness check.
@comphead, this bears on #5666: it gates growth on the allocator balance and
forces the `unbounded` pool while doing so. The data above suggests the balance
will not tell you when you are near a container limit, and the forced
`unbounded` pool also stops Spark's `TaskMemoryManager` from seeing Comet's
usage. Worth considering the cgroup signal instead before investing further in
that direction.
Traces are retained if anyone wants a different cut of this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
