-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[core] Automatically detect changes to the expiration policy. #5944
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
base: master
Are you sure you want to change the base?
Conversation
ccde03f
to
232d135
Compare
@@ -79,6 +87,12 @@ public ExpireSnapshots config(ExpireConfig expireConfig) { | |||
|
|||
@Override | |||
public int expire() { | |||
TableSchema latestTableSchema = this.schemaManager.latest().get(); |
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.
This will bring additional IO access. Can you introduce a new option to describe whether a refresh is needed?
232d135
to
a431454
Compare
a431454
to
9fc3b16
Compare
@@ -443,6 +443,12 @@ public InlineElement getDescription() { | |||
.withDescription( | |||
"The maximum number of snapshots allowed to expire at a time."); | |||
|
|||
public static final ConfigOption<Boolean> DETECT_EXPIRATION_SETTING_ENABLED = | |||
key("detect-expiration-setting.enabled") |
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.
snapshot.expire.detect-config.enabled
?
Thanks for the review @JingsongLi . All comments have been addressed, Please take another look when you have a moment. |
Purpose
Linked issue: close #5943
Tests
API and Format
Documentation