andygrove opened a new pull request, #6360: URL: https://github.com/apache/datafusion-comet/pull/6360
## Which issue does this PR close? Part of #5485. It is the first of the changes promised in the #5634 review before the in-memory cache is turned on by default. ## Rationale for this change `CometDriverPlugin` installs `ArrowCachedBatchSerializer` as `spark.sql.cache.serializer` whenever `spark.comet.exec.inMemoryCache.enabled` is true at startup, even when the application starts with `spark.comet.enabled=false` or `spark.comet.exec.enabled=false`. `spark.sql.cache.serializer` is static, so every cache in such an application is stored in Comet's format but can never be scanned by `CometInMemoryTableScan`. Spark operators read all of it, which is the slow path described in #5485 and in the Limitations section of the in-memory cache guide. Keeping the plugin in `spark.plugins` cluster-wide and switching Comet off per application is a common setup, and @mbutrovich pointed out on #5634 that flipping the default would give every such application Comet's format. #5485 lists this check as its second option. ## What changes are included in this PR? `maybeSetCacheSerializer` now also requires `spark.comet.enabled` and `spark.comet.exec.enabled`. All three keys are read through the existing `getBooleanConf` helper, so an unset key takes its config default. That includes the cache key itself, which was read with a hard-coded `false`, so the plugin follows the default when it changes. A session that starts with either config off and turns it on later keeps Spark's format. `CometExecRule` already records a fallback reason for a relation cached with another serializer. The config's doc string and the in-memory cache guide now say when the plugin installs the serializer. ## How are these changes tested? A new test in `CometInMemoryCacheSuite` calls `maybeSetCacheSerializer` with all three configs on (installed), with Comet off and with native execution off (not installed), and with the keys unset (each follows its config default). It also checks that the driver conf and the `extraConfs` sent to executors agree. With the old condition restored, the Comet-off case fails. I ran the two plugin tests in `CometInMemoryCacheSuite` and all of `CometPluginsSuite` on Spark 4.1 with Scala 2.13 and on Spark 3.4 with Scala 2.12. -- 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]
