comphead opened a new pull request, #6698: URL: https://github.com/apache/datafusion-comet/pull/6698
## Which issue does this PR close? Closes #6467. ## Rationale for this change On the nested TPC-H benchmark in #6467, Comet lost q21 to Spark because its spills were uncompressed. DataFusion 55.1 defaults `datafusion.execution.spill_compression` to `uncompressed`, while Spark compresses its own spill files with `lz4` by default. With `spark.comet.datafusion.execution.spill_compression=lz4_frame`, Comet's q21 went from 380 s to 250 s, against 367 s for Spark (Spark 4, Iceberg, SF1000, numbers from the issue). Comet forwards `spark.comet.datafusion.*` keys to DataFusion only when the testing flag `spark.comet.exec.respectDataFusionConfigs` is true. That flag forwards every such key, and the [versioning policy](https://datafusion.apache.org/comet/about/versioning_policy.html#testing-and-internal-configurations-are-exempt) says a testing key must not be the only way to reach a behavior that production users need. ## What changes are included in this PR? - A new config, `spark.comet.exec.allowedDataFusionConfigs`, in the tuning category. It is a comma-separated list of full `spark.comet.datafusion.*` keys that are passed to native execution even when `spark.comet.exec.respectDataFusionConfigs` is false. The default list is `spark.comet.datafusion.execution.spill_compression`. Spill compression itself still defaults to `uncompressed`, so nothing changes unless that key is set. - `CometExecIterator.serializeCometSQLConfs` applies the list. The native side already forwards every `spark.comet.datafusion.*` key it receives, so only a comment in `jni_api.rs` changes, which no longer calls that pass-through a testing escape hatch. - Docs: a "Compressing Spill Files" section on the memory tuning page, and a "Nested and Wide Data" section in the tuning guide overview that links the settings that matter for such data. The configuration reference picks up the new config from its description. ## How are these changes tested? A new test in `CometExecSuite` checks that, with `respectDataFusionConfigs` off, the default list forwards the spill compression key and drops another `spark.comet.datafusion.*` key, and that a configured list forwards the keys it names. Forwarding every key with `respectDataFusionConfigs=true` is already covered by existing tests, such as the skip partial aggregation tests in `CometAggregateSuite`, which only pass when a forwarded DataFusion threshold takes effect. -- 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]
