Merge remote-tracking branch 'dakrone/delete-closed-shadow-indices'

core/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -437,7 +437,7 @@ public class IndicesService extends AbstractLifecycleComponent<IndicesService> 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<IndicesService> 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<IndicesService> 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<IndicesService> 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<IndicesService> 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<IndicesService> 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;
             }

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<IndexRequestBuilder> 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);
+    }
 }

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