andygrove opened a new pull request, #6162: URL: https://github.com/apache/datafusion-comet/pull/6162
## Which issue does this PR close? Part of #4576. ## Rationale for this change Comet's memory pools only see what operators reserve, so when an executor is OOM-killed there is currently no way to tell from its logs how much native memory Comet was really holding. #5934 added the `alloc-accounting` feature to count what the Rust allocator hands out, but it was off by default and only surfaced through tracing, so in practice nobody has the number when they need it. This turns the feature on by default (jemalloc stays opt-in) and logs the allocated bytes next to the pool reservations, so the gap between the two is in every executor log and can be used to size `spark.executor.memoryOverhead`. ## What changes are included in this PR? `alloc-accounting` is now a default feature of the native crate. Building with `--no-default-features --features hdfs-opendal` removes the wrapper. A new JNI call, `Native.getMemoryUsage`, returns the allocator balance, the total reserved across every Comet memory pool (counting a shared pool once), and the number of live pools and plans. It reads only the allocation counter and the pool registry, never an execution context, so it is safe to call from any thread. It copies the pools out of the registry and reads their reservations after releasing the registry lock. That matters because `CometFairMemoryPool` holds its own lock across the JNI acquire from Spark, which Spark can park until another task frees memory, and a finishing task only frees its reservations after `releasePlan` has taken the registry lock to unregister, so reading reservations under the registry lock can deadlock. The existing tracing path in `executePlan` still reads them under the lock. I left that alone here, but it has the same hazard whenever tracing is enabled. The first `CometExecIterator` on an executor starts a single daemon thread that logs one line per interval for the whole executor while native plans are running, and one more after the last plan finishes so that allocation outliving the plans is visible: ``` Comet native memory usage: allocated 5412.3 MiB, reserved 3890.0 MiB (16 native plans, 8 memory pools) ``` It samples on a timer rather than between batches because a plan can spend its whole run inside one `executePlan` call: a plan rooted at a native shuffle writer consumes all of its input before it returns, and a plan fed by native scans parks the task thread until its next batch is ready. The interval is the new `spark.comet.memory.logInterval` (default 10s, 0 disables). Since one log serves every session on the executor, it is read from the executor's SparkConf when the first plan starts rather than from the session conf. The tuning guide gets a new section on using the log to size `spark.executor.memoryOverhead`, and the tracing and memory management contributor guides are updated now that the counter is on by default. The rust-test CI step that used to opt in to the feature now lints and tests the jemalloc arm and the build without the feature, which is the only build where the thread-exit settling test can run. ## How are these changes tested? New native unit tests cover the plan and pool counts (a pool shared by two plans on different threads is counted once), that the allocation figure is reported only when the feature is built in, and that reservations are read outside the registry lock, using a probe pool that records whether the registry lock was held while its reservation was read. In `CometExecIteratorLifecycleSuite`, one test checks through JNI that a created plan is counted until it is released and that the default build reports a nonzero allocation. Another covers the log line format and the rule that the log goes quiet after reporting once past the last plan. -- 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]
