andygrove opened a new pull request, #6421:
URL: https://github.com/apache/datafusion-comet/pull/6421
## Which issue does this PR close?
Closes #6420.
## Rationale for this change
Spark only collects `Dataset.observe` metrics recorded inside a cached plan
through an `InMemoryTableScanExec` over it, so replacing that node with
`CometInMemoryTableScanExec` loses them. The issue has the details and a
reproduction.
## What changes are included in this PR?
`CometExecRule` keeps Spark's `InMemoryTableScanExec` for a relation whose
cached plan records observed metrics, and records a fallback reason. The data
is still stored in Comet's format and read through the existing fallback path,
which is what already happens when the native scan is turned off at runtime.
The check, `CometInMemoryTableScanExec.recordsObservedMetrics`, walks the
cached plan the way `CollectMetricsExec.collect` does, through subqueries,
adaptive plans and nested caches. It only runs when the native scan would
otherwise be used.
The in-memory cache guide mentions the exception.
## How are these changes tested?
A new test in `CometInMemoryCacheSuite` nests `observe` and `persist` the
way Spark's `SPARK-35695` test does, with a shuffle in the inner cached plan so
that AQE plans it. It checks `observedMetrics`, the `Observation` API with a
bounded wait so that a regression fails instead of hanging on Spark 3.4, the
fallback reason, that the batches are still in Comet's format, and that a
relation without `observe` keeps the native scan. It fails on `main` with
`Map() did not equal Map("inner_event" -> [100,99], "outer_event" -> [0])`.
Spark's `DataFrameCallbackSuite`, run with Comet's serializer installed,
passes with this change, including both `SPARK-35695` tests.
`CometInMemoryCacheSuite`, `CometInMemoryCacheKryoSuite` and
`CometInMemoryCachePruningSuite` pass on Spark 3.4, 3.5, 4.0, 4.1 and 4.2, and
`CometExecSuite` passes on 4.1.
--
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]
