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.

Sign up for GitHub

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

Jump to bottom

SourceOnlySnapshotRepository is Leaking Files #50231

Open
original-brownbear opened this issue Dec 16, 2019 · 2 comments
Open

SourceOnlySnapshotRepository is Leaking Files #50231

original-brownbear opened this issue Dec 16, 2019 · 2 comments
Labels
>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Meta label for distributed team

Comments

@original-brownbear
Copy link
Member

original-brownbear commented Dec 16, 2019 •
edited
Loading

When using SourceOnlySnapshotRepository a _snapshot directory is created in each snapshotted shard's data path. The contents of this path directory are never cleaned up it seems.

I tried fixing this via a trivial fix:

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/SourceOnlySnapshotRepository.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/SourceOnlySnapshotRepository.java
index af9d95a519a..5b46d786f7f 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/SourceOnlySnapshotRepository.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/SourceOnlySnapshotRepository.java
@@ -134,10 +134,11 @@ public final class SourceOnlySnapshotRepository extends FilterRepository {
         Path dataPath = ((FSDirectory) unwrap).getDirectory().getParent();
         // TODO should we have a snapshot tmp directory per shard that is maintained by the system?
         Path snapPath = dataPath.resolve(SNAPSHOT_DIR_NAME);
-        final List<Closeable> toClose = new ArrayList<>(3);
+        final List<Closeable> toClose = new ArrayList<>(4);
         try {
             FSDirectory directory = new SimpleFSDirectory(snapPath);
             toClose.add(directory);
+            toClose.add(() -> IOUtils.rm(snapPath));
             Store tempStore = new Store(store.shardId(), store.indexSettings(), directory, new ShardLock(store.shardId()) {
                 @Override
                 protected void closeInternal() {

but this seems to severely alter the incrementality properties of the source only snapshotting (tests fails because the expected file counts change).
This is kind of obvious from the way it works, but shouldn't we at least delete these _snapshot folders when removing a source only repository? Also, shouldn't we document the fact that using a source only repository may require significant disk space to track the source_only on disk index used to make things incremental?

Originally reported in https://discuss.elastic.co/t/large-snapshot-directories-on-disk/211988
@original-brownbear original-brownbear added >bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Dec 16, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@original-brownbear original-brownbear self-assigned this Jan 29, 2020
@original-brownbear original-brownbear mentioned this issue Jan 31, 2020
Merged
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Feb 2, 2020
@original-brownbear
Fix Source Only Snapshot Uploading Files for Unchanged Shards
be27429 
We shouldn't be creating a new commit (by bootstrapping an new history)
if nothing has changed about the shard.

Relates elastic#50231 in that it prevents a bunch of redundant `segments_N` from being uploaded
and makes a fix shorter/clearer
@original-brownbear original-brownbear mentioned this issue Feb 2, 2020
Closed
@ywelsch ywelsch mentioned this issue Mar 12, 2020
Merged
ywelsch added a commit that referenced this issue Mar 23, 2020
@ywelsch
Only link fd* files during source-only snapshot (#53463)
c829079 
Source-only snapshots currently create a second full source-only copy of the shard on disk to
support incrementality during upload. Given that stored fields are occupying a substantial part
of a shard's storage, this means that clusters with source-only snapshots can require up to
50% more local storage. Ideally we would only generate source-only parts of the shard for the
things that need to be uploaded (i.e. do incrementality checks on original file instead of
trimmed-down source-only versions), but that requires much bigger changes to the snapshot
infrastructure. This here is an attempt to dramatically cut down on the storage used by the
source-only copy of the shard by soft-linking the stored-fields files (fd*) instead of copying
them.

Relates #50231
ywelsch added a commit that referenced this issue Mar 23, 2020
@ywelsch
Only link fd* files during source-only snapshot (#53463)
060c72c 
Source-only snapshots currently create a second full source-only copy of the shard on disk to
support incrementality during upload. Given that stored fields are occupying a substantial part
of a shard's storage, this means that clusters with source-only snapshots can require up to
50% more local storage. Ideally we would only generate source-only parts of the shard for the
things that need to be uploaded (i.e. do incrementality checks on original file instead of
trimmed-down source-only versions), but that requires much bigger changes to the snapshot
infrastructure. This here is an attempt to dramatically cut down on the storage used by the
source-only copy of the shard by soft-linking the stored-fields files (fd*) instead of copying
them.

Relates #50231
@codebrain codebrain mentioned this issue Apr 1, 2020
Closed
@rjernst rjernst added the Team:Distributed Meta label for distributed team label May 4, 2020
@original-brownbear original-brownbear removed their assignment Jul 28, 2022
@elasticsearchmachine
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Meta label for distributed team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rjernst @original-brownbear @elasticmachine @elasticsearchmachine