-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Introduce retention lease persistence #37375
Introduce retention lease persistence #37375
Conversation
This commit introduces the persistence of retention leases by persisting them in index commits and recovering them when recovering a shard from store.
Pinging @elastic/es-distributed |
@elasticmachine run gradle build tests 2 |
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.
I left two minor comments.
Thanks @dnhatn, I have responded to your feedback. |
…rsistence * elastic/master: Fix PrimaryAllocationIT Race Condition (elastic#37355) SQL: Make `FULL` non-reserved keyword in the grammar (elastic#37377) SQL: [Tests] Fix and enable internalClusterTests (elastic#37300) ML: Fix testMigrateConfigs (elastic#37373) Fix RollupDocumentation test to wait for job to stop
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.
LGTM. Thanks @jasontedor.
@elasticmachine run gradle build tests 1 |
This commit introduces the persistence of retention leases by persisting them in index commits and recovering them when recovering a shard from store.
Objects.requireNonNull(retentionLease); | ||
return String.format( | ||
Locale.ROOT, | ||
"id:%s;retaining_seq_no:%d;timestamp:%d;source:%s", |
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.
I wonder if we should use JSON, with the upside of being more extensible and will allow us to use our parsing infra (and the validation features it has). WDYT?
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.
I am not particularly concerned about this needing to be extensible. I can be convinced that JSON is worth it but I am doubting.
This commit introduces the persistence of retention leases by persisting them in index commits and recovering them when recovering a shard from store.
Relates #37165