Skip to content

fix: bound sort-merge join spill readback memory - #94

Merged
Vedin merged 1 commit into
embucket-sync-df55.0.0from
autoresearch/smj-bounded-restore
Sep 15, 2026
Merged

Vedin merged 1 commit into
embucket-sync-df55.0.0from
autoresearch/smj-bounded-restore

Conversation

@Vedin

@Vedin Vedin commented Sep 15, 2026

Copy link
Copy Markdown

Which issue does this PR close?

No existing issue. Reproduced by the fork's existing
smj_with_mem_limit_1 RSS validation and the new focused regression below.

Rationale for this change

A sort-merge join with a large equal-key group can spill during ingestion and
then progressively load the whole group back into memory. Already-visited
payloads remain decoded until the group is removed. On the pinned baseline,
the existing RSS guard fails in all three predetermined runs: 198.6, 197.7 and
198.2 MiB against its unchanged 152.6 MiB allowance.

With this standalone fix the same guard passes all three repeats at 84.5, 84.2
and 84.2 MiB. These are the guard's RSS increases over its reference query,
not absolute process RSS. Neither its assertions nor allocator settings were
changed to obtain this result.

What changes are included in this PR?

  • Keep the original spill file while a batch is decoded. Evict restored payloads
    outside the next output's working set, releasing their reservations without
    rewriting spill files.
  • Restore Full-join batches with pending null output and preserve restoration
    of the dequeued head for deferred filter failures.
  • Keep cached indices/file lifetimes correct when the buffered head is removed.
  • Add a regression and module maintenance/readback documentation.
  • Include one independent test-only prerequisite in aggregate.slt: the
    pinned planner already rejects nonexistent SELECT * REPLACE targets. Update
    its stale expected success to the exact existing error and retain a positive
    grouped/HAVING replacement case. No planner/runtime SQL change or relaxed check.

The four separate autoresearch runtime optimizations are deliberately excluded.

Are these changes tested?

Local verification uses DataFusion 15ad0e236, Rust 1.97.0 and the application's
Arrow/Parquet fork pin cabd25a16, supplied through a Cargo patch configuration.
The target fork branch is currently 31a96ad54 (55.1); the touched runtime/test
files are unchanged between these bases. This PR contains no Cargo pin change
or dependency downgrade. The target branch's eventual merge result is not
claimed as a locally tested tree; the results below are for the exact PR head.

  • Ablation: the new test fails on unchanged SMJ code at 1,598,976 reserved bytes
    against a 376,008-byte bound. All 72 SMJ tests pass with the fix.
  • The regression compares exact spilled/no-spill output for inner/left/right/
    full joins, revisits a 64-batch key group, and checks reservation cleanup after
    completion and early cancellation.
  • cargo fmt --all and all-target/all-feature Clippy with -D warnings: pass.
  • Required extended workspace command: 10,541 Rust tests pass, zero failures,
    eight pre-existing ignored cases; all 508 SQL-logic files pass. Includes the
    core integration RSS runners and 114 fuzz tests.
  • Three additional executions of the exact original RSS guard: all pass, using
    one SHA-verified executable.

The existing SMJ SQL benchmark also completed 24 runs (Q01/Q03/Q08/Q23, three
alternating baseline/fixed pairs), selecting SMJ and matching every expected row
count. With two partitions, batch size 8192 and a 1 GiB pool, median time changes were
−4.24%/−7.88%/+3.49%/+1.30%. These are an unoptimized CI-profile local screen,
not a release speedup claim; the small Full-join regressions are retained. This
screen does not quantify extra read I/O when revisiting large spilled groups.

The full unchanged dev/rust_lint.sh also passes, including its alternate-feature
Clippy, TOML format, CI-pinned license/spelling checkers, Prettier, workflow check
and private-item workspace rustdoc with -D warnings.

Are there any user-facing changes?

Lower accumulation of decoded spill payloads, without a SQL/wire API change,
new runtime dependency or dataset-specific branch. Repeated streamed keys may
require extra spill reads after eviction: this is a memory/I/O tradeoff, not a
claim that every spilled join becomes faster. Join keys, match state, output
batches and the current restore window still consume memory; this is not a
strict total-process RSS or memory-pool cap.

This fix is separate from, and not included in, the frozen E21 local/SPCS
performance measurements.

Retain spill backing files and evict decoded payloads outside the next output working set. Preserve Full-join null output and deferred filter failures. Add exact spill/no-spill, memory-bound, revisit and cancellation coverage.

Include the independent stale aggregate.slt REPLACE fixture repair needed by the full workspace gate; SQL planner behavior and the original SMJ RSS allowance remain unchanged. Standalone extended workspace, all-feature Clippy, full rust_lint.sh, and three original RSS guard repeats pass. The separate autoresearch runtime optimizations are excluded.
@Vedin
Vedin merged commit 8775702 into embucket-sync-df55.0.0 Sep 15, 2026
35 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants