Skip to content
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

Prevent deletion of snapshots that are mounted and ILM delete action #73947

Open
tlrx opened this issue Jun 9, 2021 · 9 comments
Open

Prevent deletion of snapshots that are mounted and ILM delete action #73947

tlrx opened this issue Jun 9, 2021 · 9 comments
Assignees
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Data Management Meta label for data/management team Team:Distributed Meta label for distributed team

Comments

@tlrx
Copy link
Member

tlrx commented Jun 9, 2021

Today nothing prevents the deletion of a snapshot that is used by a searchable snapshot index. If such a snapshot is deleted the mounted index will go RED in the future. We started to implement a check in #73821 to prevent the deletion of snapshots that are used by searchable snapshots indices within the same cluster and it made some ILM tests to fail.

ILM has a delete index action DeleteAction with a delete_searchable_snapshot flag that, when set to true (the default value), deletes the snapshot that is mounted before deleting the index. ILM deletes the snapshot before the index because the information about the snapshot are stored in the index's metadata (under the ILM custom metedata). If ILM was deleting the searchable snapshot index before the snapshot it would not know which snapshot to delete.

I still think it is important to prevent the deletion of mounted snapshots so we should investigate how to keep the ILM behavior while implementing a safety check in #73821.

I can see different solution and I'm opening this issue to discuss them:

  • Adds a specific flag to the delete snapshot request that allows to bypass the safety check. This flag should not be exposed at the REST API level and can be kept as internal, only set by ILM delete action.
  • Allow to delete a searchable snapshot index and its snapshot together. The index metadata should be removed from the cluster state at the same time the snapshot is added as a snapshot deletion in progress. It implies multiple verifications (if the snapshot is used by another index, if the snapshot is used by a restore etc) but it should be doable. Wether this should be implemented in the existing delete index action, the existing snapshot delete action or a new action (like Unmount API) is left to decide.
  • ILM keeps the information of snapshots to delete somewhere else
@tlrx tlrx added >enhancement discuss :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Jun 9, 2021
@elasticmachine elasticmachine added Team:Distributed Meta label for distributed team Team:Data Management Meta label for data/management team labels Jun 9, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

@tlrx tlrx removed the discuss label Jun 14, 2021
@tlrx tlrx self-assigned this Jun 14, 2021
@tlrx
Copy link
Member Author

tlrx commented Jun 14, 2021

@henningandersen suggested the idea to add a flag to the existing Mount API to indicate that the index "owns" the snapshot and that it should be deleted when the searchable snapshot index is deleted. I'm going to give it try.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see elastic#73947). This
commit clarifies this by saying `must not` instead.

Closes elastic#108450
DaveCTurner added a commit that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see #73947). This
commit clarifies this by saying `must not` instead.

Closes #108450
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see elastic#73947). This
commit clarifies this by saying `must not` instead.

Closes elastic#108450
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see elastic#73947). This
commit clarifies this by saying `must not` instead.

Closes elastic#108450
elasticsearchmachine pushed a commit that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see #73947). This
commit clarifies this by saying `must not` instead.

Closes #108450
elasticsearchmachine pushed a commit that referenced this issue May 9, 2024
The word `cannot` implies Elasticsearch prevents you from doing these
things, but it doesn't have this protection today (see #73947). This
commit clarifies this by saying `must not` instead.

Closes #108450
@wortmanb

This comment was marked as off-topic.

@KyleOnK8s

This comment was marked as off-topic.

@adavidsmeier

This comment was marked as off-topic.

@kunisen

This comment was marked as off-topic.

@kunisen

This comment was marked as off-topic.

@DaveCTurner

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Data Management Meta label for data/management team Team:Distributed Meta label for distributed team
Projects
None yet
Development

No branches or pull requests

7 participants