andygrove opened a new issue, #2513: URL: https://github.com/apache/datafusion-ballista/issues/2513
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.** Using the Python client with a Ballista cluster from a different major release should be an error. `ballista` 55.x.x from PyPI should only work with a 55.x.x scheduler and executors. Pointing it at a 54.x.x or 56.x.x cluster should fail up front with a clear message that names both versions. Today a mismatch is at most a warning. #2378 added a `server: ballista/<version>` header to scheduler responses, and `check_scheduler_version` in `ballista/core/src/execution_plans/distributed_query.rs` logs a warning when it doesn't match the client's version. The query then runs anyway. That isn't safe across major versions. Each Ballista major moves to a new DataFusion major, and DataFusion's proto can change in ways that still decode but mean something different. In #2370 @avantgardnerio pointed out that DataFusion 55 replaced `UnnestOptions.preserve_nulls` with a new field. An unnest from a 54 client that should drop nulls would keep them on a 55 cluster. That's different rows and no error. #2376 was a similar case in our own codec, and we avoided it by reverting #2348 in #2379. We can't do that when the change is in DataFusion. This becomes more important if we release the Python client separately from the Rust crates (#2511). Between the two releases, the newest Python client will be a major version behind the newest cluster. Python users should keep their cluster on the previous release until the matching client ships. If they don't, they should get an error rather than wrong results. **Describe the solution you'd like** Return an error when the client and scheduler have different major versions, and keep the warning for minor and patch differences. A client built against 55.0.0 would work with a 55.1.0 cluster, but not with 54.1.0 or 56.0.0. Comparing only the major version also means the check doesn't depend on how we number Python-only releases. The version it compares is `BALLISTA_VERSION` from the `ballista-core` crate the wheel was built against, not the PyPI package version. Some things the current check doesn't handle yet: - **It runs after the job is submitted.** The client reads the header from the `ExecuteQuery` / `ExecuteQueryPush` response, and by then the scheduler has already called `submit_plan`. Remote contexts don't talk to the scheduler before the first query. So if we just turn the warning into an error, the job keeps running on the cluster with nobody reading the results. The check should happen before the job is submitted, or the client should cancel the job when it fails. - **Released clients have no check.** 54.0.0 and 54.1.0 predate #2378, so only the scheduler can reject them. If clients sent their version as gRPC metadata on `ExecuteQuery`, the scheduler could return `failed_precondition` on a major mismatch, the same way `check_protocol_version` does for executors. We'd also need to decide what to do with a client that sends no version. That covers everything up to 54.1.0, but it could also be a third-party client of the scheduler gRPC API. - **A missing header is ambiguous.** 54.x schedulers don't send `server`, but neither does the in-process standalone scheduler in `ballista/scheduler/src/standalone.rs`, which serves gRPC without `with_version_headers`. Today both cases pass silently. The standalone scheduler should probably send the header too, so that a missing header reliably means an older scheduler. - **Proxies can rewrite `server`.** Envoy, for example, overwrites it with its own name by default. The check shouldn't fail on a value that doesn't look like `ballista/<version>`. A dedicated header, or a version field on `ExecuteQueryResult`, might be more reliable. This check lives in `ballista-core`, which the Python client is built on, so Rust clients get the same error. The DataFusion proto changes affect them in the same way. **Describe alternatives you've considered** - Keep the warning. It's easy to miss, and it doesn't stop a query from returning wrong results. - Require the exact same version, like the executor handshake does. That's too strict for clients, since users would have to upgrade clients and clusters in lockstep for every patch release. - Feature or capability negotiation, which came up on #2370. It's more precise but a lot more work, and it can't catch DataFusion proto changes we don't know about. "Same major version" is a simple rule to explain to users. **Additional context** This was one of the open questions on #2511, where @avantgardnerio agreed that a major version mismatch should be an error. Related: #2370, #2378, #2376, #2261, #2372. -- 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]
