andygrove opened a new pull request, #2259:
URL: https://github.com/apache/datafusion-ballista/pull/2259

   # Which issue does this PR close?
   
   Closes #1923.
   
   > **Stacked on #2256.** GitHub shows the union of both branches until that 
one merges, so review the last commit here (`feat: event log and history 
server`) rather than the full diff. It builds on the `dto_build` extraction and 
the `ballista-api-types` crate that #2256 introduces.
   
   This supersedes #1925, which was the same feature as a single unreviewable 
change against a base that is now 189 commits stale.
   
   # Rationale for this change
   
   Ballista's TUI shows jobs, stages, tasks and metrics by reading the 
scheduler's REST API, but that state is ephemeral. Completed jobs are cleaned 
up after `finished_job_state_clean_up_interval_seconds`, and everything is gone 
when the scheduler restarts. There is no way to look at a job after the fact, 
which is exactly when you usually want to.
   
   `user-personas.md` lists "a history server / UI" among the things Persona 2 
depends on, so this is filling in a guarantee already written down rather than 
adding a new one.
   
   # What changes are included in this PR?
   
   **New `ballista-history` crate.** Depends on `ballista-api-types` plus 
serde, tokio and log.
   
   - A versioned JSONL event schema. `JobStart`, `StageStart`, `StageEnd` and 
`TaskEnd` form an incremental timeline; the terminal `JobEnd` embeds the 
finished REST responses.
   - An async buffered `EventLogWriter`. All file I/O happens on a background 
task, so the scheduler's event loop never waits on disk. Timeline events are 
dropped rather than allowed to block if the queue backs up, on the grounds that 
losing a progress record beats stalling scheduling. `JobEnd` is the exception 
and waits for capacity, because a job missing it is invisible to the history 
server.
   - A reader that folds a completed log back into the served payload.
   
   **Scheduler.** A new `--event-log-dir` flag, off by default. When it is 
unset there is no channel, no task, no file, and no per-event work beyond one 
`Option` check.
   
   When it is set, a tee at the top of `QueryStageScheduler::on_receive` maps 
`JobSubmitted`, `TaskUpdating`, `JobFinished`, `JobRunningFailed` and 
`JobCancel` onto history events. `JobCancel` is handled there specifically 
because the handler below it drops the graph, so that is the last point at 
which a cancelled job can be recorded. `JobPlanningFailed` is deliberately 
absent: it is posted instead of `JobSubmitted`, so the job has neither a graph 
nor an open log.
   
   **New `ballista-history-server` binary.** `ballista-history-server 
--event-log-dir <dir>` loads completed logs at startup and serves `/api/*` from 
the stored responses. Corrupt or partial logs are skipped rather than failing 
startup, so one bad file cannot hide every other job.
   
   **Docs.** A new [History 
Server](https://github.com/apache/datafusion-ballista/blob/main/docs/source/user-guide/history-server.md)
 user-guide page covering both flags, the endpoints, and the operational 
caveats below.
   
   # Why replayed output can be trusted
   
   The history server never re-derives a response. `JobEnd` stores the 
`JobResponse` and `QueryStagesResponse` the scheduler already built from its 
live execution graph, and replay deserializes and re-serializes them unchanged. 
Byte-identity is a structural property, not two implementations agreeing.
   
   `history_store_serves_byte_identical_json_to_live_scheduler` pins this: it 
builds the live DTOs, emits a real `JobEnd` through the real async writer, 
loads it back through the history server's own `HistoryStore::load`, and 
asserts the serialized JSON matches.
   
   Two things fell out of getting that test deterministic, and both are 
improvements in their own right:
   
   - `JobEnd` renders its stage snapshot as of `completed_at` rather than the 
wall clock, so the stored record is a snapshot of the job at the moment it 
ended.
   - `get_job_config` now returns the sorted `JobConfig` map. It previously 
served `SessionConfig::to_props()` directly, which is a `HashMap`, so live 
output had non-deterministic key order and could never have matched a replay.
   
   # Are there any user-facing changes?
   
   Yes, all additive and opt-in:
   
   - New scheduler flag `--event-log-dir <dir>`, default disabled.
   - New `ballista-history-server` binary.
   - The TUI can point at a history server with `--host` / `--port` to browse 
completed jobs with no live scheduler.
   - `GET /api/job/{job_id}/config` now returns keys in sorted order. Same 
content, deterministic ordering.
   
   No breaking API changes.
   
   Verified locally: `cargo test -p ballista-scheduler --lib` passes (338 
tests, 13 of them new), `cargo test -p ballista-history` passes, clippy is 
clean for all three crates with `--all-features -D warnings`, 
fmt/taplo/prettier are clean, and the `--no-default-features` check still 
passes.
   
   I also smoke-tested the binary against a hand-written event log: it loads 
the job and serves `/api/jobs`, `/api/job/{id}`, `/api/job/{id}/stages` and 
`/api/job/{id}/config` correctly, and 404s on an unknown job.
   
   # Known limitations
   
   Called out in the docs rather than left to be discovered:
   
   - **Local filesystem only**, and logs are read once at startup. Restart the 
history server to pick up newly finished jobs.
   - **Disk is not reclaimed automatically.** Logs accumulate until pruned.
   - **Plans are rendered once, at write time**, so `?plan_format=` has no 
effect against a history server.
   - **Only the terminal record is served.** The `TaskEnd` timeline is captured 
so a future UI can show a job progressing, but nothing reads it yet.
   
   # Note on size
   
   This is ~1700 lines, which is large for one review. It was originally scoped 
as three PRs (event crate, scheduler wiring, history server) and can still be 
split that way if reviewers would prefer. The three parts are cleanly 
separable: the crate has no scheduler dependencies, and the wiring and the 
server touch disjoint files.
   


-- 
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]

Reply via email to