adriangb opened a new pull request, #11239: URL: https://github.com/apache/arrow-rs/pull/11239
> [!NOTE] > This PR is stacked on https://github.com/apache/arrow-rs/pull/11238. Only the last commit is new (+189/−18). I will rebase when https://github.com/apache/arrow-rs/pull/11238 merges. # Which issue does this PR close? - Part of https://github.com/apache/arrow-rs/issues/11234 (step F). # Rationale for this change With `FetchGranularity::Batch` from https://github.com/apache/arrow-rs/pull/11238, the decoder requests one batch at a time, but it holds the pages of a row group until the row group ends. Its peak memory is still one row group. With this PR, the decoder releases each data page when all readers of its column have passed it. Prototype measurement (one row group of 84 MB, 50 ms request latency, caller read-ahead of 8 MB): | Mode | Peak buffered bytes | |---|---:| | `FetchGranularity::RowGroup` | 84 MB | | `FetchGranularity::Batch` with this PR | 8.1 MB | # What changes are included in this PR? | Bytes | Before this PR | With this PR | |---|---|---| | Data page | at the end of the row group | after all readers of its column have passed its rows | | Dictionary page | at the end of the row group | at the end of the row group | | Column chunk without an offset index | at the end of the row group | at the end of the row group | `IncrementalRowGroup::release_passed_pages` computes, for each read column, the first row that a reader can read again, and releases the pages that end before it: | Reader of the column | First row that it can read again | |---|---| | a predicate | the start of the current window | | the output | the first row of the output batch, else of the queue. Not after the start of the current window. | The page is removed from the `PageStore` and from `PushBuffers` (also bytes that the caller pushed ahead). `FetchGranularity::Batch` documents this in its *Memory* section. The decoder now makes many small releases. https://github.com/apache/arrow-rs/pull/11237 (sorted `PushBuffers`) makes each release a binary search. This PR is correct without it. The predicate cache PR (step G of https://github.com/apache/arrow-rs/issues/11234) must add one rule to `release_passed_pages` if it merges after this PR: a column that the output reads from the cache is released at cache-batch boundaries. # Are these changes tested? Yes. These tests fail without the change: - `test_decoder_first_pages_only_batch_granularity`: after the first batch, only the dictionary pages stay buffered. - `full_scan`: the peak of `buffered_bytes()` is less than a third of a row group. - `releases_bytes_pushed_ahead`: the caller pushes two row groups, one buffer per page. The buffered bytes decrease with each batch. - `releases_parts_of_one_buffer`: the caller pushes the file as one buffer. The buffered bytes decrease with each batch. The equivalence tests of https://github.com/apache/arrow-rs/pull/11238 also run with the release, and they check that no range is requested two times. # Are there any user-facing changes? `buffered_bytes()` decreases during a row group with `FetchGranularity::Batch`. There is no API change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
