andygrove opened a new issue, #2370:
URL: https://github.com/apache/datafusion-ballista/issues/2370
**Is your feature request related to a problem or challenge? Please describe
what you are trying to do.**
The scheduler validates `BALLISTA_PROTOCOL_VERSION` when an executor
registers, but there is no equivalent check for clients.
`check_protocol_version` in `ballista/scheduler/src/scheduler_server/grpc.rs`
is only wired into the executor registration RPCs, and nothing in
`ballista/core/src/execution_plans/distributed_query.rs` or the client crate
sends or verifies a protocol version.
The result is that a client built against a different Ballista version
connects happily, plans and runs a job successfully, and then fails much later
with a confusing error when it tries to read the results.
This came up while investigating #2367, where the client asked the executor
for `.../{job}/3/0/data.arrow` while the executor had written
`.../{job}/3/0/data-0.arrow`. The shuffle file layout for the final stage
changed after 54.0.0: the passthrough `ShuffleWriterExec` used to report
`file_id: None` and write `data.arrow`, and now reports `file_id:
Some(task_id)` and writes `data-{task_id}.arrow`. A client and cluster that
disagree about that surface as:
```
Failed to open partition file at ".../data.arrow": Os { code: 2, kind:
NotFound, ... }
```
which reads like data loss or a cleanup race rather than a version mismatch.
The job even reports as `Completed`, so the first thing a user suspects is the
shuffle machinery.
**Describe the solution you'd like**
Extend the existing protocol version handshake to the client:
- Have the client send its compiled-in `BALLISTA_PROTOCOL_VERSION` on
`ExecuteQuery` (or on a dedicated handshake RPC).
- Have the scheduler reject a mismatch with a clear `failed_precondition`
naming both versions, the same way it already does for executors.
- Bump `BALLISTA_PROTOCOL_VERSION` whenever the on-disk shuffle layout or
the client-visible wire contract changes, and document that rule next to the
constant in `ballista/core/src/lib.rs`.
**Describe alternatives you've considered**
Only bumping the version on wire-format changes and leaving layout changes
alone. That does not help here, since the proto messages (`PartitionLocation`,
`ShuffleWritePartition`, `FetchPartition`) are byte-identical between 54.0.0
and main. What actually diverged is the meaning of `file_id`, so the version
rule needs to cover the shuffle layout too.
Alternatively, make the failure message itself smarter (e.g. when a
partition file is missing, check whether the sibling name under the other
layout exists and say so). Useful as a diagnostic, but it papers over the
mismatch instead of refusing it up front.
**Additional context**
Related: #2367.
--
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]
