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

Crashing during snapshot deletion might result in unreferenced data left in repository #13159

Closed
nkvoll opened this issue Aug 27, 2015 · 5 comments · Fixed by #42189
Closed
Assignees
Labels
>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs help wanted adoptme

Comments

@nkvoll
Copy link
Member

nkvoll commented Aug 27, 2015

It's currently possible to end up with unreferenced data in a snapshot repository, given the following steps:

  1. Create a index, foobar, with size X bytes
  2. Snapshot cluster
  3. Start deleting the snapshot, crash after deleting snapshot-{} and metadata-{} files
  4. Delete index foobar
  5. Snapshot cluster again

Normally, step 5 would cause the files no longer referenced by any snapshots do be deleted, but if the underlying index is deleted as well, they won't get cleaned up. In the example above, there would be X bytes of disk space used without any snapshot referencing them. Given sufficiently large values of X, this could be a significant amount of storage wasted. Even with small amounts of data, this might accrue over time to become significant.

Suggestion: create a deleting-{} file as a sibling of the snapshot-{} file that gets written before the files referenced by the snapshot gets deleted. When the deletion has been completed, this file should be the last one deleted. These files indicates that a deletion is in progress or have been attempted so it's possible to tell that the snapshot might be in a half-deleted state (so we can avoid using it). It should also enable later snapshot processes to continue the deletion process where the previous left off.

@nkvoll nkvoll added the :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs label Aug 27, 2015
@tlrx
Copy link
Member

tlrx commented Mar 21, 2018

We talked about it today with @ywelsch and we still think that this is a real issue. We don't have any plan about it yet but this is something we need to think about for the future of the snapshot/restore functionality.

@original-brownbear original-brownbear self-assigned this Jan 28, 2019
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Feb 4, 2019
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Feb 12, 2019
@original-brownbear
Copy link
Member

There is another scenario here that can lead to the same stale data that I found:

  1. Create a index, foobar, with size X bytes
  2. Start snapshotting cluster
  3. Abort snapshot
    4 .Temporarily disconnect datanode from master (via network partition or so) while it is writing shard data to repository
  4. Master node finishes deleting snapshot

If in this scenario the data node continues to write segment files (as seen in this test failure #39852) we get unreferenced data since step 5 has removed any metadata reference to the index and snapshotting it again will lead to a new index uuid.

@original-brownbear
Copy link
Member

I opened #40228 with a suggestion on how to write the tombstones to get a better handle on this situation.

original-brownbear added a commit that referenced this issue Jun 21, 2019
* Use ability to list child "folders" in the blob store to implement recursive delete on all stale index folders when cleaning up instead of using the diff between two `RepositoryData` instances to cover aborted deletes
* Runs after ever delete operation
* Relates  #13159 (fixing most of this issues caused by unreferenced indices, leaving some meta files to be cleaned up only)
@original-brownbear
Copy link
Member

original-brownbear commented Jun 21, 2019

Leaving this open, since there's still a little work left here in cleaning up unreferenced top level blobs. I'll raise a PR for that shortly.

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Jul 8, 2019
* Use ability to list child "folders" in the blob store to implement recursive delete on all stale index folders when cleaning up instead of using the diff between two `RepositoryData` instances to cover aborted deletes
* Runs after ever delete operation
* Relates  elastic#13159 (fixing most of this issues caused by unreferenced indices, leaving some meta files to be cleaned up only)
original-brownbear added a commit that referenced this issue Jul 8, 2019
* Use ability to list child "folders" in the blob store to implement recursive delete on all stale index folders when cleaning up instead of using the diff between two `RepositoryData` instances to cover aborted deletes
* Runs after ever delete operation
* Relates  #13159 (fixing most of this issues caused by unreferenced indices, leaving some meta files to be cleaned up only)
@original-brownbear
Copy link
Member

Closing this as #42189 (and numerous follow-ups) resolved the bulk of the un-referenced file leaks on every delete operation and #43900 will bring a solution to cleanup the remainder (while acknowledging that some leaking can occur on errors that must be cleaned up via the cleanup endpoint that #43900 introdcues)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs help wanted adoptme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants