mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-02-26 23:07:45 +00:00
Snapshot repository cleans up empty index folders (#19751)
This commit cleans up indices in a snapshot repository when all snapshots containing the index are all deleted. Previously, empty indices folders would lay around after all snapshots containing them were deleted.
This commit is contained in:
parent
284b9794c0
commit
f59ca9083b
@ -33,9 +33,11 @@ import java.io.InputStream;
|
||||
import java.io.OutputStream;
|
||||
import java.nio.file.DirectoryStream;
|
||||
import java.nio.file.FileAlreadyExistsException;
|
||||
import java.nio.file.FileVisitResult;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.NoSuchFileException;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.SimpleFileVisitor;
|
||||
import java.nio.file.StandardCopyOption;
|
||||
import java.nio.file.StandardOpenOption;
|
||||
import java.nio.file.attribute.BasicFileAttributes;
|
||||
@ -89,7 +91,19 @@ public class FsBlobContainer extends AbstractBlobContainer {
|
||||
@Override
|
||||
public void deleteBlob(String blobName) throws IOException {
|
||||
Path blobPath = path.resolve(blobName);
|
||||
Files.delete(blobPath);
|
||||
if (Files.isDirectory(blobPath)) {
|
||||
// delete directory recursively as long as it is empty (only contains empty directories),
|
||||
// which is the reason we aren't deleting any files, only the directories on the post-visit
|
||||
Files.walkFileTree(blobPath, new SimpleFileVisitor<Path>() {
|
||||
@Override
|
||||
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
|
||||
Files.delete(dir);
|
||||
return FileVisitResult.CONTINUE;
|
||||
}
|
||||
});
|
||||
} else {
|
||||
Files.delete(blobPath);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -38,15 +38,19 @@ public final class IndexId implements Writeable, ToXContent {
|
||||
|
||||
private final String name;
|
||||
private final String id;
|
||||
private final int hashCode;
|
||||
|
||||
public IndexId(final String name, final String id) {
|
||||
this.name = name;
|
||||
this.id = id;
|
||||
this.hashCode = computeHashCode();
|
||||
|
||||
}
|
||||
|
||||
public IndexId(final StreamInput in) throws IOException {
|
||||
this.name = in.readString();
|
||||
this.id = in.readString();
|
||||
this.hashCode = computeHashCode();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -90,6 +94,10 @@ public final class IndexId implements Writeable, ToXContent {
|
||||
|
||||
@Override
|
||||
public int hashCode() {
|
||||
return hashCode;
|
||||
}
|
||||
|
||||
private int computeHashCode() {
|
||||
return Objects.hash(name, id);
|
||||
}
|
||||
|
||||
|
@ -101,6 +101,7 @@ import java.io.FileNotFoundException;
|
||||
import java.io.FilterInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.nio.file.DirectoryNotEmptyException;
|
||||
import java.nio.file.NoSuchFileException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
@ -406,7 +407,8 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
|
||||
}
|
||||
try {
|
||||
// Delete snapshot from the index file, since it is the maintainer of truth of active snapshots
|
||||
writeIndexGen(repositoryData.removeSnapshot(snapshotId));
|
||||
final RepositoryData updatedRepositoryData = repositoryData.removeSnapshot(snapshotId);
|
||||
writeIndexGen(updatedRepositoryData);
|
||||
|
||||
// delete the snapshot file
|
||||
safeSnapshotBlobDelete(snapshot, snapshotId.getUUID());
|
||||
@ -436,6 +438,27 @@ public abstract class BlobStoreRepository extends AbstractLifecycleComponent imp
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// cleanup indices that are no longer part of the repository
|
||||
final Collection<IndexId> indicesToCleanUp = Sets.newHashSet(repositoryData.getIndices().values());
|
||||
indicesToCleanUp.removeAll(updatedRepositoryData.getIndices().values());
|
||||
final BlobContainer indicesBlobContainer = blobStore().blobContainer(basePath().add("indices"));
|
||||
for (final IndexId indexId : indicesToCleanUp) {
|
||||
try {
|
||||
indicesBlobContainer.deleteBlob(indexId.getId());
|
||||
} catch (DirectoryNotEmptyException dnee) {
|
||||
// if the directory isn't empty for some reason, it will fail to clean up;
|
||||
// we'll ignore that and accept that cleanup didn't fully succeed.
|
||||
// since we are using UUIDs for path names, this won't be an issue for
|
||||
// snapshotting indices of the same name
|
||||
logger.debug("[{}] index [{}] no longer part of any snapshots in the repository, but failed to clean up " +
|
||||
"its index folder due to the directory not being empty.", dnee, metadata.name(), indexId);
|
||||
} catch (IOException ioe) {
|
||||
// a different IOException occurred while trying to delete - will just log the issue for now
|
||||
logger.debug("[{}] index [{}] no longer part of any snapshots in the repository, but failed to clean up " +
|
||||
"its index folder.", ioe, metadata.name(), indexId);
|
||||
}
|
||||
}
|
||||
} catch (IOException ex) {
|
||||
throw new RepositoryException(metadata.name(), "failed to update snapshot in repository", ex);
|
||||
}
|
||||
|
@ -20,7 +20,7 @@ package org.elasticsearch.snapshots;
|
||||
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.unit.ByteSizeUnit;
|
||||
import org.elasticsearch.repositories.ESBlobStoreRepositoryIntegTestCase;
|
||||
import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
|
||||
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
|
||||
|
@ -27,7 +27,7 @@ import org.elasticsearch.common.unit.TimeValue;
|
||||
import org.elasticsearch.env.Environment;
|
||||
import org.elasticsearch.plugin.repository.gcs.GoogleCloudStoragePlugin;
|
||||
import org.elasticsearch.plugins.Plugin;
|
||||
import org.elasticsearch.repositories.ESBlobStoreRepositoryIntegTestCase;
|
||||
import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
|
||||
import org.junit.BeforeClass;
|
||||
|
||||
import java.util.Collection;
|
||||
|
@ -16,13 +16,18 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
package org.elasticsearch.repositories;
|
||||
package org.elasticsearch.repositories.blobstore;
|
||||
|
||||
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequestBuilder;
|
||||
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
|
||||
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequestBuilder;
|
||||
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
|
||||
import org.elasticsearch.action.index.IndexRequestBuilder;
|
||||
import org.elasticsearch.client.Client;
|
||||
import org.elasticsearch.common.blobstore.BlobContainer;
|
||||
import org.elasticsearch.repositories.IndexId;
|
||||
import org.elasticsearch.repositories.RepositoriesService;
|
||||
import org.elasticsearch.repositories.RepositoryData;
|
||||
import org.elasticsearch.test.ESIntegTestCase;
|
||||
|
||||
import java.util.Arrays;
|
||||
@ -161,6 +166,58 @@ public abstract class ESBlobStoreRepositoryIntegTestCase extends ESIntegTestCase
|
||||
}
|
||||
}
|
||||
|
||||
public void testIndicesDeletedFromRepository() throws Exception {
|
||||
Client client = client();
|
||||
|
||||
logger.info("--> creating repository");
|
||||
final String repoName = "test-repo";
|
||||
createTestRepository(repoName);
|
||||
|
||||
createIndex("test-idx-1", "test-idx-2", "test-idx-3");
|
||||
ensureGreen();
|
||||
|
||||
logger.info("--> indexing some data");
|
||||
for (int i = 0; i < 20; i++) {
|
||||
index("test-idx-1", "doc", Integer.toString(i), "foo", "bar" + i);
|
||||
index("test-idx-2", "doc", Integer.toString(i), "foo", "baz" + i);
|
||||
index("test-idx-3", "doc", Integer.toString(i), "foo", "baz" + i);
|
||||
}
|
||||
refresh();
|
||||
|
||||
logger.info("--> take a snapshot");
|
||||
CreateSnapshotResponse createSnapshotResponse =
|
||||
client.admin().cluster().prepareCreateSnapshot(repoName, "test-snap").setWaitForCompletion(true).get();
|
||||
assertEquals(createSnapshotResponse.getSnapshotInfo().successfulShards(), createSnapshotResponse.getSnapshotInfo().totalShards());
|
||||
|
||||
logger.info("--> indexing more data");
|
||||
for (int i = 20; i < 40; i++) {
|
||||
index("test-idx-1", "doc", Integer.toString(i), "foo", "bar" + i);
|
||||
index("test-idx-2", "doc", Integer.toString(i), "foo", "baz" + i);
|
||||
index("test-idx-3", "doc", Integer.toString(i), "foo", "baz" + i);
|
||||
}
|
||||
|
||||
logger.info("--> take another snapshot with only 2 of the 3 indices");
|
||||
createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot(repoName, "test-snap2")
|
||||
.setWaitForCompletion(true)
|
||||
.setIndices("test-idx-1", "test-idx-2")
|
||||
.get();
|
||||
assertEquals(createSnapshotResponse.getSnapshotInfo().successfulShards(), createSnapshotResponse.getSnapshotInfo().totalShards());
|
||||
|
||||
logger.info("--> delete a snapshot");
|
||||
assertAcked(client().admin().cluster().prepareDeleteSnapshot(repoName, "test-snap").get());
|
||||
|
||||
logger.info("--> verify index folder deleted from blob container");
|
||||
RepositoriesService repositoriesSvc = internalCluster().getInstance(RepositoriesService.class, internalCluster().getMasterName());
|
||||
@SuppressWarnings("unchecked") BlobStoreRepository repository = (BlobStoreRepository) repositoriesSvc.repository(repoName);
|
||||
BlobContainer indicesBlobContainer = repository.blobStore().blobContainer(repository.basePath().add("indices"));
|
||||
RepositoryData repositoryData = repository.getRepositoryData();
|
||||
for (IndexId indexId : repositoryData.getIndices().values()) {
|
||||
if (indexId.getName().equals("test-idx-3")) {
|
||||
assertFalse(indicesBlobContainer.blobExists(indexId.getId())); // deleted index
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
protected void addRandomDocuments(String name, int numDocs) throws ExecutionException, InterruptedException {
|
||||
IndexRequestBuilder[] indexRequestBuilders = new IndexRequestBuilder[numDocs];
|
||||
for (int i = 0; i < numDocs; i++) {
|
Loading…
x
Reference in New Issue
Block a user