better lifecycle handling when deleting an index

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/IndexGateway.java

@@ -20,17 +20,13 @@
 package org.elasticsearch.index.gateway;
 
 import org.elasticsearch.index.IndexComponent;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 
 /**
- * @author kimchy (Shay Banon)
+ * @author kimchy (shay.banon)
  */
-public interface IndexGateway extends IndexComponent, CloseableComponent {
+public interface IndexGateway extends IndexComponent, CloseableIndexComponent {
 
     Class<? extends IndexShardGateway> shardGatewayClass();
 
-    /**
-     * Deletes the content of the index gateway.
-     */
-    void delete();
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/IndexShardGateway.java

@@ -23,11 +23,12 @@ import org.elasticsearch.ElasticSearchIllegalStateException;
 import org.elasticsearch.index.deletionpolicy.SnapshotIndexCommit;
 import org.elasticsearch.index.shard.IndexShardComponent;
 import org.elasticsearch.index.translog.Translog;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 
 /**
  * @author kimchy (Shay Banon)
  */
-public interface IndexShardGateway extends IndexShardComponent {
+public interface IndexShardGateway extends IndexShardComponent, CloseableIndexComponent {
 
     /**
      * Recovers the state of the shard from the gateway.
@@ -45,8 +46,6 @@ public interface IndexShardGateway extends IndexShardComponent {
      */
     boolean requiresSnapshotScheduling();
 
-    void close();
-
     public static class Snapshot {
         private final SnapshotIndexCommit indexCommit;
         private final Translog.Snapshot translogSnapshot;

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/IndexShardGatewayService.java

@@ -36,7 +36,7 @@ import org.elasticsearch.index.translog.Translog;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.util.StopWatch;
 import org.elasticsearch.util.TimeValue;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.settings.Settings;
 
 import java.io.IOException;
@@ -46,7 +46,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 /**
  * @author kimchy (Shay Banon)
  */
-public class IndexShardGatewayService extends AbstractIndexShardComponent implements CloseableComponent {
+public class IndexShardGatewayService extends AbstractIndexShardComponent implements CloseableIndexComponent {
 
     private final boolean snapshotOnClose;
 
@@ -166,16 +166,20 @@ public class IndexShardGatewayService extends AbstractIndexShardComponent implem
         }
     }
 
-    public void close() {
+    public void close(boolean delete) {
         if (snapshotScheduleFuture != null) {
             snapshotScheduleFuture.cancel(true);
             snapshotScheduleFuture = null;
         }
-        if (snapshotOnClose) {
+        if (!delete && snapshotOnClose) {
             logger.debug("Snapshotting on close ...");
             snapshot();
         }
-        shardGateway.close();
+        // don't really delete the shard gateway if we are primary...
+        if (!indexShard.routingEntry().primary()) {
+            delete = false;
+        }
+        shardGateway.close(delete);
     }
 
     private synchronized void scheduleSnapshotIfNeeded() {

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/fs/FsIndexGateway.java

@@ -30,11 +30,12 @@ import org.elasticsearch.index.gateway.IndexGateway;
 import org.elasticsearch.index.gateway.IndexShardGateway;
 import org.elasticsearch.index.settings.IndexSettings;
 import org.elasticsearch.util.Strings;
-import org.elasticsearch.util.io.FileSystemUtils;
 import org.elasticsearch.util.settings.Settings;
 
 import java.io.File;
 
+import static org.elasticsearch.util.io.FileSystemUtils.*;
+
 /**
  * @author kimchy (Shay Banon)
  */
@@ -84,11 +85,15 @@ public class FsIndexGateway extends AbstractIndexComponent implements IndexGatew
         return FsIndexShardGateway.class;
     }
 
-    @Override public void delete() {
-        FileSystemUtils.deleteRecursively(indexGatewayHome, false);
-    }
-
-    @Override public void close() {
+    @Override public void close(boolean delete) {
+        try {
+            String[] files = indexGatewayHome.list();
+            if (files == null || files.length == 0) {
+                deleteRecursively(indexGatewayHome, true);
+            }
+        } catch (Exception e) {
+            // ignore
+        }
     }
 
     public File indexGatewayHome() {

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/fs/FsIndexShardGateway.java

@@ -93,7 +93,10 @@ public class FsIndexShardGateway extends AbstractIndexShardComponent implements
         return "fs[" + location + "]";
     }
 
-    @Override public void close() {
+    @Override public void close(boolean delete) {
+        if (delete) {
+            deleteRecursively(location, true);
+        }
     }
 
     @Override public RecoveryStatus recover() throws IndexShardGatewayRecoveryException {

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/none/NoneIndexGateway.java

@@ -44,9 +44,6 @@ public class NoneIndexGateway extends AbstractIndexComponent implements IndexGat
         return "none";
     }
 
-    @Override public void delete() {
-    }
-
-    @Override public void close() {
+    @Override public void close(boolean delete) {
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/gateway/none/NoneIndexShardGateway.java

@@ -58,6 +58,6 @@ public class NoneIndexShardGateway extends AbstractIndexShardComponent implement
         return false;
     }
 
-    @Override public void close() {
+    @Override public void close(boolean delete) {
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/service/IndexService.java

@@ -30,6 +30,7 @@ import org.elasticsearch.index.query.IndexQueryParserService;
 import org.elasticsearch.index.routing.OperationRouting;
 import org.elasticsearch.index.shard.service.IndexShard;
 import org.elasticsearch.index.similarity.SimilarityService;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 
 import java.util.Set;
 
@@ -37,9 +38,7 @@ import java.util.Set;
  * @author kimchy (Shay Banon)
  */
 @IndexLifecycle
-public interface IndexService extends IndexComponent, Iterable<IndexShard> {
-
-    void close();
+public interface IndexService extends IndexComponent, Iterable<IndexShard>, CloseableIndexComponent {
 
     Injector injector();
 

modules/elasticsearch/src/main/java/org/elasticsearch/index/service/InternalIndexService.java

@@ -49,7 +49,7 @@ import org.elasticsearch.index.store.StoreModule;
 import org.elasticsearch.index.translog.TranslogModule;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.plugins.ShardsPluginsModule;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.guice.Injectors;
 import org.elasticsearch.util.settings.Settings;
 
@@ -154,9 +154,9 @@ public class InternalIndexService extends AbstractIndexComponent implements Inde
         return similarityService;
     }
 
-    @Override public synchronized void close() {
+    @Override public synchronized void close(boolean delete) {
         for (int shardId : shardIds()) {
-            deleteShard(shardId, true);
+            deleteShard(shardId, delete);
         }
     }
 
@@ -209,20 +209,20 @@ public class InternalIndexService extends AbstractIndexComponent implements Inde
     }
 
     @Override public synchronized void deleteShard(int shardId) throws ElasticSearchException {
-        deleteShard(shardId, false);
+        deleteShard(shardId, true);
     }
 
-    private synchronized void deleteShard(int shardId, boolean close) throws ElasticSearchException {
+    private synchronized void deleteShard(int shardId, boolean delete) throws ElasticSearchException {
         Map<Integer, Injector> tmpShardInjectors = newHashMap(shardsInjectors);
         Injector shardInjector = tmpShardInjectors.remove(shardId);
         if (shardInjector == null) {
-            if (close) {
+            if (!delete) {
                 return;
             }
             throw new IndexShardMissingException(new ShardId(index, shardId));
         }
         shardsInjectors = ImmutableMap.copyOf(tmpShardInjectors);
-        if (!close) {
+        if (delete) {
             logger.debug("Deleting Shard Id [{}]", shardId);
         }
 
@@ -231,8 +231,8 @@ public class InternalIndexService extends AbstractIndexComponent implements Inde
         shards = ImmutableMap.copyOf(tmpShardsMap);
 
 
-        for (Class<? extends CloseableComponent> closeable : pluginsService.shardServices()) {
-            shardInjector.getInstance(closeable).close();
+        for (Class<? extends CloseableIndexComponent> closeable : pluginsService.shardServices()) {
+            shardInjector.getInstance(closeable).close(delete);
         }
 
         // close shard actions
@@ -241,11 +241,10 @@ public class InternalIndexService extends AbstractIndexComponent implements Inde
         RecoveryAction recoveryAction = shardInjector.getInstance(RecoveryAction.class);
         if (recoveryAction != null) recoveryAction.close();
 
-        shardInjector.getInstance(IndexShardGatewayService.class).close();
+        shardInjector.getInstance(IndexShardGatewayService.class).close(delete);
 
         indexShard.close();
 
-
         Engine engine = shardInjector.getInstance(Engine.class);
         engine.close();
 

modules/elasticsearch/src/main/java/org/elasticsearch/index/store/fs/AbstractFsStore.java

@@ -47,7 +47,8 @@ public abstract class AbstractFsStore<T extends Directory> extends AbstractStore
     @Override public void fullDelete() throws IOException {
         FileSystemUtils.deleteRecursively(fsDirectory().getFile());
         // if we are the last ones, delete also the actual index
-        if (fsDirectory().getFile().getParentFile().list().length == 0) {
+        String[] list = fsDirectory().getFile().getParentFile().list();
+        if (list == null || list.length == 0) {
             FileSystemUtils.deleteRecursively(fsDirectory().getFile().getParentFile());
         }
     }

modules/elasticsearch/src/main/java/org/elasticsearch/indices/InternalIndicesService.java

@@ -44,7 +44,7 @@ import org.elasticsearch.indices.cluster.IndicesClusterStateService;
 import org.elasticsearch.plugins.IndicesPluginsModule;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.util.component.AbstractLifecycleComponent;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.concurrent.ThreadSafe;
 import org.elasticsearch.util.guice.Injectors;
 import org.elasticsearch.util.settings.Settings;
@@ -90,7 +90,7 @@ public class InternalIndicesService extends AbstractLifecycleComponent<IndicesSe
     @Override protected void doStop() throws ElasticSearchException {
         clusterStateService.stop();
         for (String index : indices.keySet()) {
-            deleteIndex(index, true);
+            deleteIndex(index, false);
         }
     }
 
@@ -182,18 +182,18 @@ public class InternalIndicesService extends AbstractLifecycleComponent<IndicesSe
     }
 
     public synchronized void deleteIndex(String index) throws ElasticSearchException {
-        deleteIndex(index, false);
+        deleteIndex(index, true);
     }
 
-    private synchronized void deleteIndex(String index, boolean internalClose) throws ElasticSearchException {
+    private synchronized void deleteIndex(String index, boolean delete) throws ElasticSearchException {
         Injector indexInjector = indicesInjectors.remove(index);
         if (indexInjector == null) {
-            if (internalClose) {
+            if (!delete) {
                 return;
             }
             throw new IndexMissingException(new Index(index));
         }
-        if (!internalClose) {
+        if (delete) {
             logger.debug("Deleting Index [{}]", index);
         }
 
@@ -201,20 +201,17 @@ public class InternalIndicesService extends AbstractLifecycleComponent<IndicesSe
         IndexService indexService = tmpMap.remove(index);
         indices = ImmutableMap.copyOf(tmpMap);
 
-        for (Class<? extends CloseableComponent> closeable : pluginsService.indexServices()) {
-            indexInjector.getInstance(closeable).close();
+        for (Class<? extends CloseableIndexComponent> closeable : pluginsService.indexServices()) {
+            indexInjector.getInstance(closeable).close(delete);
         }
 
-        indexService.close();
+        indexService.close(delete);
 
         indexInjector.getInstance(FilterCache.class).close();
         indexInjector.getInstance(AnalysisService.class).close();
         indexInjector.getInstance(IndexServiceManagement.class).close();
 
-        if (!internalClose) {
-            indexInjector.getInstance(IndexGateway.class).delete();
-        }
-        indexInjector.getInstance(IndexGateway.class).close();
+        indexInjector.getInstance(IndexGateway.class).close(delete);
 
         Injectors.close(injector);
     }

modules/elasticsearch/src/main/java/org/elasticsearch/plugins/AbstractPlugin.java

@@ -21,7 +21,7 @@ package org.elasticsearch.plugins;
 
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Module;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.component.LifecycleComponent;
 
 import java.util.Collection;
@@ -57,7 +57,7 @@ public abstract class AbstractPlugin implements Plugin {
     /**
      * Defaults to return an empty list.
      */
-    @Override public Collection<Class<? extends CloseableComponent>> indexServices() {
+    @Override public Collection<Class<? extends CloseableIndexComponent>> indexServices() {
         return ImmutableList.of();
     }
 
@@ -71,7 +71,7 @@ public abstract class AbstractPlugin implements Plugin {
     /**
      * Defaults to return an empty list.
      */
-    @Override public Collection<Class<? extends CloseableComponent>> shardServices() {
+    @Override public Collection<Class<? extends CloseableIndexComponent>> shardServices() {
         return ImmutableList.of();
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/plugins/Plugin.java

@@ -20,7 +20,7 @@
 package org.elasticsearch.plugins;
 
 import com.google.inject.Module;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.component.LifecycleComponent;
 
 import java.util.Collection;
@@ -60,7 +60,7 @@ public interface Plugin {
     /**
      * Per index services that will be automatically closed.
      */
-    Collection<Class<? extends CloseableComponent>> indexServices();
+    Collection<Class<? extends CloseableIndexComponent>> indexServices();
 
     /**
      * Per index shard module.
@@ -70,5 +70,5 @@ public interface Plugin {
     /**
      * Per index shard service that will be automatically closed.
      */
-    Collection<Class<? extends CloseableComponent>> shardServices();
+    Collection<Class<? extends CloseableIndexComponent>> shardServices();
 }

modules/elasticsearch/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -26,7 +26,7 @@ import com.google.inject.Inject;
 import com.google.inject.Module;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.util.component.AbstractComponent;
-import org.elasticsearch.util.component.CloseableComponent;
+import org.elasticsearch.util.component.CloseableIndexComponent;
 import org.elasticsearch.util.component.LifecycleComponent;
 import org.elasticsearch.util.io.Streams;
 import org.elasticsearch.util.settings.Settings;
@@ -93,8 +93,8 @@ public class PluginsService extends AbstractComponent {
         return modules;
     }
 
-    public Collection<Class<? extends CloseableComponent>> indexServices() {
-        List<Class<? extends CloseableComponent>> services = Lists.newArrayList();
+    public Collection<Class<? extends CloseableIndexComponent>> indexServices() {
+        List<Class<? extends CloseableIndexComponent>> services = Lists.newArrayList();
         for (Plugin plugin : plugins.values()) {
             services.addAll(plugin.indexServices());
         }
@@ -109,8 +109,8 @@ public class PluginsService extends AbstractComponent {
         return modules;
     }
 
-    public Collection<Class<? extends CloseableComponent>> shardServices() {
-        List<Class<? extends CloseableComponent>> services = Lists.newArrayList();
+    public Collection<Class<? extends CloseableIndexComponent>> shardServices() {
+        List<Class<? extends CloseableIndexComponent>> services = Lists.newArrayList();
         for (Plugin plugin : plugins.values()) {
             services.addAll(plugin.shardServices());
         }

modules/elasticsearch/src/main/java/org/elasticsearch/util/component/CloseableIndexComponent.java

@@ -0,0 +1,37 @@
+/*
+ * Licensed to Elastic Search and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Elastic Search licenses this
+ * file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.util.component;
+
+import org.elasticsearch.ElasticSearchException;
+
+/**
+ * @author kimchy (shay.banon)
+ */
+public interface CloseableIndexComponent {
+
+    /**
+     * Closes the index component. A boolean indicating if its part of an actual index
+     * deletion or not is passed.
+     *
+     * @param delete <tt>true</tt> if the index is being deleted.
+     * @throws ElasticSearchException
+     */
+    void close(boolean delete) throws ElasticSearchException;
+}

modules/test/integration/src/test/java/org/elasticsearch/test/integration/gateway/AbstractSimpleIndexGatewayTests.java

@@ -154,6 +154,9 @@ public abstract class AbstractSimpleIndexGatewayTests extends AbstractServersTes
         logger.info("Getting #3 (not from the translog, but from the index)");
         getResponse = client("server1").get(getRequest("test").type("type1").id("3")).actionGet();
         assertThat(getResponse.sourceAsString(), equalTo(source("3", "test")));
+
+        logger.info("Deleting the index");
+        client("server1").admin().indices().delete(deleteIndexRequest("test")).actionGet();
     }
 
     private String mappingSource() {