mbutrovich opened a new pull request, #4008:
URL: https://github.com/apache/iceberg-python/pull/4008

   Partially addresses #4007
   
   # Rationale for this change
   
   `ManifestWriter.add_entry` calls `AvroOutputFile.write_block` once per entry 
([`manifest.py#L1213-L1238`](https://github.com/apache/iceberg-python/blob/0d584073f67948c46f34d4c2a1672455bc10b5c9/pyiceberg/manifest.py#L1213-L1238)),
 and `write_block` writes exactly one Avro block for the list it is given. A 
manifest with 10,000 entries is therefore 10,000 blocks, each with its own 
deflate stream and 16-byte sync marker. Every stream starts cold, so 
compression barely applies, and every reader pays per-block setup to read a 
single record.
   
   This change buffers encoded records in `AvroOutputFile` and flushes a block 
once the buffer reaches a sync interval. The default is 64,000 bytes, which is 
what Avro Java's `DataFileWriter` uses 
(`DataFileConstants.DEFAULT_SYNC_INTERVAL`, `4000 * SYNC_SIZE`) and what 
Iceberg Java writes with, since it never calls `setSyncInterval`. The Iceberg 
spec says nothing about block sizes, so the value is a writer choice rather 
than a requirement.
   
   Measured on 10,000 entries with 12 columns and full column stats, reading 
through `ManifestFile.fetch_manifest_entry`, best of 7 runs:
   
   | codec | | blocks | size | read |
   |---|---|--:|--:|--:|
   | deflate | before | 10,000 | 3.59 MB | 106.5 ms |
   | deflate | after | 75 | 0.73 MB | 75.7 ms |
   | null | before | 10,000 | 4.97 MB | 82.2 ms |
   | null | after | 75 | 4.78 MB | 74.2 ms |
   
   The default was chosen by sweeping the interval on the same data with 
deflate, rather than by copying Java:
   
   | sync interval | blocks | size | read |
   |--:|--:|--:|--:|
   | 4,000 | 1,112 | 1.06 MB | 80.9 ms |
   | 16,000 | 295 | 0.80 MB | 77.6 ms |
   | 64,000 | 75 | 0.73 MB | 75.6 ms |
   | 128,000 | 38 | 0.72 MB | 76.2 ms |
   | 256,000 | 19 | 0.72 MB | 76.6 ms |
   | 1,000,000 | 5 | 0.71 MB | 78.6 ms |
   
   Both curves flatten at 64,000, and larger intervals read slightly slower. 
With the `null` codec the file size is flat from 4,000 upward, so the size win 
comes from giving the compressor a larger window rather than from fewer sync 
markers.
   
   # Are these changes tested?
   
   Yes, in `tests/avro/test_file.py` and `tests/utils/test_manifest.py`.
   
   - A manifest of 2,000 entries has fewer blocks than entries, every block 
except the last reaches the sync interval, the record counts add up, and 
`fetch_manifest_entry` returns all entries in order across block boundaries. 
Covered for V1 and V2, with both `null` and `deflate`.
   - The same for `write_manifest_list` with 2,000 manifests, which now spans 
multiple blocks because that writer passes its whole list in one call.
   - A non-positive sync interval raises `ValueError`.
   - A writer that is given no records writes no block at all.
   - A record that on its own reaches the interval is flushed as its own block, 
checked with `sync_interval=1`.
   - `tell()` grows after a record that is still buffered.
   
   Coverage of `pyiceberg/avro/file.py` is 97%, and every added line is 
executed. The uncovered lines are pre-existing.
   
   # Are there any user-facing changes?
   
   Manifests and manifest lists that PyIceberg writes now contain fewer, larger 
blocks. The file format does not change, so any Avro reader reads them, and 
files written by earlier versions still read the same way. Manifests get 
substantially smaller when a compression codec is set.
   
   `AvroOutputFile` gains an optional `sync_interval` argument. No existing 
signature or behavior changes, and no table property is added, matching Java, 
which does not expose the setting. `tell()` now includes buffered bytes so that 
it stays monotonic per entry, which keeps `ManifestWriter.tell` meaningful for 
callers that use it to decide when to roll over to a new manifest.
   
   ## AI disclosure
   
   Developed with the help of Claude Code, but I understand and support these 
changes.
   


-- 
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