grantatspothero commented on PR #16207: URL: https://github.com/apache/iceberg/pull/16207#issuecomment-4389286986
> Or in other words, if the REST server returns a reduced snapshotLog in REFS mode, than your problem is already solved without this PR, if it sends the full snapshotLog then this PR won't solve the caching overhead issues for you. Walking through the two points you mentioned above: 1. "REST server returns a reduced snapshotLog in REFS mode": This improves client side memory usage, but without snapshot lazy loading the iceberg TableMetadata interface is broken. Snapshot lazy loading is needed to maintain correctness and it is why lazy loading already exists in iceberg. 2. "If it sends the full snapshotLog then this PR won't solve the caching overhead". Yes, that is why this PR also modified `suppressHistoricalSnapshots` to remove snapshot log entries not referenced by REFs. This is called by REST catalog server side in REFs mode. Summary: we need both the client side and server side change. Server side suppression of historical snapshots in REFS mode and client side lazy loading of historical snapshots when requested. Can you help me understand what your commit is trying to test above? It removes the server side change to suppress historical snapshots in REFs mode, and as mentioned above we need both parts. -- 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]
