andygrove opened a new pull request, #2514: URL: https://github.com/apache/datafusion-ballista/pull/2514
# Which issue does this PR close? Closes #2513. # Rationale for this change A client and a scheduler from different major versions can run a query and return wrong results. Each Ballista major moves to a new DataFusion major, and DataFusion's plan encoding can change in ways that still decode but mean something different (see the `preserve_nulls` example in #2370). Today a mismatch only logs a warning, which #2378 added. This matters more if we release the Python client separately from the Rust crates (#2511). There would be a window where the newest Python client is a major version behind the newest cluster, and we want that to be an error rather than a source of wrong results. # What changes are included in this PR? - A new `ballista_core::version` module with the major version check, shared by the client and the scheduler. The version travels in a `ballista-version` gRPC header. A side that doesn't send it is treated as older than 55.0.0, the first release that does. - The scheduler checks the client's version in `ExecuteQuery` and `ExecuteQueryPush` before it creates a session or decodes the plan, and returns `failed_precondition` on a mismatch. It sends its own version back in the response. The check lives in the gRPC handlers, so the standalone scheduler and custom scheduler binaries get it too. - The client sends its version with both RPCs and checks the version in the scheduler's reply. This replaces the client-side `server` header warning from #2378. The scheduler still sends `server` and `x-powered-by`. A different major version is now an error, and a different minor or patch version is still a warning. - The reply check is for schedulers older than 55.0.0. They don't check versions and have already queued the job, so the client cancels it before returning the error. In push mode the job id only arrives with the first status update, so that cancellation happens in the background. - `examples/standalone-substrait.rs` talks to the scheduler's gRPC API directly, so it now sends the header. - An entry in the 55.0.0 upgrade guide. `main` is still versioned `54.0.0`, and a peer without the header can't be told apart from a 54.x one. So nothing is enforced until the 55.0.0 version bump. If this lands after `branch-55` is cut, `FIRST_MAJOR_WITH_VERSION_HEADER` needs to become 56. Tests: - A table test for the version rules. - Scheduler tests that a mismatched client is rejected before its plan is decoded on both RPCs, and that the scheduler's version comes back on success and when the plan fails to parse. - Client tests against a fake scheduler that accepts jobs without checking and reports version `1.0.0`. They check that the client returns an error and cancels the job in both pull and push mode. - Since CI can't exercise the post-bump behavior, I also bumped the workspace to 55.0.0 locally. The `ballista` client integration tests pass with the check active in both standalone and remote setups, and a submission without the header is rejected with the message shown in the upgrade guide. The Python bindings don't need any changes. On `main` they still build against the published 54.0.0 crates, and they'll pick this up when they move to 55. # Are there any user-facing changes? Yes. From 55.0.0, a client and a scheduler with different major versions refuse to work together. That includes the Python client, so the 54.x `ballista` package from PyPI can't run jobs on a 55.0.0 cluster. Hand-written clients of the scheduler's gRPC API need to send the `ballista-version` header with `ExecuteQuery` and `ExecuteQueryPush`, or a 55.0.0 scheduler will reject them. `ballista_core::version::insert_version_header` does this for a `tonic` request. I've added the `api-change` label for this. -- 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]
