New setting to prevent automatically importing dangling indices

docs/reference/modules/gateway.asciidoc

@@ -52,9 +52,22 @@ NOTE: These settings only take effect on a full cluster restart.
 [[modules-gateway-dangling-indices]]
 === Dangling indices
 
-When a node joins the cluster, any shards stored in its local data
-directory which do not already exist in the cluster will be imported into the 
-cluster. This functionality is intended as a best effort to help users who 
-lose all master nodes. If a new master node is started which is unaware of 
-the other indices in the cluster, adding the old nodes will cause the old 
-indices to be imported, instead of being deleted.
+When a node joins the cluster, it will search for any shards that are
+stored in its local data directory and do not already exist in the
+cluster. If the static setting `gateway.auto_import_dangling_indices` is
+`true` (the default is `false`), then those shards will be imported into
+the cluster. This functionality is intended as a best effort to help users
+who lose all master nodes. If a new master node is started which is unaware
+of the other indices in the cluster, adding the old nodes will cause the
+old indices to be imported, instead of being deleted.
+
+Enabling `gateway.auto_import_dangling_indices` should only be done if
+absolutely necessary, after understanding the possible consequences (this is not an exhaustive list):
+
+* A deleted index might suddenly reappear when a node joins the cluster.
+* You might delete an index and see the immediate creation of another index
+  with the same name, containing stale mappings and old data.
+* New documents could be written to the index before anyone realises that
+  it has been recovered
+* {es} may not be able to find copies of all of the shards of the index,
+  resulting in a red cluster state.

server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -70,6 +70,7 @@
 import org.elasticsearch.discovery.SettingsBasedSeedHostsProvider;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
+import org.elasticsearch.gateway.DanglingIndicesState;
 import org.elasticsearch.gateway.GatewayService;
 import org.elasticsearch.gateway.IncrementalClusterStateWriter;
 import org.elasticsearch.http.HttpTransportSettings;
@@ -184,6 +185,7 @@ public void apply(Settings value, Settings current, Settings previous) {
             BalancedShardsAllocator.THRESHOLD_SETTING,
             ClusterRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ALLOW_REBALANCE_SETTING,
             ConcurrentRebalanceAllocationDecider.CLUSTER_ROUTING_ALLOCATION_CLUSTER_CONCURRENT_REBALANCE_SETTING,
+            DanglingIndicesState.AUTO_IMPORT_DANGLING_INDICES_SETTING,
             EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING,
             EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING,
             FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING,

server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java

@@ -30,6 +30,7 @@
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.index.Index;
@@ -55,19 +56,33 @@ public class DanglingIndicesState implements ClusterStateListener {
 
     private static final Logger logger = LogManager.getLogger(DanglingIndicesState.class);
 
+    public static final Setting<Boolean> AUTO_IMPORT_DANGLING_INDICES_SETTING = Setting.boolSetting(
+        "gateway.auto_import_dangling_indices",
+        false,
+        Setting.Property.NodeScope
+    );
+
     private final NodeEnvironment nodeEnv;
     private final MetaStateService metaStateService;
     private final LocalAllocateDangledIndices allocateDangledIndices;
 
     private final Map<Index, IndexMetaData> danglingIndices = ConcurrentCollections.newConcurrentMap();
 
+    private boolean allocateDanglingIndices;
+
     @Inject
     public DanglingIndicesState(NodeEnvironment nodeEnv, MetaStateService metaStateService,
                                 LocalAllocateDangledIndices allocateDangledIndices, ClusterService clusterService) {
         this.nodeEnv = nodeEnv;
         this.metaStateService = metaStateService;
         this.allocateDangledIndices = allocateDangledIndices;
         clusterService.addListener(this);
+
+        this.allocateDanglingIndices = AUTO_IMPORT_DANGLING_INDICES_SETTING.get(clusterService.getSettings());
+    }
+
+    public void setAllocateDanglingIndicesSetting(boolean allocateDanglingIndices) {
+        this.allocateDanglingIndices = allocateDanglingIndices;
     }
 
     /**
@@ -171,10 +186,15 @@ private IndexMetaData stripAliases(IndexMetaData indexMetaData) {
      * Allocates the provided list of the dangled indices by sending them to the master node
      * for allocation.
      */
-    private void allocateDanglingIndices() {
+    void allocateDanglingIndices() {
+        if (this.allocateDanglingIndices == false) {
+            return;
+        }
+
         if (danglingIndices.isEmpty()) {
             return;
         }
+
         try {
             allocateDangledIndices.allocateDangled(Collections.unmodifiableCollection(new ArrayList<>(danglingIndices.values())),
                 new ActionListener<>() {

server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapAndDetachCommandIT.java

@@ -44,6 +44,7 @@
 import java.util.Locale;
 
 import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
+import static org.elasticsearch.gateway.DanglingIndicesState.AUTO_IMPORT_DANGLING_INDICES_SETTING;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
@@ -396,8 +397,13 @@ public boolean clearData(String nodeName) {
             }
         });
 
+        final Settings settingsWithAutoImport = Settings.builder()
+            .put(dataNodeDataPathSettings)
+            .put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), true)
+            .build();
+
         logger.info("--> start data-only only node and ensure 2 nodes stable cluster");
-        internalCluster().startDataOnlyNode(dataNodeDataPathSettings);
+        internalCluster().startDataOnlyNode(settingsWithAutoImport);
         ensureStableCluster(2);
 
         logger.info("--> verify that the dangling index exists and has green status");

server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.index.Index;
@@ -35,51 +36,73 @@
 import java.nio.file.StandardCopyOption;
 import java.util.Map;
 
+import static org.elasticsearch.gateway.DanglingIndicesState.AUTO_IMPORT_DANGLING_INDICES_SETTING;
 import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class DanglingIndicesStateTests extends ESTestCase {
 
     private static Settings indexSettings = Settings.builder()
-            .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
-            .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
-            .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-            .build();
+        .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
+        .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
+        .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+        .build();
 
     public void testCleanupWhenEmpty() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
 
+            // Given an empty state
             assertTrue(danglingState.getDanglingIndices().isEmpty());
+
+            // When passed an empty metadata
             MetaData metaData = MetaData.builder().build();
             danglingState.cleanupAllocatedDangledIndices(metaData);
+
+            // Then the state remains empty
             assertTrue(danglingState.getDanglingIndices().isEmpty());
         }
     }
+
     public void testDanglingIndicesDiscovery() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
 
+            // Given an empty state
             assertTrue(danglingState.getDanglingIndices().isEmpty());
+
+            // When passed a metdata with an unknown index
             MetaData metaData = MetaData.builder().build();
             final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
             IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
             metaStateService.writeIndex("test_write", dangledIndex);
             Map<Index, IndexMetaData> newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
+
+            // Then that index is considered dangling
             assertTrue(newDanglingIndices.containsKey(dangledIndex.getIndex()));
+
+            // And when passed another metadata with that index
             metaData = MetaData.builder().put(dangledIndex, false).build();
             newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
+
+            // Then then index is not considered to be a new dangling index for a second time
             assertFalse(newDanglingIndices.containsKey(dangledIndex.getIndex()));
         }
     }
 
     public void testInvalidIndexFolder() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
+            // Given an empty state
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
 
+            // When passed settings for an index whose folder does not exist
             MetaData metaData = MetaData.builder().build();
             final String uuid = "test1UUID";
             final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, uuid);
@@ -90,6 +113,8 @@ public void testInvalidIndexFolder() throws Exception {
                     Files.move(path, path.resolveSibling("invalidUUID"), StandardCopyOption.ATOMIC_MOVE);
                 }
             }
+
+            // Then an exception is thrown describing the problem
             try {
                 danglingState.findNewDanglingIndices(metaData);
                 fail("no exception thrown for invalid folder name");
@@ -145,25 +170,31 @@ public void testDanglingProcessing() throws Exception {
 
     public void testDanglingIndicesNotImportedWhenTombstonePresent() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
+            // Given an empty state
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
 
+            // When passed a dangling index
             final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
             IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
             metaStateService.writeIndex("test_write", dangledIndex);
 
+            // And there is a tombstone for that index
             final IndexGraveyard graveyard = IndexGraveyard.builder().addTombstone(dangledIndex.getIndex()).build();
             final MetaData metaData = MetaData.builder().indexGraveyard(graveyard).build();
-            assertThat(danglingState.findNewDanglingIndices(metaData).size(), equalTo(0));
 
+            // Then that index is not imported
+            assertThat(danglingState.findNewDanglingIndices(metaData).size(), equalTo(0));
         }
     }
 
     public void testDanglingIndicesStripAliases() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
+            // Given an empty state
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
 
+            // When passed an index that has an alias
             final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
             IndexMetaData dangledIndex = IndexMetaData.builder("test1")
                 .settings(settings)
@@ -174,14 +205,86 @@ public void testDanglingIndicesStripAliases() throws Exception {
 
             final MetaData metaData = MetaData.builder().build();
             Map<Index, IndexMetaData> newDanglingIndices = danglingState.findNewDanglingIndices(metaData);
+
+            // Then the index is identifying as dangling
             assertThat(newDanglingIndices.size(), equalTo(1));
             Map.Entry<Index, IndexMetaData> entry = newDanglingIndices.entrySet().iterator().next();
             assertThat(entry.getKey().getName(), equalTo("test1"));
+
+            // And the alias is removed
             assertThat(entry.getValue().getAliases().size(), equalTo(0));
         }
     }
 
+    public void testDanglingIndicesAreNotAllocatedWhenDisabled() throws Exception {
+        try (NodeEnvironment env = newNodeEnvironment()) {
+            MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
+            LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
+            DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices);
+
+            assertTrue(danglingState.getDanglingIndices().isEmpty());
+
+            // Given a metadata that does not enable allocation of dangling indices
+            MetaData metaData = MetaData.builder().build();
+
+            final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
+            IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
+            metaStateService.writeIndex("test_write", dangledIndex);
+
+            danglingState.findNewAndAddDanglingIndices(metaData);
+
+            // When calling the allocate method
+            danglingState.allocateDanglingIndices();
+
+            // Then allocation is not attempted
+            verify(localAllocateDangledIndices, never()).allocateDangled(any(), any());
+        }
+    }
+
+    public void testDanglingIndicesAreAllocatedWhenEnabled() throws Exception {
+        try (NodeEnvironment env = newNodeEnvironment()) {
+            MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
+            LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
+            DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, localAllocateDangledIndices);
+
+            assertTrue(danglingState.getDanglingIndices().isEmpty());
+
+            // Given a state where automatic allocation is enabled
+            danglingState.setAllocateDanglingIndicesSetting(true);
+
+            MetaData metaData = MetaData.builder().build();
+
+            final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
+            IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(settings).build();
+            metaStateService.writeIndex("test_write", dangledIndex);
+
+            danglingState.findNewAndAddDanglingIndices(metaData);
+
+            // When calling the allocate method
+            danglingState.allocateDanglingIndices();
+
+            // Ensure that allocation is attempted
+            verify(localAllocateDangledIndices).allocateDangled(any(), any());
+        }
+    }
+
     private DanglingIndicesState createDanglingIndicesState(NodeEnvironment env, MetaStateService metaStateService) {
-        return new DanglingIndicesState(env, metaStateService, null, mock(ClusterService.class));
+        return createDanglingIndicesState(env, metaStateService, null);
+    }
+
+    private DanglingIndicesState createDanglingIndicesState(
+        NodeEnvironment env,
+        MetaStateService metaStateService,
+        LocalAllocateDangledIndices indexAllocator
+    ) {
+        final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), false).build();
+
+        final ClusterService clusterServiceMock = mock(ClusterService.class);
+        when(clusterServiceMock.getClusterSettings()).thenReturn(
+            new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS)
+        );
+        when(clusterServiceMock.getSettings()).thenReturn(allocateSettings);
+
+        return new DanglingIndicesState(env, metaStateService, indexAllocator, clusterServiceMock);
     }
 }