You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "aihuaxu (via GitHub)" <gi...@apache.org> on 2025/05/19 21:55:10 UTC
[PR] core: Send single snapshot to remove rather than in bulk [iceberg]
aihuaxu opened a new pull request, #13100:
URL: https://github.com/apache/iceberg/pull/13100
https://github.com/apache/iceberg/commit/06f667ada5a5b9edeaa20ae9269ff5de1721b91d introduces a behavior change that requires the REST catalog server to upgrade to 1.9.x as well, otherwise, the client with 1.9.x cannot talk to the existing server.
This PR is to revert back the client to send single snapshot to remove that the existing server can understand, while the server now can accept receiving single snapshot or bulk snapshots.
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#discussion_r2098703160
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1838,11 +1832,7 @@ private static List<HistoryEntry> updateSnapshotLog(
Set<Long> intermediateSnapshotIds = intermediateSnapshotIdSet(changes, currentSnapshotId);
boolean hasIntermediateSnapshots = !intermediateSnapshotIds.isEmpty();
boolean hasRemovedSnapshots =
- changes.stream()
- .anyMatch(
- change ->
- change instanceof MetadataUpdate.RemoveSnapshots
- || change instanceof MetadataUpdate.RemoveSnapshot);
Review Comment:
Shouldn't this part remain as is? That way if a server is handling the bulk update we still update the history entries. I know clients won't send it in this release, but it'd make the change more minimal
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2894989473
@RussellSpitzer I'm making the change to make `snapshot expiry` compatible with existing REST catalog server since you are releasing 1.9.x.
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2893077764
Should this be targeting 1.9.x only maybe? From how I understand things is that we want to produce a single snapshot to be removed for 1.9.1 and then get back to the existing code and potentially produce multiple snapshots during removal for 1.10.0 (main)
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2898465593
Thank @aihuaxu, thanks to @amogh-jahagirdar for review
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2894957903
@nastra and @amogh-jahagirdar Can you please review again? Thanks.
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2894929376
> Yeah @aihuaxu I think this should be targeting the 1.9.x branch. I think the main branch can be left as is, since on the next release we'd have the ability for both clients/servers to work.
>
> The change itself looks right but marking as requesting changes just to prevent merges into main (but please let me know if folks disagree with only merging this into 1.9.x)
I think that works. Basically we are expecting the server to upgrade in 1.9.x and the clients finish the upgrade in 1.10.x. Let me make the change.
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer merged PR #13100:
URL: https://github.com/apache/iceberg/pull/13100
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#issuecomment-2892398076
@amogh-jahagirdar and @ricardopereira33 Can you please take a look? Thanks.
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#discussion_r2098703160
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1838,11 +1832,7 @@ private static List<HistoryEntry> updateSnapshotLog(
Set<Long> intermediateSnapshotIds = intermediateSnapshotIdSet(changes, currentSnapshotId);
boolean hasIntermediateSnapshots = !intermediateSnapshotIds.isEmpty();
boolean hasRemovedSnapshots =
- changes.stream()
- .anyMatch(
- change ->
- change instanceof MetadataUpdate.RemoveSnapshots
- || change instanceof MetadataUpdate.RemoveSnapshot);
Review Comment:
Shouldn't this part remain as is? That way if a server is handling the bulk update we still update the history entries
--
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: Send single snapshot to remove rather than in bulk [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on code in PR #13100:
URL: https://github.com/apache/iceberg/pull/13100#discussion_r2098728228
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1838,11 +1832,7 @@ private static List<HistoryEntry> updateSnapshotLog(
Set<Long> intermediateSnapshotIds = intermediateSnapshotIdSet(changes, currentSnapshotId);
boolean hasIntermediateSnapshots = !intermediateSnapshotIds.isEmpty();
boolean hasRemovedSnapshots =
- changes.stream()
- .anyMatch(
- change ->
- change instanceof MetadataUpdate.RemoveSnapshots
- || change instanceof MetadataUpdate.RemoveSnapshot);
Review Comment:
Sounds good. Let me revert this minimize the change.
--
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