nastra commented on code in PR #8096:
URL: https://github.com/apache/iceberg/pull/8096#discussion_r1277233190
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1646,7 +1674,7 @@ private static List<HistoryEntry> updateSnapshotLog(
List<HistoryEntry> newSnapshotLog = Lists.newArrayList();
for (HistoryEntry logEntry : snapshotLog) {
long snapshotId = logEntry.snapshotId();
- if (snapshotsById.containsKey(snapshotId)) {
+ if (snapshotsById.containsKey(snapshotId) ||
lazySnapshotLoadingEnabled) {
Review Comment:
In my initial version of the code I had almost the same logic, but it was a
bit difficult to understand because we basically need to handle the snapshot
log handling both for the client and the server side.
I'm also not a fan of exposing `removeLazilyLoadedSnapshotsIf()`, but it
makes the intent a bit clearer.
We've also talked with @danielcweeks to explore lazy metadata loading in
general and I've opened https://github.com/apache/iceberg/pull/8173 so that we
can explore both options. This would allow solving us this particular issue but
also allow the server to send a "slim" version of `TableMetadata` and load
remaining data when requested. In particular, snapshots / history / metadata
log can become quite long.
--
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]
