You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "stevenzwu (via GitHub)" <gi...@apache.org> on 2025/07/23 18:31:40 UTC

[PR] Core: Revert #12670 to Temporarily Restore Sending Single Snapshot Changes Rather than in Bulk (#13100) [iceberg]

stevenzwu opened a new pull request, #13647:
URL: https://github.com/apache/iceberg/pull/13647

   (no comment)


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Core: backport PR #13100 from 1.9 to main branch that partially revert 12670 by sending single snapshot rather than in bulk [iceberg]

Posted by "Hugo-WB (via GitHub)" <gi...@apache.org>.
Hugo-WB commented on PR #13647:
URL: https://github.com/apache/iceberg/pull/13647#issuecomment-4432217771

   Looking forward to re-merging this on 1.11! When expiring snapshots on large tables (100k+) snapshots to remove. Our REST Catalog struggles a lot due to how many times it hits `rewriteSnapshotsInternal`. Very slow + very large memory usage.
   
   Temporary fix for us has been was coalescing the RemoveSnapshots into a single large RemoveSnapshot which fixed the slowness.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Core: Revert #12670 to Temporarily Restore Sending Single Snapshot Changes Rather than in Bulk (#13100) [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #13647:
URL: https://github.com/apache/iceberg/pull/13647#discussion_r2226397030


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1424,14 +1424,12 @@ public Builder removeSnapshots(Collection<Long> idsToRemove) {
     private Builder rewriteSnapshotsInternal(Collection<Long> idsToRemove, boolean suppress) {
       List<Snapshot> retainedSnapshots =
           Lists.newArrayListWithExpectedSize(snapshots.size() - idsToRemove.size());
-      Set<Long> snapshotIdsToRemove = Sets.newHashSet();
-
       for (Snapshot snapshot : snapshots) {
         long snapshotId = snapshot.snapshotId();
         if (idsToRemove.contains(snapshotId)) {
           snapshotsById.remove(snapshotId);
           if (!suppress) {
-            snapshotIdsToRemove.add(snapshotId);
+            changes.add(new MetadataUpdate.RemoveSnapshots(snapshotId));

Review Comment:
   this is where it differs with the original PR #13100.
   
   `RemoveSnapshot` was removed in main branch for the deprecation cycle for 1.10. Switching to `RemoveSnapshots` as suggested by @amogh-jahagirdar. All servers should support `RemoveSnapshots` with a single snapshot id. 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Core: Revert #12670 to Temporarily Restore Sending Single Snapshot Changes Rather than in Bulk (#13100) [iceberg]

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #13647:
URL: https://github.com/apache/iceberg/pull/13647#discussion_r2226397030


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1424,14 +1424,12 @@ public Builder removeSnapshots(Collection<Long> idsToRemove) {
     private Builder rewriteSnapshotsInternal(Collection<Long> idsToRemove, boolean suppress) {
       List<Snapshot> retainedSnapshots =
           Lists.newArrayListWithExpectedSize(snapshots.size() - idsToRemove.size());
-      Set<Long> snapshotIdsToRemove = Sets.newHashSet();
-
       for (Snapshot snapshot : snapshots) {
         long snapshotId = snapshot.snapshotId();
         if (idsToRemove.contains(snapshotId)) {
           snapshotsById.remove(snapshotId);
           if (!suppress) {
-            snapshotIdsToRemove.add(snapshotId);
+            changes.add(new MetadataUpdate.RemoveSnapshots(snapshotId));

Review Comment:
   this is where it differs with PR #13100.
   
   `RemoveSnapshots` was removed in main branch for the deprecation cycle for 1.10. Switching to `RemoveSnapshots` as suggested by @amogh-jahagirdar. All servers should support `RemoveSnapshots` with a single snapshot id. 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Core: backport PR #13100 from 1.9 to main branch that partially revert 12670 by sending single snapshot rather than in bulk [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #13647:
URL: https://github.com/apache/iceberg/pull/13647


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org