HaoXuAI commented on PR #17219: URL: https://github.com/apache/iceberg/pull/17219#issuecomment-4997377772
Thanks for the review! Let me address each point. **How it's used / not wired into `TableMetadataParser`.** Intentional for this PR — it's a standalone reader so it can be adopted incrementally. The intended integration is to let `TableMetadataParser.read` install a streaming-backed `snapshotsSupplier`, so `loadTable` paths that never call `snapshots()` skip materializing the array entirely. I kept that out of this PR to land the reader first; happy to follow up with the integration if you're open to the direction. **File descriptors.** The returned `CloseableIterable` isn't attached to a `TableMetadata` and holds nothing for a table's lifetime. Each iteration opens a fresh stream and closes it on exhaustion or `close()`, so caching a table keeps no descriptor open. **1 IO per snapshot?** No — it makes a single pass over the one `metadata.json` and yields every snapshot from it (1 read, not N). For operations that genuinely need all snapshots (e.g. expire), it does equivalent work to the eager path; the win is for consumers that need only a subset. **Numbers.** Measured on a `metadata.json` with 200k snapshots (39 MB); read time from a JMH benchmark, heap from a fixed-`-Xmx` probe: | Metric | Eager `TableMetadataParser.read` | Streaming `SnapshotParser.fromJson` | | --- | --- | --- | | Peak retained heap (200k) | 49 MB | 1 MB | | Min heap to complete (200k) | OOM at `-Xmx128m` | completes at `-Xmx64m` | | Full read, 100k snapshots | ~79 ms | ~53 ms | | Subset lookup, 100k snapshots | loads all first | ~25 ms | So there's no read-overhead tradeoff for the full-scan case (streaming is if anything slightly faster, since it skips the intermediate JSON tree and the snapshot list), and a large memory saving plus faster subset access. -- 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]
