viirya commented on PR #6883: URL: https://github.com/apache/datafusion-comet/pull/6883#issuecomment-6547596750
Thanks @andygrove, I read through #7289 and #7288. They share the driver side, but I don't think an in-process Ballista cluster would replace what this PR does. The two run a query in different places. **Where they overlap.** Both run Comet's native blocks from the converted plan in place of Spark's tasks, through a leaf that `CometRule` substitutes, and both refuse blocks that call back into the JVM. Once local execution admits exchanges, it will also need what your stage-graph builder does: split the plan into blocks at Comet's exchanges and map each exchange's partitioning. That part could be shared, and I'd be glad to build on yours rather than write a second one. **Where they differ.** Local execution keeps the query inside a Spark task of the application's own JVM: - Memory comes from Spark's off-heap pool through the task's `CometTaskMemoryManager`, and spills go to Spark's local directories. sunchao's last round was about getting that share right when several queries run at once. A Ballista executor in the same process would reserve from its own pool, outside what Spark accounts for. - Cancellation, job groups and the result-size limit work through the Spark job, and `collect` results reach the driver within the JVM. - A query that isn't admitted runs as it does today, rather than failing as it would in a strict-native mode. - Within one process, the exchanges #6499 targets become DataFusion repartitions in memory. Ballista's executor writes each shuffle to files in its work directory (`COMET_EXECUTOR_WORK_DIR` in #7289) and serves them over Flight. Is there an in-process mode that skips that? That is the cost #6499 is trying to remove on a local master. Running the cluster in-process would also put Ballista into libcomet. #7288 keeps it in a separate workspace, so that Comet isn't tied to Ballista's DataFusion releases. #7289 currently builds on the `comet-native` branch with four unmerged Ballista PRs. So I'd keep the two separate at execution and share at planning. Once #7289's stage graph settles, local execution could run that same stage graph in one process, with `RepartitionExec` in place of Ballista's shuffles. That would also be the "run it against Ballista too" path you mentioned in #6499, from the other direction. Does that split work for you? -- 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]
