Skip to content

Commit 610d70c

Browse files
committed
vortex-datafusion: stop scan concurrency multiplying with partitions
A Vortex scan runs `concurrency * available_parallelism` splits per stream, and DataFusion opens one stream per partition, one per core by default. Vortex's default concurrency of 4 multiplied with that: TPC-H SF1000 on 96 cores kept ~37k decoded splits in flight, and DataFusion Q1 on Vortex peaked at 115 GiB against Parquet's 37 GiB. With concurrency 1 it peaked at 31 GiB and ran faster (8.7s vs 10.0s). Default the DataFusion scan concurrency to 1 when the table options leave it unset; `vortex.scan_concurrency` still overrides it. DO NOT MERGE as-is: this commit also points the diagnosis run at the fix, with and without a memory limit, in its own concurrency group. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDifRZhxcVd5gvt3dsDhu5
1 parent d0e9a1b commit 610d70c

4 files changed

Lines changed: 27 additions & 22 deletions

File tree

‎.github/workflows/tpch-sf1000.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
name: TPC-H SF1000 (OOM diagnosis)
1212

1313
concurrency:
14-
group: tpch-sf1000-oom-diag
14+
group: tpch-sf1000-oom-diag-${{ github.sha }}
1515
cancel-in-progress: false
1616

1717
on:

‎scripts/tpch-oom-diag.sh‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,15 @@
33
# SPDX-License-Identifier: Apache-2.0
44
# SPDX-FileCopyrightText: Copyright the Vortex contributors
55

6-
# DO NOT MERGE: tries DataFusion with a memory limit and spilling at TPC-H SF1000.
7-
#
8-
# With DATAFUSION_BENCH_MEMORY_LIMIT_GIB set, aggregations and sorts spill to disk and hash join
9-
# builds that do not fit fail with a resources-exhausted error instead of growing until the
10-
# process is killed. prefer_hash_join=false swaps hash joins for sort-merge joins, which spill.
6+
# DO NOT MERGE: checks whether DataFusion runs TPC-H SF1000 with the Vortex scan concurrency fix
7+
# (one split per core per stream by default), with and without a memory limit and spilling.
118

129
set -Eeuo pipefail
1310

1411
BIN=${BIN:-target/release_debug/datafusion-bench}
1512
ALL_QUERIES=(1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22)
16-
JOIN_QUERIES=(9 18 21)
1713
SPILL_DIR="${GITHUB_WORKSPACE:-$PWD}/df-spill"
18-
19-
# The memory pool does not track scan buffers or batches in flight, so leave headroom below the
20-
# machine's 188 GiB for them.
21-
export DATAFUSION_BENCH_MEMORY_LIMIT_GIB=${DATAFUSION_BENCH_MEMORY_LIMIT_GIB:-100}
22-
export DATAFUSION_BENCH_SPILL_DIR="$SPILL_DIR"
14+
RUNNER="ec2_${RUNS_ON_INSTANCE_TYPE:-unknown}"
2315
export RUST_BACKTRACE=0
2416

2517
run_variant() {
@@ -40,12 +32,16 @@ run_variant() {
4032
rm -rf "$SPILL_DIR"
4133
}
4234

43-
run_variant parquet-spill parquet "${ALL_QUERIES[*]}"
44-
run_variant vortex-spill vortex "${ALL_QUERIES[*]}"
45-
run_variant vortex-spill-scan1 vortex "${ALL_QUERIES[*]}" VORTEX_SCAN_CONCURRENCY=1
46-
run_variant parquet-spill-smj parquet "${JOIN_QUERIES[*]}" DATAFUSION_OPTIMIZER_PREFER_HASH_JOIN=false
47-
run_variant vortex-spill-scan1-smj vortex "${JOIN_QUERIES[*]}" \
48-
VORTEX_SCAN_CONCURRENCY=1 DATAFUSION_OPTIMIZER_PREFER_HASH_JOIN=false
35+
run_variant vortex-fix vortex "${ALL_QUERIES[*]}"
36+
run_variant vortex-fix-spill vortex "${ALL_QUERIES[*]}" \
37+
DATAFUSION_BENCH_MEMORY_LIMIT_GIB=100 DATAFUSION_BENCH_SPILL_DIR="$SPILL_DIR"
38+
run_variant parquet-spill parquet "${ALL_QUERIES[*]}" \
39+
DATAFUSION_BENCH_MEMORY_LIMIT_GIB=100 DATAFUSION_BENCH_SPILL_DIR="$SPILL_DIR"
40+
41+
# Q2 and Q16 still used more memory on Vortex than on Parquet with the old scan settings, and
42+
# only responded to the partition count; compare their plans.
43+
"$BIN" tpch --explain --formats parquet,vortex --queries 2,16 --runner "$RUNNER" \
44+
--opt scale-factor="$SCALE_FACTOR" > "$OUT_DIR/explain-q2-q16.txt" 2>&1 || true
4945

5046
for dir in "$OUT_DIR"/*/; do
5147
[[ -f "$dir/queries.jsonl" ]] || continue

‎vortex-datafusion/src/persistent/format.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ extensions_options! {
223223
/// This does not affect the overall parallelism
224224
/// across partitions, which is controlled by DataFusion's execution configuration.
225225
///
226-
/// Leave as `None` to use Vortex's scan default. Override per session
226+
/// Leave as `None` for one split per core per stream: DataFusion already runs one stream
227+
/// per partition, so Vortex's own default would multiply with it. Override per session
227228
/// with `SET vortex.scan_concurrency = <n>`, or per table with
228229
/// `OPTIONS(scan_concurrency '<n>')`.
229230
pub scan_concurrency: Option<usize>, default = None

‎vortex-datafusion/src/persistent/opener.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ use crate::persistent::cache::CachedVortexMetadata;
7070
use crate::persistent::reader::VortexReaderFactory;
7171
use crate::persistent::stream::PrunableStream;
7272

73+
/// Per-core scan concurrency when the table options leave it unset. It is lower than the Vortex
74+
/// scan default because DataFusion already runs one scan stream per partition.
75+
const DATAFUSION_DEFAULT_SCAN_CONCURRENCY: usize = 1;
76+
7377
#[derive(Clone)]
7478
pub(crate) struct VortexOpener {
7579
/// The partition this opener is assigned to. Only used for labeling metrics.
@@ -409,9 +413,13 @@ impl FileOpener for VortexOpener {
409413
scan_builder = scan_builder.with_limit(limit);
410414
}
411415

412-
if let Some(concurrency) = scan_concurrency {
413-
scan_builder = scan_builder.with_concurrency(concurrency);
414-
}
416+
// The scan runs `concurrency * available_parallelism` splits per stream, and DataFusion
417+
// already opens one stream per partition (one per core by default). Vortex's default of
418+
// 4 multiplies with that: at SF1000 on 96 cores it kept ~37k decoded splits in flight
419+
// and Q1 peaked at 115 GiB against Parquet's 37 GiB. One split per core per stream
420+
// keeps every core busy without the second multiplier.
421+
scan_builder = scan_builder
422+
.with_concurrency(scan_concurrency.unwrap_or(DATAFUSION_DEFAULT_SCAN_CONCURRENCY));
415423

416424
// Set before the byte-range translation below, which computes natural splits for
417425
// the fields the scan's projection and filter reference.

0 commit comments

Comments
 (0)