Increase the default PyArrow IO buffer to 8 MiB - #3971
kevinjqliu wants to merge 1 commit into
Conversation
Use one buffer default across FileIO and Parquet readers while preserving explicit overrides. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Retain or deprecate the removed public ONE_MEGABYTE symbol.
Pull request overview
This PR increases PyArrow IO and Parquet reader default buffers from 1 MiB to 8 MiB while preserving explicit overrides.
Changes:
- Applies the 8 MiB default to streams and Parquet readers.
- Adds coverage for defaults and custom buffer sizes.
File summaries
| File | Summary |
|---|---|
tests/io/test_pyarrow.py |
Tests constructor and factory buffer defaults and overrides. |
pyiceberg/io/pyarrow.py |
Updates buffering defaults; retain the existing public ONE_MEGABYTE alias for compatibility. |
Review details
Suppressed comments (1)
pyiceberg/io/pyarrow.py:199
- This removes the existing non-private
pyiceberg.io.pyarrow.ONE_MEGABYTEsymbol even though this module is imported directly and does not define__all__. The buffer default can change without breaking callers that import the constant; please retain the old alias (or deprecate it) while defining the new 8 MiB default.
_DEFAULT_BUFFER_SIZE = 8 * 1024 * 1024 # 8 MiB
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rambleraptor
left a comment
There was a problem hiding this comment.
Looks great! Surprised we don't have existing tests around buffer size. One small nit. Thanks a lot!
|
|
||
| ONE_MEGABYTE = 1024 * 1024 | ||
| BUFFER_SIZE = "buffer-size" | ||
| _DEFAULT_BUFFER_SIZE = 8 * 1024 * 1024 # 8 MiB |
There was a problem hiding this comment.
This doesn't really need to be a private variable. None of the others are.
|
The local benchmarks you did look great. It looks like a huge speed up on larger cases with basically no impact on the smaller cases. (0.2ms is just small enough where I think there's the possibility of some noise there) |
Rationale for this change
I've been thinking about doing this for a long time 😄 and was just reminded of this recently.
I think 8MB buffer is a good default for fetching from object store.
This PR raise the PyArrow FileIO buffer default from 1 MiB to 8 MiB. Use the same default for input/output streams and the Parquet scan and positional-delete readers. Regular Parquet scans was already specified 8 MiB.
8 MiB is a reasonable starting point for sequential object-store reads. Hadoop’s S3 prefetcher and Trino’s Parquet reader use that size for read blocks. These are precedents, not directly comparable benchmarks.
Local benchmark
I compared 1 MiB and 8 MiB buffers against RustFS in Docker, using PyArrow 25.0.1 and Zstd Parquet files. These are whole-file downloads with
open(seekable=False)and 64 KiB reads. Numbers are medians of 10 paired runs after warmup, with buffer order shuffled.Times and GETs show 1 MiB → 8 MiB. Improvement means less download time. Direct runs bypass the proxy; +10 ms adds a delay per HTTP request through the proxy, not real S3 latency.
Fewer GETs help more as latency increases. At 30 ms/request, the largest file went from 19.63 to 3.69 seconds. The smallest file saw no benefit.
The larger buffer helps download big files with fewer requests. For the ~512 MiB file, sequential downloads went from 512 GETs to 64. The tradeoff is 7 MiB more memory per buffered reader.
KEVIN: This result is pretty intuitive. We were fetching via 1MB requests multiple times before, now we're doing 8MB which leads to fewer requests
Are these changes tested?
Added assertions for the constructor default and both FileIO factories, with and without a buffer-size override. Focused PyArrow and Avro tests pass.
Are there any user-facing changes?
The default stream buffer increases to 8 MiB for reads and writes, including output files converted to input files. Existing
buffer-sizeoverrides still work. Seekable reads and Arrow’s S3 multipart upload size are unchanged.