[vuanhphung]

What type of PR is this?

• Other (performance)

Description

Large Thrift CloudFetch results used 1.3-1.7x the Arrow table size in peak RSS (a 1.7 GB fetchall_arrow() peaked at +2.2 GB). Arrow IPC reads are already zero-copy, so the extra memory was not a second copy of the data. memray and glibc malloc_stats() show it is allocator fragmentation: python-lz4 allocates each decompressed file inside a download thread's malloc arena, next to that thread's short-lived response bodies. At peak, 17 arenas held 752 MB of free-but-resident memory, and the overhead grows across repeated fetches in one process.

This PR decompresses each file with pyarrow's LZ4 frame codec (CompressedInputStream.readinto) into its own private anonymous mmap, sized from the link's bytesNum. The Arrow table still references the buffer zero-copy. The mapping is returned to the OS when the table is released. The decompressed size is checked against the stream, and the old python-lz4 path is used when pyarrow or its codec is missing, when bytesNum is too small, when the data is corrupt (so the error is unchanged), and on SEA, where bytesNum is the compressed size. As a side effect, decompression is faster: multi-frame files no longer go through python-lz4's GIL-bound chunk loop. Only cloudfetch/downloader.py changes; inline results and the kernel backend are untouched.

How is this tested?

• Unit tests
• E2E Tests
• Manually

New unit tests cover the mmap path (private mapping, exact size when bytesNum is too large, fallback when it is too small, missing, equal to the compressed size, or when mmap fails), concatenated frames, corrupt and truncated input raising the same RuntimeError as before, and no Arrow codec. They also pass with pyarrow 14.0.2. The full unit suite passes. E2E: TestPySQLLargeWideResultSet, TestPySQLLargeNarrowResultSet (Thrift and SEA, lz4 on and off), and test_complex_types.py pass. CloudFetch and inline results were identical for 2M rows of mixed types (nulls, decimals, timestamps, dates, arrays, structs, maps) via fetchall_arrow, fetchmany_arrow, and fetchall.

Peak RSS increase during the fetch, live Thrift warehouse, median of 5 fresh processes per cell, interleaved with main (13 runs for 1.7 GB fetchall_arrow):

Workload	main	this PR
fetchall_arrow() 110 MB	182 MB	151 MB (-17%)
fetchall_arrow() 562 MB	750 MB	599 MB (-20%)
fetchall_arrow() 1.7 GB	2226 MB	1818 MB (-18%)
fetchmany_arrow(100k) loop, 562 MB	233 MB	132 MB (-43%)
fetchmany_arrow(100k) loop, 1.7 GB	352 MB	195 MB (-45%)
fetchall() rows, 562 MB	8450 MB	8281 MB (-2%)

Live wall time is network-bound and within noise. With the same captured files served locally over HTTPS with 60 ms latency (5 runs), fetchall_arrow() went from 0.56 s to 0.17 s at 110 MB and from 3.84 s to 3.61 s at 1.7 GB. RSS after the result is freed is unchanged on the warehouse (~150 MB). Four back-to-back 1.7 GB fetches in one process peak at 2160 MB instead of 3132 MB.

Related Tickets & Documents

None.

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 2 Low

Looks good overall: the mmap path is well guarded, falls back correctly, and the edge cases are well tested. Two low concerns: per-file mappings add up on very large results, and no test runs run() end-to-end with real data. Nit: convert_arrow_based_file_to_arrow_table / create_arrow_table_from_arrow_file in utils.py are still annotated file_bytes: bytes, but they now also receive pyarrow.Buffer.

[vuanhphung]

Review-body nit: done in a9b039a. create_arrow_table_from_arrow_file and convert_arrow_based_file_to_arrow_table now take Union[bytes, pyarrow.Buffer]. The same push fixes the check-types failure on Python 3.13+: typeshed's mmap() has a trackfd: bool parameter there, so the **dict[str, int] flags call didn't type-check; flags are now passed explicitly.

[peco-review-bot]

Verdict: 1 Low

Looks good overall; one low-severity note on the fallback cost. The mmap path is limited to Thrift-style bytesNum (larger than the compressed length), falls back cleanly, and returns a pyarrow.Buffer that works with both len() in run() and ipc.open_stream. The tests cover the boundary sizes, concatenated frames, and corrupt input. Nit: the CompressedInputStream is never closed explicitly; this is harmless because garbage collection handles it.

[vuanhphung]

Re the second review's nit about the unclosed CompressedInputStream: leaving it as is. It wraps an in-memory BufferReader and holds no OS resources, and it goes away when the helper returns, as the review notes.