Dandandan opened a new pull request, #23207:
URL: https://github.com/apache/datafusion/pull/23207

   ## Which issue does this PR close?
   
   <!-- No issue filed; self-contained robustness change. Happy to open one if 
preferred. -->
   
   - Closes #.
   
   ## Rationale for this change
   
   `WindowAggExec` buffers the **entire** input into `WindowAggStream.batches` 
and then `concat_batches` it into one contiguous copy (a transient ~2x peak) 
before computing — with **no** consultation of the task memory pool (there is 
currently no `MemoryReservation`/`MemoryConsumer` anywhere in the window 
operators). On a large unbounded-window input this can be OOM-killed by the OS 
rather than failing gracefully, and the usage is invisible to pool accounting / 
`EXPLAIN ANALYZE`.
   
   ## What changes are included in this PR?
   
   Registers a `MemoryConsumer` for the stream (mirroring `ExternalSorter`) and 
reserves:
   - **each buffered input batch** as it arrives, via a 
`RecordBatchMemoryCounter` so buffers shared across batches (e.g. zero-copy 
slices from an upstream Sort/Repartition) are counted **once**, not once per 
batch; and
   - the **freshly concatenated** batch in `compute_aggregates`, reflecting the 
transient doubling during the final compute.
   
   When the pool limit is exceeded, the operator returns `ResourcesExhausted` 
attributed to its consumer, matching `ExternalSorter`/`AggregateExec`.
   
   Scope note: this is accounting + backpressure only. It does **not** by 
itself reduce peak memory or add spilling (that would be follow-up work); it 
makes the existing usage visible and fail gracefully.
   
   ## Are these changes tested?
   
   Yes:
   - A new unit test asserts the operator errors with `ResourcesExhausted` 
(naming its `WindowAggStream` consumer) under a tiny (100-byte) memory pool.
   - The full `window` sqllogictest suite passes under the default pool.
   
   ## Are there any user-facing changes?
   
   The default memory pool is unbounded, so **default behavior is unchanged**. 
Queries running against a *bounded* pool that previously over-committed may now 
fail deterministically with `ResourcesExhausted` instead of being OOM-killed — 
the intended behavior.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to