test: add HA chaos harness that drives the scheduler's fault-tolerance paths on a real cluster - #2026
Merged
Merged
Conversation
Disable reqwest's default-tls feature in ballista-chaos so it no longer pulls native-tls/openssl-sys into the workspace's dependency graph; object_store already depends on the same reqwest version with rustls only, and cargo unifies features across that shared node. The harness only ever talks to http://127.0.0.1, so no TLS backend is needed at all. Also redirect each spawned scheduler/executor child's stdout and stderr to per-process log files under the cluster's temp directory instead of Stdio::piped() with nothing reading the pipes. Once a child's output exceeds the OS pipe buffer, its next write blocks forever, which turns long-running kill/restart scenarios into a silent hang. A restarted executor appends to its existing log rather than truncating it, so the killed process's output survives. Add TestCluster::log_dir() so tests can locate the logs, and have Drop log a warning pointing at a child's log file when it exited non-zero.
…on, and panic Scenario A's AQE-on case reproduces a real Ballista bug: an IO fault during a broadcast join's shared build-side collection surfaces as DataFusionError::Shared(IoError), which the failure classifier in ballista/core/src/error.rs does not recognize as retryable, so the job fails on the first attempt instead of retrying. Marked #[ignore] with the finding recorded inline; not fixed here since it requires touching a production crate outside this plan's scope.
TestCluster::job_status was reading the "job_status" REST field, which
the scheduler populates with a long human-readable sentence (e.g.
"Completed. Produced 1 partition containing 50 rows. Elapsed time: 49
ms."), never the short "Successful"/"Failed" values its doc comment
promised. The short, matchable categorical value ("Queued", "Running",
"Completed", "Failed", "Invalid") lives in the sibling "status" field.
Read that instead so callers can assert on job state.
…e, document findings
This was referenced Jul 13, 2026
Retryable IO errors are misclassified as non-retryable when wrapped in DataFusionError::Shared
#2028
Closed
…scenarios The harness located its child-process binaries by inferring the profile directory from cfg!(debug_assertions), which only holds for the stock dev and release profiles. CI builds with --profile ci, which inherits dev but disables debug assertions, so every cluster-spawning test looked in target/release and panicked on a binary that was sitting in target/ci. Derive the directory from the running test executable instead, which is correct under any profile. Ignore the five scenarios that reproduce known bugs, each against its issue: scenario A under AQE (apache#2028), scenario D (apache#2027), and scenario G (apache#2029). Their assertions are unchanged, so un-ignoring them once the bugs are fixed turns them into the regression tests. Note that scenario D is a race between the fetch- failure and heartbeat-expiry recovery paths, and only fails when the former wins. Hold a process-wide lock for a TestCluster's lifetime so cluster-spawning tests serialize themselves. The suite required --test-threads=1, but CI runs a plain cargo test over the workspace and cannot pass it.
# Conflicts: # Cargo.toml
- Rename concurrent_tasks to vcores in chaos-executor for the ExecutorProcessConfig change on main - Ignore executor_killed_after_shuffle_write_is_recovered against apache#2027: CI showed the reduce stage hitting the dead executor's FetchFailed flattened inside DataFusionError::Shared, so the map stage is never resubmitted; the scenario only passes when the kill loses the race - Ignore both retryable-fault scenarios against apache#2027's error flattening: apache#2028 was fixed on main (apache#2119) but the sort-shuffle writer refactor now Debug-formats task errors into opaque Execution strings before classification, so the injected IoError is misclassified as non-retryable under both AQE settings - Serialize clusters machine-wide with an advisory flock so concurrent cargo invocations or binary-parallel runners cannot start two clusters at once - Raise the executor registration deadline from 30s to 120s and attach every child process log tail to the timeout error, making the CI startup timeouts diagnosable if they recur - Update the README findings to match
andygrove
marked this pull request as ready for review
July 28, 2026 21:55
Member
Author
phillipleblanc
approved these changes
Jul 29, 2026
phillipleblanc
left a comment
Contributor
There was a problem hiding this comment.
This is great, looking forward to integrating this into our project's ballista testing setup.
Member
Author
|
Thanks for the review @phillipleblanc. I'll go ahead and merge so that others can iterate on improving this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #2030.
This harness reproduces the following, each of which is filed separately:
DataFusionError::Shared(fixed on main by Retryable IO errors misclassified when wrapped in DataFusionError::Shared #2119, but see the test status section below)Rationale for this change
Ballista has a substantial high-availability state machine in
ballista/scheduler/src/state/execution_graph.rs(update_task_status): failures are classifiedretryable/count_to_failures, tasks retry up totask_max_failures, aFetchPartitionErrorrolls back the running stage and resubmits the map stage, and a lost executor resets its running
tasks and removes the shuffle output it produced so map stages re-run.
None of it was tested end to end. Every existing test of these paths fabricates
TaskStatusprotobuf messages by hand (
execution_graph.rstests,scheduler/src/test_utils.rs). No test everdrove the HA state machine from a real query on a real cluster, and no test ever killed an executor.
Running the state machine for real immediately surfaced three bugs, two of which mean Ballista
fails queries that it is designed to recover from. See the linked issues.
There is an existing chaos operator (
ChaosExec), but it is unusable as a regression harness: it iswired only into the AQE planner (
state/aqe/planner.rs), so it cannot inject faults with AQE off; itwraps a randomly chosen plan node; and it fires probabilistically. No test uses the
ballista.testing.chaos_execution.*keys today.What changes are included in this PR?
A new non-published workspace crate,
chaos-testing(ballista-chaos). No production crate ismodified — the only change outside
chaos-testing/is adding the member to the workspaceCargo.toml.chaos_fail(guard, mode, budget_dir)andchaos_delay(guard, ms)are pass-through scalar UDFs spliced into the query text, so the identicalinjection works with AQE on and AQE off with zero planner wiring.
modeselects which of Ballista'sthree failure classifications is exercised:
io(retryable),exec(non-retryable),panic(caught by the executor's
catch_unwind, non-retryable).guardpredicate over fixed data selects which partitions fault;a filesystem token budget bounds how many attempts fault, cluster-wide, across task retries and
executor restarts (consuming a token is
fs::remove_file, which is atomic across processes). Budget 1means exactly one attempt faults anywhere, so a retry must succeed; budget ≫
task_max_failuresmeans retries must exhaust.
TestClusterspawns realchaos-scheduler/chaos-executorbinaries as OS processes (they inject the UDFs via the existing
override_function_registry/override_session_builderhooks, followingexamples/custom-executor.rs), so executors can beSIGKILLed and restarted. It tunesexecutor_timeout_seconds/ heartbeat down from the productiondefaults of 180s/60s, without which a killed executor would not be noticed for three minutes.
Clusters serialize themselves via a process-wide mutex plus a machine-wide
flock, and aregistration timeout attaches every child process's log tail to the error so CI failures are
diagnosable from test output alone.
/api/job/{id}/stages) so a killhappens while a stage genuinely has running tasks, rather than after a guessed
sleep.panicking task; executor killed mid-stage; executor killed after writing shuffle output; executor
killed and restarted; every executor killed.
The load-bearing assertion in every recovery scenario is result equality against a chaos-free baseline
run, not merely "it did not error" — re-running a stage is exactly where duplicated or dropped
partitions would appear, and only a correctness check catches that.
Current test status
cargo test -p ballista-chaos→ lib: 18 passed. ha scenarios: 8 passed, 8 ignored, 0 failed.Every scenario that reproduces a known bug is
#[ignore]d against the issue it reproduces, with itsoriginal assertions intact — nothing is relaxed to manufacture a pass. Run them with
cargo test -p ballista-chaos -- --ignoredto see the failures. Once the linked issues are fixed,un-ignoring these scenarios turns them into the regression tests for those fixes. Each is documented
in
chaos-testing/README.mdwith its root cause:executor_killed_mid_stage_is_recoveredandexecutor_killed_after_shuffle_write_is_recovered(both AQE settings) reproduce Shuffle-fetch failures lose their type, so the map-stage resubmit never fires and the job fails #2027: the typed
FetchFailedis flattened before the scheduler canclassify it, so the map stage is never resubmitted and the job fails instead of recovering. The
shuffle-write case failed in CI with
FetchFailedburied insideDataFusionError::Shared; it onlypasses when the kill happens to land after the reduce stage already fetched its input.
retryable_fault_is_retried_and_result_is_correct_{aqe_off,aqe_on}originally demonstrated Retryable IO errors are misclassified as non-retryable when wrapped in DataFusionError::Shared #2028(
Shared(IoError)missing the retryability classifier's shallow match). Retryable IO errors misclassified when wrapped in DataFusionError::Shared #2119 fixed that byclassifying on
find_root()— but the sort-shuffle writer refactor (Multi-partition tasks: partition_slice plumbing + SortShuffleWriter refactor #2038/Remove the hash-partitioning shuffle writer #2106) now flattens anytask error into
DataFusionError::Execution(format!("{e:?}"))in the shuffle-write coordinator, sothe
IoErrorreaches the classifier as inert text and both cases fail again, this time under bothAQE settings. This is the same type-erasing mechanism as Shuffle-fetch failures lose their type, so the map-stage resubmit never fires and the job fails #2027, and arguably makes that issue more
urgent: every transient IO error in any stage that writes shuffle output is now non-retryable.
killing_every_executor_terminates_the_job(both AQE settings) reproduces Job hangs indefinitely instead of failing when all executors are lost #2029: the job hangsinstead of failing once no executor can ever satisfy the remaining tasks.
Passing: baseline correctness (both AQE settings), retry exhaustion, panicking task (job fails
cleanly, executor survives), and executor kill + restart.
Are there any user-facing changes?
No. The crate is test-only and
publish = false; no production crate is touched.