dl/coordinator: add tag to redpanda commits #25038
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Retry command for Build#61608please wait until all jobs are finished before running the slash command |
CI test resultstest results on build#61608
test results on build#61628
test results on build#61697
|
andrwng
force-pushed
the
dl-coordinator-tag-appends
branch
from
1bde4d8 to
1958111
Compare
Is this part of the Iceberg spec? Cursory check didn't turn anything up, but I may not be searching for the right thing. EDIT: ah, i see, tags & branches have separate expiration policies. so in principle we could still get in a funky situation w/ customized table maintenance, but the default behavior is what we want? |
oleiman
reviewed
Feb 6, 2025
bharathv
reviewed
Feb 6, 2025
Generally it's not part of the spec, but it is in the Java implementation of snapshot expiry -- the default is set per table. Though you raise a good point that there is a mechanism per snapshot that we can set to not expire. Will update. |
andrwng
force-pushed
the
dl-coordinator-tag-appends
branch
from
1958111 to
491e6d1
Compare
Adds the capability to tag a snapshot when appended. This will be useful for pinning a snapshot to the table, since tagged snapshots don't expire by default. We'd like to keep some metadata around in Redpanda-generated snapshots for exactly once delivery, and this helps ensure that metadata don't get wiped out.
andrwng
force-pushed
the
dl-coordinator-tag-appends
branch
from
491e6d1 to
d3f1710
Compare
oleiman
previously approved these changes
Feb 6, 2025
Adds a tag to snapshots created by Redpanda. This ensures that the latest Redpanda snapshot won't be removed by snapshot expiry, since tags don't expire by default. This assurance helps us guarantee exactly once delivery even in the face of external table updates.
andrwng
force-pushed
the
dl-coordinator-tag-appends
branch
from
d3f1710 to
e2e7ff2
Compare
oleiman
approved these changes
Feb 6, 2025
bharathv
approved these changes
Feb 6, 2025
nvartolomei
reviewed
Feb 17, 2025
| {{commit_meta_prop, to_json_str(commit_meta)}}); | ||
| {{commit_meta_prop, to_json_str(commit_meta)}}, | ||
| commit_tag_name, | ||
| /*tag_expiration_ms=*/std::numeric_limits<long>::max()); |
There was a problem hiding this comment.
nit: this should be std::numeric_limits<long long>::max()std::numeric_limits<int64_t>::max() (together with the underlying type def). We never compile for a platform where this would result in 32 bit integer afaik but being explicit doesn't hurt.
long is defined as "at least" 32 bit by the C++ standard, on unix x86/arm 64bit it is actually a 64 bit number so all good, windows is 32 bit (who cares)
32 bit max as ms is ~24 days
There was a problem hiding this comment.
Actually, this should be int64_t in case the definition of long long changes... (i.e. it becomes 128 bit) long in iceberg spec is defined as a 64 bit wide integer.
andrwng
added a commit
to andrwng/redpanda
that referenced
this pull request
Feb 19, 2025
Longs in Iceberg are 64-bit, so we should be explicit about that. Review follow-up from redpanda-data#25038
7 tasks
|
Nessie dislikes this: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports Required
Release Notes