From 701dc340a8eeabd059aec57140707c69e371142c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 3 Sep 2015 06:08:22 -0600 Subject: [PATCH] Allow deleting closed indices with shadow replicas Previously we skip deleting the index store for indices on a shared filesystem, because we don't want to delete the data when the shard is relocating around the cluster. This adds a flag to the `deleteIndexStore` method signifying that the index is closed and that we should allow deleting the contents even if it is on a shared filesystem. Includes a unit test for the IndicesService.canDeleteIndexContents and integration tests ensure a closed shadow replica index deletes files correctly. Resolves #13297 --- .../elasticsearch/indices/IndicesService.java | 22 +++++---- .../index/IndexWithShadowReplicasIT.java | 49 +++++++++++++++++++ .../indices/IndicesServiceTests.java | 21 ++++++-- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index 56cd225e996..3008d4fa549 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -437,7 +437,7 @@ public class IndicesService extends AbstractLifecycleComponent i final Settings indexSettings = indexService.getIndexSettings(); indicesLifecycle.afterIndexDeleted(indexService.index(), indexSettings); // now we are done - try to wipe data on disk if possible - deleteIndexStore(reason, indexService.index(), indexSettings); + deleteIndexStore(reason, indexService.index(), indexSettings, false); } } catch (IOException ex) { throw new ElasticsearchException("failed to remove index " + index, ex); @@ -490,7 +490,7 @@ public class IndicesService extends AbstractLifecycleComponent i final IndexMetaData index = clusterState.metaData().index(indexName); throw new IllegalStateException("Can't delete closed index store for [" + indexName + "] - it's still part of the cluster state [" + index.getIndexUUID() + "] [" + metaData.getIndexUUID() + "]"); } - deleteIndexStore(reason, metaData, clusterState); + deleteIndexStore(reason, metaData, clusterState, true); } catch (IOException e) { logger.warn("[{}] failed to delete closed index", e, metaData.index()); } @@ -501,7 +501,7 @@ public class IndicesService extends AbstractLifecycleComponent i * Deletes the index store trying to acquire all shards locks for this index. * This method will delete the metadata for the index even if the actual shards can't be locked. */ - public void deleteIndexStore(String reason, IndexMetaData metaData, ClusterState clusterState) throws IOException { + public void deleteIndexStore(String reason, IndexMetaData metaData, ClusterState clusterState, boolean closed) throws IOException { if (nodeEnv.hasNodeFile()) { synchronized (this) { String indexName = metaData.index(); @@ -518,18 +518,18 @@ public class IndicesService extends AbstractLifecycleComponent i } Index index = new Index(metaData.index()); final Settings indexSettings = buildIndexSettings(metaData); - deleteIndexStore(reason, index, indexSettings); + deleteIndexStore(reason, index, indexSettings, closed); } } - private void deleteIndexStore(String reason, Index index, Settings indexSettings) throws IOException { + private void deleteIndexStore(String reason, Index index, Settings indexSettings, boolean closed) throws IOException { boolean success = false; try { // we are trying to delete the index store here - not a big deal if the lock can't be obtained // the store metadata gets wiped anyway even without the lock this is just best effort since // every shards deletes its content under the shard lock it owns. logger.debug("{} deleting index store reason [{}]", index, reason); - if (canDeleteIndexContents(index, indexSettings)) { + if (canDeleteIndexContents(index, indexSettings, closed)) { nodeEnv.deleteIndexDirectorySafe(index, 0, indexSettings); } success = true; @@ -583,11 +583,11 @@ public class IndicesService extends AbstractLifecycleComponent i logger.debug("{} deleted shard reason [{}]", shardId, reason); if (clusterState.nodes().localNode().isMasterNode() == false && // master nodes keep the index meta data, even if having no shards.. - canDeleteIndexContents(shardId.index(), indexSettings)) { + canDeleteIndexContents(shardId.index(), indexSettings, false)) { if (nodeEnv.findAllShardIds(shardId.index()).isEmpty()) { try { // note that deleteIndexStore have more safety checks and may throw an exception if index was concurrently created. - deleteIndexStore("no longer used", metaData, clusterState); + deleteIndexStore("no longer used", metaData, clusterState, false); } catch (Exception e) { // wrap the exception to indicate we already deleted the shard throw new ElasticsearchException("failed to delete unused index after deleting its last shard (" + shardId + ")", e); @@ -606,9 +606,11 @@ public class IndicesService extends AbstractLifecycleComponent i * @param indexSettings {@code Settings} for the given index * @return true if the index can be deleted on this node */ - public boolean canDeleteIndexContents(Index index, Settings indexSettings) { + public boolean canDeleteIndexContents(Index index, Settings indexSettings, boolean closed) { final IndexServiceInjectorPair indexServiceInjectorPair = this.indices.get(index.name()); - if (IndexMetaData.isOnSharedFilesystem(indexSettings) == false) { + // Closed indices may be deleted, even if they are on a shared + // filesystem. Since it is closed we aren't deleting it for relocation + if (IndexMetaData.isOnSharedFilesystem(indexSettings) == false || closed) { if (indexServiceInjectorPair == null && nodeEnv.hasNodeFile()) { return true; } diff --git a/core/src/test/java/org/elasticsearch/index/IndexWithShadowReplicasIT.java b/core/src/test/java/org/elasticsearch/index/IndexWithShadowReplicasIT.java index 3b955b271af..b280389e173 100644 --- a/core/src/test/java/org/elasticsearch/index/IndexWithShadowReplicasIT.java +++ b/core/src/test/java/org/elasticsearch/index/IndexWithShadowReplicasIT.java @@ -44,6 +44,7 @@ import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalTestCluster; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportRequest; @@ -759,4 +760,52 @@ public class IndexWithShadowReplicasIT extends ESIntegTestCase { assertShardCountOn(newFooNode, 5); assertNoShardsOn(barNodes.get()); } + + public void testDeletingClosedIndexRemovesFiles() throws Exception { + Path dataPath = createTempDir(); + Path dataPath2 = createTempDir(); + Settings nodeSettings = nodeSettings(dataPath.getParent()); + + internalCluster().startNodesAsync(2, nodeSettings).get(); + String IDX = "test"; + String IDX2 = "test2"; + + Settings idxSettings = Settings.builder() + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 5) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1) + .put(IndexMetaData.SETTING_DATA_PATH, dataPath.toAbsolutePath().toString()) + .put(IndexMetaData.SETTING_SHADOW_REPLICAS, true) + .put(IndexMetaData.SETTING_SHARED_FILESYSTEM, true) + .build(); + Settings idx2Settings = Settings.builder() + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 5) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1) + .put(IndexMetaData.SETTING_DATA_PATH, dataPath2.toAbsolutePath().toString()) + .put(IndexMetaData.SETTING_SHADOW_REPLICAS, true) + .put(IndexMetaData.SETTING_SHARED_FILESYSTEM, true) + .build(); + + prepareCreate(IDX).setSettings(idxSettings).addMapping("doc", "foo", "type=string").get(); + prepareCreate(IDX2).setSettings(idx2Settings).addMapping("doc", "foo", "type=string").get(); + ensureGreen(IDX, IDX2); + + int docCount = randomIntBetween(10, 100); + List builders = new ArrayList<>(); + for (int i = 0; i < docCount; i++) { + builders.add(client().prepareIndex(IDX, "doc", i + "").setSource("foo", "bar")); + builders.add(client().prepareIndex(IDX2, "doc", i + "").setSource("foo", "bar")); + } + indexRandom(true, true, true, builders); + flushAndRefresh(IDX, IDX2); + + logger.info("--> closing index {}", IDX); + client().admin().indices().prepareClose(IDX).get(); + + logger.info("--> deleting non-closed index"); + client().admin().indices().prepareDelete(IDX2).get(); + assertPathHasBeenCleared(dataPath2); + logger.info("--> deleting closed index"); + client().admin().indices().prepareDelete(IDX).get(); + assertPathHasBeenCleared(dataPath); + } } diff --git a/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 08c33380a2b..b1ed006d695 100644 --- a/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -23,9 +23,11 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.gateway.GatewayMetaState; +import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.shard.ShardPath; @@ -51,6 +53,19 @@ public class IndicesServiceTests extends ESSingleNodeTestCase { return true; } + public void testCanDeleteIndexContent() { + IndicesService indicesService = getIndicesService(); + + Settings idxSettings = settings(Version.CURRENT) + .put(IndexMetaData.SETTING_SHADOW_REPLICAS, true) + .put(IndexMetaData.SETTING_DATA_PATH, "/foo/bar") + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 4)) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, randomIntBetween(0, 3)) + .build(); + assertFalse("shard on shared filesystem", indicesService.canDeleteIndexContents(new Index("test"), idxSettings, false)); + assertTrue("shard on shared filesystem and closed", indicesService.canDeleteIndexContents(new Index("test"), idxSettings, true)); + } + public void testCanDeleteShardContent() { IndicesService indicesService = getIndicesService(); IndexMetaData meta = IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas( @@ -71,7 +86,7 @@ public class IndicesServiceTests extends ESSingleNodeTestCase { assertTrue(test.hasShard(0)); try { - indicesService.deleteIndexStore("boom", firstMetaData, clusterService.state()); + indicesService.deleteIndexStore("boom", firstMetaData, clusterService.state(), false); fail(); } catch (IllegalStateException ex) { // all good @@ -98,7 +113,7 @@ public class IndicesServiceTests extends ESSingleNodeTestCase { assertTrue(path.exists()); try { - indicesService.deleteIndexStore("boom", secondMetaData, clusterService.state()); + indicesService.deleteIndexStore("boom", secondMetaData, clusterService.state(), false); fail(); } catch (IllegalStateException ex) { // all good @@ -108,7 +123,7 @@ public class IndicesServiceTests extends ESSingleNodeTestCase { // now delete the old one and make sure we resolve against the name try { - indicesService.deleteIndexStore("boom", firstMetaData, clusterService.state()); + indicesService.deleteIndexStore("boom", firstMetaData, clusterService.state(), false); fail(); } catch (IllegalStateException ex) { // all good