core: Send single snapshot to remove rather than in bulk#13100
Conversation
|
@amogh-jahagirdar and @ricardopereira33 Can you please take a look? Thanks. |
|
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) |
amogh-jahagirdar
left a comment
amogh-jahagirdar
left a comment
There was a problem hiding this comment.
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. |
aihuaxu
force-pushed
the
aixu-client-single-snapshot
branch
from
e6bb24c to
5971612
Compare
|
@nastra and @amogh-jahagirdar Can you please review again? Thanks. |
|
@RussellSpitzer I'm making the change to make |
| changes.stream() | ||
| .anyMatch( | ||
| change -> | ||
| change instanceof MetadataUpdate.RemoveSnapshots | ||
| || change instanceof MetadataUpdate.RemoveSnapshot); |
There was a problem hiding this 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
There was a problem hiding this comment.
Sounds good. Let me revert this minimize the change.
There was a problem hiding this comment.
Ok this looks good to me. I tested this with a local Iceberg rest image running an older Iceberg version and expiration succeeds. Thank you @aihuaxu . I'll give it a bit in case @RussellSpitzer wanted to review, but will merge shortly
|
Thank @aihuaxu, thanks to @amogh-jahagirdar for review |
…hot Changes Rather than in Bulk (apache#13100)
…t 12670 by sending single snapshot rather than in bulk (#13647)
4 participants
#12670 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 to the 1.8.x behavior in core that the client will send single snapshot to remove that the existing server can understand.
This PR is only fixing in 1.9.x branch.