andygrove commented on PR #2721: URL: https://github.com/apache/datafusion-ballista/pull/2721#issuecomment-6579636851
Fair question, sorry the description was dense. Here's the plain version. **The problem** Ballista sometimes splits one stage across several tasks. Say a stage has 4 partitions and 2 tasks. Task A gets partitions 0 and 1, and task B gets 2 and 3. Before sending a task out, the scheduler rewrites the plan so each task only sees its own partitions. It knows how to do this for the nodes it recognises: shuffle readers, Parquet and in-memory scans, and a couple of operators on a hand-kept list. If a plan contains some other node that holds data per partition, the scheduler leaves it alone. Task B then runs "partitions 0 and 1" of that node instead of 2 and 3. The result is that partitions 0 and 1 come back twice and 2 and 3 never come back. There is no error, just wrong results. **Why Comet needs it** Comet native executors build plans with their own leaf nodes that hold per-partition data. The scheduler doesn't know about them, so it can't slice them. Today Comet works around that by giving every fragment a fake `MemorySourceConfig` leaf with one row per partition, purely so the scheduler has something it recognises. See apache/datafusion-comet#7288. **What the PR does** 1. `PartitionSliceable` can now be implemented by any node, including leaves and nodes with several children. The method is renamed from `slice_to_partitions` to `restrict_to_partitions`. 2. `SchedulerConfig` gets a `partition_sliceable_resolver`. A downstream crate uses it to tell the scheduler how to find the `PartitionSliceable` for its own nodes. 3. A leaf that nobody can restrict now fails the job with a clear message instead of silently duplicating output. The scheduler also checks that a restricted node returns the right number of partitions. The first commit is unrelated. It's a one-line import fix for `main`'s CI, which has been red since #2613. **Breaking changes** The trait method rename, a third argument on `restrict_plan_to_partitions`, and a new public field on `SchedulerConfig`. All are listed in the 56.0.0 upgrade guide. -- 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]
