akupchinskiy opened a new pull request, #27084:
URL: https://github.com/apache/datafusion/pull/27084
## Which issue does this PR close?
- Closes #27083.
## Rationale for this change
`RecursiveQueryExec` reserves `get_array_memory_size()` for every batch it
keeps for the next
iteration. When the recursive term is a hash join over `Utf8View` columns,
every output batch
references the build side's string buffers, so the same buffers get reserved
once per batch. A
recursive CTE over Parquet string keys can then fail with a pool many times
larger than the memory
it actually uses. The issue has a repro.
## What changes are included in this PR?
`RecursiveQueryStream` now keeps a `RecordBatchMemoryCounter`, like the hash
join, asof join and
symmetric hash join do for their buffered batches. `push_batch` reserves
only the buffers and array
objects it hasn't seen yet (`count_batch_with_array_overhead`), and the
counter starts over when the
batches move to the work table for the next iteration.
This doesn't make the accounting exact. A buffer can still be counted by
more than one reservation:
by the work table and by the next iteration when the recursive term passes
work-table columns
through, or by `RecursiveQueryExec` and the hash join that produced it. So
it's counted once per
reservation that holds it, instead of once per batch.
## What is the testing strategy for this PR?
- `recursive_query_reserves_shared_buffers_once_per_iteration`: batches that
share string buffers
go through two iterations. The peak reservation has to be at least twice
the string data (the
work table and the next iteration both hold it) and below the old
per-batch sum. It fails with
the old accounting and without the counter reset.
- `recursive_query_reserves_array_objects`: `NullArray` batches have no
buffers, but their array
objects still have to be reserved.
- A `cte.slt` case: a recursive walk over Parquet string keys with a
`CollectLeft` join and a 50 MB
limit. Without the fix it fails asking for 76.4 MB with nothing reserved
yet, for about 2.4 MB of
string data.
## Are there any user-facing changes?
No API changes. Recursive queries whose buffered batches share buffers need
much less of the memory
pool.
## AI disclosure
I found this while comparing query engines on a benchmark of mine (a
recursive span-tree query
failed under a 64 GB memory limit, where DuckDB ran it in 1 GB). Claude Code
helped write the fix,
the tests and this description, and Codex did a review pass. I reviewed the
code and verified it
myself: the tests above, with and without the fix.
--
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]
