-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[core] Introduce ChangelogDeletion to Expiration #3383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| .maxDeletes(options.snapshotExpireLimit()) | ||
| .retainMin(options.changelogNumRetainMin()) | ||
| .retainMax(options.changelogNumRetainMax()); | ||
| newExpireChangelog().config(options.expireConfig().build()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reuse options.expireConfig().build().
|
|
||
| protected final Map<BinaryRow, Set<Integer>> deletionBuckets; | ||
| protected final Executor ioExecutor; | ||
| protected boolean changelogDecoupled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changelogDecoupled who use this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, SnapshotDeletion will use this to determine whether to clean changelog manifests. It will also be used in ChangelogDeletion in following PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but there is no assignment for it.
maybe you need to add it in next pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is setChangelogDecoupled . I feel difficult to make this field final, so I add a set method for it.
paimon-core/src/main/java/org/apache/paimon/operation/FileDeletionBase.java
Show resolved
Hide resolved
03e0fb1 to
621a34e
Compare
| public Predicate<ManifestEntry> dataFileSkipper(Snapshot fromSnapshot) throws Exception { | ||
| return dataFileSkipper(Collections.singletonList(fromSnapshot)); | ||
| } | ||
| protected Collection<ManifestEntry> readMergedDataFiles(Snapshot snapshot) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate codes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest commit is reverted, please take a look again when you are free.
JingsongLi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Purpose
This PR is the second step of #3178 to simply the clean up logic of snapshot and changelog
Tests
API and Format
Documentation