-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[SR] add ability to execute snapshot retention manually #47150
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
[SR] add ability to execute snapshot retention manually #47150
Conversation
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
💚 Build Succeeded |
|
@elasticmachine merge upstream |
💔 Build Failed |
|
@elasticmachine merge upstream |
💚 Build Succeeded |
jloleysens
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.
This looks great @alisonelizabeth
Thanks for putting this together. The code looks really clean and is easy to follow.
I did run into the following issue when testing per the instruction in the PR description:
One other thing; do we want to take this opportunity to add Jest tests for the new functionality, WDYT?
| <EuiSpacer /> | ||
| </Fragment> | ||
| ); | ||
| return renderRetentionPanel(retentionSettings.retentionSchedule); |
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.
👍
| }); | ||
| }); | ||
|
|
||
| describe('executeRetentionHandler()', () => { |
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.
Not entirely sure I understand what coverage this test provides?
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.
good point, i removed this test for now.
|
@jloleysens thanks for the review! It seems the API change is not in the latest snapshot for some reason. Do you mind building ES from source and trying again? Also, I agree test coverage is important here. SR as a whole is lacking coverage. I added/updated some of the tests via #45193, but there is still more work to be done here. I opened a separate issue to track this: #47595 |
jloleysens
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.
Re-tested with ES built from source and it worked 😄
💔 Build Failed |
💚 Build Succeeded |

This PR is continuation of #45193, and adds the ability to run snapshot retention without scheduling it.
Related ES API change: elastic/elasticsearch#47405
Release notes
This feature adds the ability to run snapshot retention without scheduling. This is useful to perform a manual invocation or to perform a one-off cleanup.
Note: This work is continuation of #45193, which added the Snapshot Retention UI to the Snapshot and Restore app.
Testing locally
Create a repository within SR. For a shared filesystem repo, you need to specify a
path.repowhen starting ES. For example:yarn es snapshot -E path.repo=/tmp/es-backupsFor more information, see the SR docs.
Create a policy with retention defined. Suggestion: Set up your policy to create a snapshot every minute, and configure retention to only retain a max of 2 snapshots.
Wait a few minutes for snapshots to be created. Once you have >2, run retention.
You should see that the snapshots were deleted, either by visiting the
Snapshotstab or viewing the statistics on the policy details panel.Screenshots
If a retention schedule is not configured:

If retention schedule is configured:

Run retention modal:
