Skip to content

Commit bd17e43

Browse files
feat: Improve TPC-H performance with AQE on (#2084)
* feat: make AQE respect broadcast_join_threshold_bytes The ballista.optimizer.broadcast_join_threshold_bytes config is only consumed by the static distributed planner (maybe_promote_to_broadcast). Under adaptive query planning, broadcast (CollectLeft) selection in DynamicJoinSelectionExec::to_actual_join instead used DataFusion's hash_join_single_partition_threshold (1 MiB default), so the Ballista key had no effect and the effective cutoff was silently a different value. Use broadcast_join_threshold_bytes as the byte threshold in the AQE join selection path, keeping DataFusion's row threshold as the absent-stats fallback. A value of 0 disables broadcast promotion, matching the static planner. This gives a single config key consistent behavior under both planners. Closes #2085 * feat: add Ballista broadcast_join_threshold_rows and use it in AQE Follow-up within the same change: the AQE join-selection path also used DataFusion's hash_join_single_partition_threshold_rows as the row-count fallback. There was no Ballista equivalent, so the row threshold still escaped the single-config goal. Add ballista.optimizer.broadcast_join_threshold_rows (default 128K, mirroring DataFusion's previous default) plus SessionConfigExt accessors, and use it in DynamicJoinSelectionExec::to_actual_join instead of the DataFusion key. AQE broadcast selection now depends only on Ballista config. Document both broadcast thresholds in the AQE tuning guide. * fix: default broadcast_join_threshold_rows to 1M to match existing behavior SessionConfig::new_with_ballista() already installs a 1,000,000 row threshold for DataFusion's hash_join_single_partition_threshold_rows. Default the new Ballista row key to the same value so consolidating AQE onto the Ballista keys does not silently lower the effective row-count broadcast cutoff. * refactor: derive new_with_ballista join thresholds from Ballista config defaults Instead of hard-coding 10 MiB / 1M for the DataFusion hash_join_single_partition_threshold[_rows] session settings, read them from BallistaConfig::default().broadcast_join_threshold_bytes()/_rows(). The Ballista broadcast-threshold defaults are now the single source of truth for both DataFusion's built-in JoinSelection and Ballista's AQE join selection. Values are unchanged. * feat: demote DataFusion CollectLeft joins over the Ballista threshold In the static planner, maybe_promote_to_broadcast trusted any broadcast-safe HashJoinExec(CollectLeft) that DataFusion's JoinSelection produced. DataFusion decides CollectLeft from its own session threshold, which can exceed a runtime override of ballista.optimizer.broadcast_join_threshold_bytes. Demote such a join back to Partitioned when its build side is not under the current Ballista threshold (or when broadcasts are disabled with threshold 0), so the Ballista key is authoritative in the static path too. Null-aware anti joins are never demoted since they require CollectLeft. * docs: apply prettier formatting to AQE tuning-guide table * test: disable AQE broadcast via Ballista threshold in repartition tests The join-selection repartition tests forced the repartition path by setting DataFusion's hash_join_single_partition_threshold[_rows] to 0. AQE join selection now reads the broadcast cutoff from the Ballista config (broadcast_join_threshold_bytes), so those DataFusion keys no longer gate CollectLeft promotion and the small test tables were promoted to broadcast. Set the Ballista broadcast byte threshold to 0 in the helper, which disables CollectLeft promotion and restores the repartitioned plans the snapshots assert. * fix: size the AQE broadcast decision by bytes, not row count `supports_collect_by_thresholds` compared a row count against `hash_join_single_partition_threshold_rows` whenever `total_byte_size` was unknown, so a build side of up to a million arbitrarily wide rows could be broadcast to every probe task without the byte threshold ever applying. Unknown `total_byte_size` is the common case, not an edge case: DataFusion discards it on every join, and rebuilding it in `Statistics::calculate_total_byte_size` only works when every column has a fixed width, so a single `Utf8` column loses it permanently. In TPC-H that covers most dimension-side join results. Estimate the size instead and hold it to the same byte threshold. Each column contributes its own `byte_size` statistic when present -- a total for that column's output, already scaled for filters and limits -- otherwise its fixed width times the row count, otherwise a default width mirroring Spark's `StringType`/`BinaryType` defaults. An overflowing estimate declines the broadcast rather than wrapping to a small number. The row threshold is retained as a ceiling, so this can only reject a broadcast the row rule would have allowed, never introduce a new one. Closes #2081. * test: cover broadcast thresholds with declared statistics The broadcast-vs-partitioned decision is a function of statistics, but the tests around it could only describe tables they were willing to materialise, so the sizes it actually turns on had no coverage: a build side of hundreds of thousands of rows, or one whose `total_byte_size` is unknown. The existing tests instead toggle the decision by zeroing the threshold, which shows the rule is self-consistent but not that the shipped thresholds behave. Add `StatsTable`, a table that declares its statistics and holds no rows, so a fixture can say "800,000 rows of unknown size" in one line. Its scan reports the declared figures and cannot be executed, which is enough for the planner tests, and it deliberately does not recompute `total_byte_size` on projection, since an unknown size is the case these fixtures exist to express. Add tests covering the decision at both edges -- wide rows of unknown size are not broadcast, while small dimensions, narrow rows, and known sizes under the threshold still are -- run under `SessionConfig::new_with_ballista` so they exercise the 10 MB / 1,000,000 row thresholds a deployment ships with rather than DataFusion's defaults, plus a test pinning those defaults directly. `wide_rows_of_unknown_size_are_not_broadcast` fails on the rule that preceded the previous commit and passes with it. The rest pass either way: they guard against the estimate rejecting broadcasts it should allow. Part of #2081. * feat: log AQE join decisions at INFO and shuffle spills at WARN Surface two previously debug-only diagnostics at levels visible under the default INFO filter, so join-strategy and memory-pressure behavior can be observed without enabling debug logging. - AQE dynamic join selection now logs each decision at INFO, naming the resolved action (CollectLeft/Hash/SortMerge/Repartition), the partition mode, and the size-aware inputs (per-side row/byte estimates and the byte/row broadcast thresholds). - Sort-shuffle write completion logs at INFO with row and spill counts; when a partition spills under memory pressure it logs at WARN with the spilled bytes, batches, and event count. Per-batch spill events stay at debug. * feat: default sort-shuffle spill cap to 0 (uncapped) and plumb it through serde The sort-shuffle writer's per-task buffered-bytes cap defaulted to 256 MB, far below the per-task memory-pool budget, so it spilled long before the pool was under pressure. Default the cap to 0, which disables it: spilling is then driven solely by memory-pool pressure. A non-zero value still adds a second spill trigger and is retained for tests and explicit tuning. The cap was also dropped during physical-plan serialization (the executor rebuilt the config with the default), so a configured value never reached the executor. Carry memory_limit_per_task_bytes through the SortShuffleWriterExecNode protobuf and apply it on decode, so an override via ballista.shuffle.sort_based.memory_limit_per_task_bytes takes effect on executors. * feat: add hash_join_max_build_partition_bytes config * feat: add max_per_partition_build_bytes helper for hash-join fit check Reads the actual materialized per-partition byte sizes off the resolved ExchangeExec feeding a Partitioned hash join's build side (the same source CoalescePartitionsRule reads) and returns the MAX rather than the average, since a single oversized partition is enough to OOM even when the average partition is small (the Q18 failure shape). * feat: fall back to SMJ when hash-join build exceeds per-slot budget * feat: log hash-join build-fit decision at INFO * refactor: hoist build-size call and tidy hash-join fit-check docs * docs: add hash-join + safety-fallback SF1000 suite results (2x16, p64) Record a full 22-query SF1000 run with prefer_hash_join=true and the AQE hash-join build-size safety fallback (hash_join_max_build_partition_bytes=64 MiB) on a 2 executor x 16 core cluster at target_partitions=64: all 22 queries complete with no OOM, where a pure hash-join run fails on Q18. Remove the AQE-off column pending a re-run at a matched core count. * docs: refresh SF1000 Ballista AQE-on results on #2084 build Re-ran the TPC-H SF1000 suite (AQE on, target_partitions=64, prefer_hash_join=false, 1 iteration) on the 2x16-core reference cluster against the PR build (becb376). Q1-Q17 from a full-suite run, Q19-Q22 as individual jobs; Q18 still OOMs (Partitioned build side, unchanged). Ballista total (excl. Q18) improves 4817.8 -> 4661.0s, led by the join-heavy queries (Q7 -88s, Q8 -191s, Q9 -169s). * revert: keep upstream 256 MiB sort-shuffle spill-cap default Drop this branch's change of the sort-shuffle per-task spill-cap default to 0 (uncapped); restore the 256 MiB default from main. The serde plumbing for the value stays (it landed on main via #2091). This PR no longer alters the shipped spill-cap default. * docs: note SF1000 results use uncapped sort-shuffle spill override Make explicit that the reference AQE-on numbers were produced with the sort-shuffle per-task spill cap overridden to 0 (uncapped); the shipped default is 256 MiB. Point the Ballista row at the current branch commit. * Update ballista/scheduler/src/state/aqe/execution_plan/dynamic_join.rs Co-authored-by: Phillip LeBlanc <phillip@spice.ai> * refactor: log per-partition shuffle write at debug; fix log import Demote the per-partition shuffle-write completion log to debug (it fires once per shuffle partition; spills stay at WARN). Update the dynamic_join log import to match the debug! call. --------- Co-authored-by: Phillip LeBlanc <phillip@spice.ai>
1 parent b809475 commit bd17e43

9 files changed

Lines changed: 1196 additions & 46 deletions

File tree

ballista/core/src/config.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ pub const BALLISTA_BROADCAST_JOIN_THRESHOLD_BYTES: &str =
111111
pub const BALLISTA_BROADCAST_JOIN_THRESHOLD_ROWS: &str =
112112
"ballista.optimizer.broadcast_join_threshold_rows";
113113

114+
/// Configuration key for the maximum per-partition hash-join build-side bytes
115+
/// permitted for a Partitioned hash join under AQE. When a build partition
116+
/// exceeds this, the join falls back to SortMergeJoin (spillable). `0` disables
117+
/// the check (hash join is used regardless of build size).
118+
pub const BALLISTA_HASH_JOIN_MAX_BUILD_PARTITION_BYTES: &str =
119+
"ballista.optimizer.hash_join_max_build_partition_bytes";
120+
114121
/// Configuration key to enable AQE coalesce-shuffle-partitions rule.
115122
/// Disabled by default — opt in when the workload benefits from larger
116123
/// downstream tasks more than from preserved parallelism.
@@ -264,6 +271,12 @@ static CONFIG_ENTRIES: LazyLock<HashMap<String, ConfigEntry>> = LazyLock::new(||
264271
promotion via the row-count path.".to_string(),
265272
DataType::UInt64,
266273
Some((1_000_000).to_string())),
274+
ConfigEntry::new(BALLISTA_HASH_JOIN_MAX_BUILD_PARTITION_BYTES.to_string(),
275+
"Maximum per-partition hash-join build-side bytes for a Partitioned \
276+
hash join under AQE. A build partition larger than this falls back to \
277+
SortMergeJoin (spillable). 0 (the default) disables the check.".to_string(),
278+
DataType::UInt64,
279+
Some("0".to_string())),
267280
ConfigEntry::new(BALLISTA_CLIENT_PULL.to_string(),
268281
"Should client employ pull or push job tracking. In pull mode client will make a request to server in the loop, until job finishes. Pull mode is kept for legacy clients.".to_string(),
269282
DataType::Boolean,
@@ -615,6 +628,11 @@ impl BallistaConfig {
615628
self.get_usize_setting(BALLISTA_BROADCAST_JOIN_THRESHOLD_ROWS)
616629
}
617630

631+
/// Maximum per-partition hash-join build-side bytes before falling back to SMJ.
632+
pub fn hash_join_max_build_partition_bytes(&self) -> usize {
633+
self.get_usize_setting(BALLISTA_HASH_JOIN_MAX_BUILD_PARTITION_BYTES)
634+
}
635+
618636
/// Returns whether the AQE coalesce-shuffle-partitions rule is enabled.
619637
pub fn coalesce_enabled(&self) -> bool {
620638
self.get_bool_setting(BALLISTA_COALESCE_ENABLED)
@@ -884,4 +902,12 @@ mod tests {
884902
assert_eq!(16777216, config.grpc_client_max_message_size());
885903
Ok(())
886904
}
905+
906+
#[test]
907+
fn hash_join_max_build_partition_bytes_defaults_to_zero() {
908+
assert_eq!(
909+
BallistaConfig::default().hash_join_max_build_partition_bytes(),
910+
0
911+
);
912+
}
887913
}

ballista/core/src/execution_plans/sort_shuffle/writer.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,8 +439,9 @@ impl SortShuffleWriterExec {
439439
let mut hash_buffer: Vec<u64> = Vec::new();
440440
let mut spill_events: u64 = 0;
441441
// Absolute buffered-bytes counter, independent of the runtime
442-
// `MemoryPool`. Drives spill decisions so the writer bounds its
443-
// RSS even when the pool is unbounded.
442+
// `MemoryPool`. When `memory_limit` is non-zero it caps this counter
443+
// as a second spill trigger; a `memory_limit` of 0 disables the cap
444+
// so spilling is driven solely by memory-pool pressure.
444445
let mut buffered_bytes: usize = 0;
445446
// A limit of 0 disables the per-task budget, leaving the runtime
446447
// `MemoryPool` as the sole spill trigger.

ballista/core/src/extension.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ pub trait SessionConfigExt {
204204
/// disables promotion via the row-count path.
205205
fn with_ballista_broadcast_join_threshold_rows(self, threshold_rows: usize) -> Self;
206206

207+
/// Returns the maximum per-partition hash-join build-side bytes before
208+
/// falling back to SortMergeJoin under AQE. `0` disables the check.
209+
fn ballista_hash_join_max_build_partition_bytes(&self) -> usize;
210+
207211
/// retrieves grpc client max message size
208212
fn ballista_grpc_client_max_message_size(&self) -> usize;
209213

@@ -513,6 +517,16 @@ impl SessionConfigExt for SessionConfig {
513517
}
514518
}
515519

520+
fn ballista_hash_join_max_build_partition_bytes(&self) -> usize {
521+
self.options()
522+
.extensions
523+
.get::<BallistaConfig>()
524+
.map(|c| c.hash_join_max_build_partition_bytes())
525+
.unwrap_or_else(|| {
526+
BallistaConfig::default().hash_join_max_build_partition_bytes()
527+
})
528+
}
529+
516530
fn ballista_shuffle_reader_maximum_concurrent_requests(&self) -> usize {
517531
self.options()
518532
.extensions

ballista/core/src/serde/mod.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,45 @@ mod test {
11431143
assert_eq!(stored.groups[0].upstream_indices, vec![0, 1, 2, 3]);
11441144
}
11451145

1146+
#[tokio::test]
1147+
async fn sort_shuffle_writer_memory_limit_survives_roundtrip() {
1148+
use datafusion::physical_plan::empty::EmptyExec;
1149+
1150+
let schema = create_test_schema();
1151+
let input: Arc<dyn ExecutionPlan> = Arc::new(EmptyExec::new(schema.clone()));
1152+
let partitioning =
1153+
Partitioning::Hash(vec![col("id", schema.as_ref()).unwrap()], 4);
1154+
1155+
let config = SortShuffleConfig::new(true, 4096)
1156+
.with_memory_limit_per_task_bytes(1024 * 1024 * 1024);
1157+
let original = SortShuffleWriterExec::try_new(
1158+
"job-1".to_string().into(),
1159+
3,
1160+
input.clone(),
1161+
String::new(),
1162+
partitioning,
1163+
config,
1164+
)
1165+
.unwrap();
1166+
1167+
let codec = BallistaPhysicalExtensionCodec::default();
1168+
let mut buf: Vec<u8> = vec![];
1169+
codec.try_encode(Arc::new(original), &mut buf).unwrap();
1170+
1171+
let ctx = SessionContext::new().task_ctx();
1172+
let decoded = codec.try_decode(&buf, &[input], &ctx).unwrap();
1173+
let decoded = decoded
1174+
.downcast_ref::<SortShuffleWriterExec>()
1175+
.expect("Expected SortShuffleWriterExec");
1176+
1177+
assert_eq!(
1178+
decoded.config().memory_limit_per_task_bytes,
1179+
1024 * 1024 * 1024,
1180+
"memory limit override must survive serialization to the executor"
1181+
);
1182+
assert_eq!(decoded.config().batch_size, 4096);
1183+
}
1184+
11461185
#[tokio::test]
11471186
async fn test_shuffle_reader_exec_coalesced_roundtrip_multi_group_mixed_sizes() {
11481187
let schema = create_test_schema();

0 commit comments

Comments
 (0)