Skip to content

Add SnapshotLifecycleService and related CRUD APIs#39795

Closed
dakrone wants to merge 6 commits intoelastic:masterfrom
dakrone:snapshot-lifecycle-management
Closed

Add SnapshotLifecycleService and related CRUD APIs#39795
dakrone wants to merge 6 commits intoelastic:masterfrom
dakrone:snapshot-lifecycle-management

Conversation

@dakrone
Copy link
Copy Markdown
Member

@dakrone dakrone commented Mar 7, 2019

This commit adds SnapshotLifecycleService as a new service under the ilm
plugin. This service handles snapshot lifecycle policies by scheduling based on
the policies defined schedule.

This also includes the get, put, and delete APIs for these policies

Relates to #38461

@dakrone dakrone added >non-issue :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. labels Mar 7, 2019
@dakrone dakrone requested a review from AthenaEryma March 7, 2019 15:30
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Mar 7, 2019

Pinging @elastic/es-core-features

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Mar 7, 2019 •
edited
Loading

I opened this as a >non-issue because once it is approved I intend to create a feature branch for this, rather than actually merging it to master. This is only a start of the framework for snapshot lifecycle management (and isn't feature-complete), and I'm opening it so we can start by having smaller reviewable chunks of the feature. That's also why there are scattered TODOs in these files for follow-up PRs.

AthenaEryma
AthenaEryma suggested changes Mar 11, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@AthenaEryma AthenaEryma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments - this looks like a good skeleton, though.

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Mar 12, 2019

@gwbrown thanks for taking a look. I pushed commits addressing your feedback, can you take another look please?

@AthenaEryma
Copy link
Copy Markdown
Contributor

AthenaEryma commented Mar 12, 2019

Thanks @dakrone, most of the changes look good but I left one more comment on the keySet thing.

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Mar 12, 2019

@gwbrown okay, I pushed a commit copying the set.

AthenaEryma
AthenaEryma approved these changes Mar 12, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@AthenaEryma AthenaEryma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (assuming CI is still happy). Thanks!

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Mar 12, 2019

@elasticmachine run elasticsearch-ci/bwc

dakrone added 6 commits March 12, 2019 17:08
@dakrone
Add SnapshotLifecycleService and related CRUD APIs
960041e 
This commit adds `SnapshotLifecycleService` as a new service under the ilm
plugin. This service handles snapshot lifecycle policies by scheduling based on
the policies defined schedule.

This also includes the get, put, and delete APIs for these policies

Relates to elastic#38461
@dakrone
Make scheduledJobIds return an immutable set
acb8135
@dakrone
Use Object.equals for SnapshotLifecyclePolicy
d7b41e0
@dakrone
Remove unneeded TODO
0043de4
@dakrone
Implement ToXContentFragment on SnapshotLifecyclePolicyItem
54ac17c
@dakrone
Copy contents of the scheduledJobIds
c5f7738
@dakrone dakrone force-pushed the snapshot-lifecycle-management branch from 16a76c5 to c5f7738 Compare March 12, 2019 23:08
@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Mar 12, 2019

Pushed branch "snapshot-lifecycle-management"

@dakrone dakrone closed this Mar 12, 2019
@dakrone dakrone mentioned this pull request Mar 15, 2019
Closed
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@AthenaEryma AthenaEryma AthenaEryma approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. >non-issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dakrone @elasticmachine @AthenaEryma