andygrove opened a new issue, #6050: URL: https://github.com/apache/datafusion-comet/issues/6050
### What is the problem the feature request solves? `CometDriverPlugin` adjusts `spark.executor.memoryOverhead` only when Comet runs in on-heap mode. `CometSparkSessionExtensions.shouldOverrideMemoryConf` requires `!offHeapMode`, and `getCometMemoryOverheadInMiB` returns `0` in off-heap mode, so the configuration we recommend to everyone — off-heap memory enabled — gets no adjustment at all. The plugin logs "Comet is running in unified memory mode and sharing off-heap memory with Spark" and leaves the overhead alone. That is only half the picture for container sizing. Reservations Comet's operators make are charged against `spark.memory.offHeap.size`, which the cluster manager already includes in the container size, so those have room. Everything Comet allocates *without* reserving it does not: per-batch working memory in expression kernels and Arrow array builders, decompression buffers and Parquet reader structures, object store request buffers and the tokio runtime, Comet's JVM-side Arrow buffers, and allocator overhead (padding, size-class rounding, fragmentation, retained pages). Those come from the Rust global allocator and live in the native heap. Nothing in the container sizing accounts for them, and `spark.executor.memoryOverhead` is the only slack the container has — slack the JVM's own non-heap usage is already drawing on. See [What the container sees](https://datafusion.apache.org/comet/contributor-guide/memory_management.html#what-the-container-sees) in the memory management guide for the full accounting. ### Describe the potential solution Three options, in increasing order of risk: 1. **Warn only.** At driver init, when off-heap mode is enabled and `spark.executor.memoryOverhead` was not explicitly set, log a warning pointing at the tuning guide. No resource sizing changes. This mirrors `CometDriverPlugin.warnIfKryoRegistratorMissing`, which already handles a "set this before the context starts or you find out later" case in the same file. 2. **Auto-adjust, opt-in.** Extend the existing bump to off-heap mode behind a config that defaults to off. 3. **Auto-adjust by default.** Make `getCometMemoryOverheadInMiB` return a non-zero value in off-heap mode. This would silently grow every Comet user's container on Kubernetes and YARN, changing bin-packing, quotas, and capacity planning for clusters that are fine today. `spark.comet.memoryOverhead` is currently in `CATEGORY_TESTING` and documented as on-heap only, so it would need re-scoping and user-facing documentation as well. Option 1 looks like the right starting point. The headroom actually needed is workload-dependent, and overshooting it is a scheduling regression rather than a safe default. ### Additional context Worth verifying before building anything on options 2 or 3, because it may also mean the existing on-heap bump does not do what the code intends: `SparkContext` materializes the default `ResourceProfile` as the argument to `PluginContainer(this, _resourceProfileManager.defaultResourceProfile.resources)`, before any plugin's `init` runs, and Kubernetes' `BasicExecutorFeatureStep` and YARN's allocator size executors from the `ResourceProfile` rather than re-reading the conf. If that ordering holds on the Spark versions Comet supports, the plugin's mutation updates `sc.conf` — which is all `CometPluginsSuite` asserts, and it runs locally where nothing sizes a container — without changing the pod or container spec. The documentation side is handled separately: the tuning guide now has a "Configuring Executor Memory Overhead" section and the `spark-shell` / `spark-submit` examples set `spark.executor.memoryOverhead=2g`. -- 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]
