viirya commented on PR #6071: URL: https://github.com/apache/datafusion-comet/pull/6071#issuecomment-5782248930
Review follow-up in ffdbbd2df833c99902c44dbc1f436520342badf3: - Removed the decoded-plan cache and its config rather than splitting it into another PR. The existing off/cache-only measurements showed only approximately 0.2%–1.4% elapsed-time reductions, insufficient evidence to justify its retention, eviction, and synchronization complexity. This PR now focuses on physical-plan sharing. - Added private-plan fallback for shared construction failures, explicit shared-task metrics and path assertions, restored default-mode suite coverage, and retained the original limit lifecycle test alongside a separately verified shared-scan case. - Local validation passed: 23 native shared-pipeline tests, one native metrics test, and 176 JVM tests across CometExecSuite, CometExecIteratorLifecycleSuite, and CometTaskMetricsSuite. Rust formatting, Clippy, JVM formatting/style, and suite-registration checks also passed. The Spark 4.1 SQL gate has been requested with `run-spark-4.1-tests`; its result is pending. Still outstanding: sharing-only versus disabled measurements after cache removal; TPC-H/TPC-DS admission and actual-bind coverage with normal defaults preserved apart from enabling sharing; and documenting/validating the execution-state assumptions for admitted DataFusion operators. The targeted positive-path tests deliberately configure eligible inputs and are not evidence of default-workload coverage. The metrics-scaling finding remains open. We plan to address it through partition-specific metrics access in DataFusion, integrate that API, and rerun the matched large-stage/long-lived-task benchmark. The remaining sharing config defaults to false, which limits default-user exposure but does not resolve the performance issue for users who enable it. I updated the PR description to distinguish current validation from historical measurements and outstanding work. -- 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]
