andygrove opened a new issue, #2261: URL: https://github.com/apache/datafusion-ballista/issues/2261
## Summary Ballista has three interfaces where two differently-versioned things can meet, and we treat all three differently, mostly by accident rather than by decision. I would like us to agree a written policy for each and put it somewhere contributors will actually see it. This is a proposal for discussion rather than a plan, and it probably wants a dev list thread. Opening it here so the current state is written down somewhere citable. ## The three surfaces | Surface | Are both sides alive at once? | Mechanism today | | --- | --- | --- | | scheduler ↔ executor gRPC | Yes, and upgraded together | `BALLISTA_PROTOCOL_VERSION`, strict equality, executor rejected on mismatch | | REST `/api/*` → TUI, and any other client | Usually the same release, but nothing enforces it | None | | Event log on disk | **No.** Written once, read arbitrarily later | One-way forward compatibility, added in #2260 | The differences are not arbitrary, they follow from the lifetimes. But we have never said so out loud, and the middle row has no mechanism at all. ## Where each one stands **Executor handshake.** `BALLISTA_PROTOCOL_VERSION` in `ballista-core` is compared on every registration and heartbeat, and a mismatched executor is rejected with `failed_precondition` and never given work. Strict equality is right here: both ends are live, an operator controls both, and refusing to proceed beats guessing. This one seems settled to me. The only gap is that "bump this when the wire format changes" is a comment on a constant rather than a release-checklist item. **Event log.** #2260 gives this one-way forward compatibility, which is the only thing that can work when you cannot upgrade the writer of a file that already exists. Readers accept any log whose version is not newer than their own, records are self-describing so a reader can check the version before parsing, stored API responses are held as raw JSON so they do not break when the REST types change, and a frozen `schema-v1.eventlog` is replayed in CI. Documented on `SCHEMA_VERSION`. **REST API.** Nothing. This is the gap. `/api/version` exists and reports the scheduler and DataFusion versions, but nothing calls it, including our own TUI. There is no negotiation, no minimum-supported-client notion, and no statement anywhere about whether the JSON shape is stable. We already know what that costs. #2257: `TaskSummary::partition_id` changed from `u32` to `Vec<u32>` in #2038, the TUI's copy of the type was not updated, and the stages popup silently stopped parsing. That was caught by reading the code, not by a test or a version check. #2258 removes the duplicate declaration, which fixes same-version drift by making it a compile error, but a TUI from release N talking to a scheduler from release N+1 is still completely unguarded. ## What I would propose Rough starting point, not a finished policy. **Executor handshake:** keep strict equality. Add "does this change the executor wire format?" to the release checklist so the bump is a deliberate step rather than something remembered. **Event log:** ratify what #2260 does. Readers accept `version <= SCHEMA_VERSION`. Bumping means breaking, additive changes stay unbumped and require `#[serde(default)]`. Every bump keeps a golden fixture from the previous version in CI. **REST API:** decide which of these we mean, and write it down. 1. *Unstable.* The API is an implementation detail of the TUI, may change in any release, and external consumers are on their own. Cheapest, and honest about where we are today. 2. *Stable within a major.* Additive changes only inside a release series. Removing or retyping a field waits for a major and gets an upgrade-guide entry. 3. *Versioned and negotiated.* Clients send a version, the scheduler serves accordingly or refuses. Most work, only worth it if we expect real third-party consumers. I lean towards 2, on the grounds that we already ship a client (the TUI) that people run against clusters they did not necessarily upgrade at the same moment, and 1 makes that a coin flip. But 1 is defensible if we would rather not commit yet, and saying so explicitly is still better than the current silence. Whichever we pick, two cheap things help immediately: - Have the TUI call `/api/version` at startup and warn on a mismatch, rather than failing to parse something later and showing an empty screen. - Note in `docs/source/contributors-guide/` that changing a type in `ballista-api-types` is a wire change, since the crate is now shared by the scheduler, the TUI, and the event log. ## Why now `user-personas.md` lists "backward-compatible public APIs, with breaking changes signalled in the upgrade guide" among the things Persona 3 depends on. We have an upgrade guide and we use it for Rust API changes, but the REST shape has been changing without appearing in it. Worth closing that gap while the surface is still small. Related: #2257, #2258, #2260. -- 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]
