apache / apache/datafusion

Clarify memory-limit guarantees for external-sort spilling

Open
#25,183 2 comments 0 reactions 1 assignee Claimed by @Phoenix500526 View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Is your feature request related to a problem or challenge?

Review of #24923 raised a question about external-sort memory-limit behavior that is separate from adding asynchronous spill I/O.

The original synchronous spill path can receive an already-built output `RecordBatch`, fail to reserve additional memory for it, and still try to spill that batch. A failed reservation at this handoff does not, by itself, cause the query to fail.

One iteration of #24923 reused available spill workspace but returned `ResourcesExhausted` if the workspace and a new reservation could not cover the output batch. This introduced a new failure condition for tight-budget queries that could previously continue spilling. The reviewer also pointed out that rejecting an already-built batch does not prevent its allocation.

Spill workspace is reserved memory budget, not a preallocated buffer. Likewise, retaining accounting for a resident batch while an asynchronous write is pending is a separate concern from deciding whether insufficient budget should terminate the query.

### Describe the solution you'd like

I'd like to clarify the intended guarantees before changing this behavior:

- What should the configured memory limit guarantee during external-sort spilling, including the handoff of an already-built output batch?
- How should resident spill output and temporary budget overages be accounted for and documented?
- If stronger enforcement is desired, where should checks happen before allocation, and how should we handle compatibility for queries that currently succeed with the same memory settings?

The outcome could be documentation of the existing policy, or agreement on a separate behavior change. This issue does not assume that stricter enforcement is required; it keeps that decision separate from the async spill API work.

### Describe alternatives you've considered

Preserve the existing ability to continue spilling after a failed output reservation, while keeping resident output accounted for during asynchronous writes.

Alternatively, design stronger enforcement as a separate change with an explicit compatibility discussion. Simply returning an error after output construction would change query behavior without preventing that allocation.

### Additional context

- [Review discussion on #24923](https://github.com/apache/datafusion/pull/24923#issuecomment-5620074706).
- #14748 discusses more accurate external-sort memory accounting.
- #22898 discusses allocation-time enforcement versus accounting after construction through Arrow memory-pool integration.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.