New setting to prevent automatically importing dangling indices (#49174)

pugnascotia · web-flow · commit f8e39d2ff18c · 2019-11-29T11:39:51.000Z
Introduce a new static setting, `gateway.auto_import_dangling_indices`, which prevents dangling indices from being automatically imported. Part of #48366.
diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/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;
@@ -186,6 +187,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,
diff --git a/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java b/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,9 +56,17 @@ 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",
+        true,
+        Setting.Property.NodeScope,
+        Setting.Property.Deprecated
+    );
+
     private final NodeEnvironment nodeEnv;
     private final MetaStateService metaStateService;
     private final LocalAllocateDangledIndices allocateDangledIndices;
+    private final boolean isAutoImportDanglingIndicesEnabled;
 
     private final Map<Index, IndexMetaData> danglingIndices = ConcurrentCollections.newConcurrentMap();
 
@@ -67,7 +76,18 @@ public DanglingIndicesState(NodeEnvironment nodeEnv, MetaStateService metaStateS
         this.nodeEnv = nodeEnv;
         this.metaStateService = metaStateService;
         this.allocateDangledIndices = allocateDangledIndices;
-        clusterService.addListener(this);
+
+        this.isAutoImportDanglingIndicesEnabled = AUTO_IMPORT_DANGLING_INDICES_SETTING.get(clusterService.getSettings());
+
+        if (this.isAutoImportDanglingIndicesEnabled) {
+            clusterService.addListener(this);
+        } else {
+            logger.warn(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey() + " is disabled, dangling indices will not be detected or imported");
+        }
+    }
+
+    boolean isAutoImportDanglingIndicesEnabled() {
+        return this.isAutoImportDanglingIndicesEnabled;
     }
 
     /**
@@ -171,7 +191,7 @@ 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 (danglingIndices.isEmpty()) {
             return;
         }
diff --git a/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java b/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java
@@ -35,8 +35,12 @@
 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.verify;
+import static org.mockito.Mockito.when;
 
 public class DanglingIndicesStateTests extends ESTestCase {
 
@@ -46,6 +50,13 @@ public class DanglingIndicesStateTests extends ESTestCase {
             .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
             .build();
 
+    // The setting AUTO_IMPORT_DANGLING_INDICES_SETTING is deprecated, so we must disable
+    // warning checks or all the tests will fail.
+    @Override
+    protected boolean enableWarningsCheck() {
+        return false;
+    }
+
     public void testCleanupWhenEmpty() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
@@ -57,11 +68,11 @@ public void testCleanupWhenEmpty() throws Exception {
             assertTrue(danglingState.getDanglingIndices().isEmpty());
         }
     }
+
     public void testDanglingIndicesDiscovery() throws Exception {
         try (NodeEnvironment env = newNodeEnvironment()) {
             MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
             DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService);
-
             assertTrue(danglingState.getDanglingIndices().isEmpty());
             MetaData metaData = MetaData.builder().build();
             final Settings.Builder settings = Settings.builder().put(indexSettings).put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID");
@@ -155,7 +166,6 @@ public void testDanglingIndicesNotImportedWhenTombstonePresent() throws Exceptio
             final IndexGraveyard graveyard = IndexGraveyard.builder().addTombstone(dangledIndex.getIndex()).build();
             final MetaData metaData = MetaData.builder().indexGraveyard(graveyard).build();
             assertThat(danglingState.findNewDanglingIndices(metaData).size(), equalTo(0));
-
         }
     }
 
@@ -181,7 +191,62 @@ public void testDanglingIndicesStripAliases() throws Exception {
         }
     }
 
+    public void testDanglingIndicesAreNotAllocatedWhenDisabled() throws Exception {
+        try (NodeEnvironment env = newNodeEnvironment()) {
+            MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
+            LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
+
+            final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), false).build();
+
+            final ClusterService clusterServiceMock = mock(ClusterService.class);
+            when(clusterServiceMock.getSettings()).thenReturn(allocateSettings);
+
+            final DanglingIndicesState danglingIndicesState = new DanglingIndicesState(
+                env,
+                metaStateService,
+                localAllocateDangledIndices,
+                clusterServiceMock
+            );
+
+            assertFalse("Expected dangling imports to be disabled", danglingIndicesState.isAutoImportDanglingIndicesEnabled());
+        }
+    }
+
+    public void testDanglingIndicesAreAllocatedWhenEnabled() throws Exception {
+        try (NodeEnvironment env = newNodeEnvironment()) {
+            MetaStateService metaStateService = new MetaStateService(env, xContentRegistry());
+            LocalAllocateDangledIndices localAllocateDangledIndices = mock(LocalAllocateDangledIndices.class);
+            final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), true).build();
+
+            final ClusterService clusterServiceMock = mock(ClusterService.class);
+            when(clusterServiceMock.getSettings()).thenReturn(allocateSettings);
+
+            DanglingIndicesState danglingIndicesState = new DanglingIndicesState(
+                env,
+                metaStateService,
+                localAllocateDangledIndices, clusterServiceMock
+            );
+
+            assertTrue("Expected dangling imports to be enabled", danglingIndicesState.isAutoImportDanglingIndicesEnabled());
+
+            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);
+
+            danglingIndicesState.findNewAndAddDanglingIndices(MetaData.builder().build());
+
+            danglingIndicesState.allocateDanglingIndices();
+
+            verify(localAllocateDangledIndices).allocateDangled(any(), any());
+        }
+    }
+
     private DanglingIndicesState createDanglingIndicesState(NodeEnvironment env, MetaStateService metaStateService) {
-        return new DanglingIndicesState(env, metaStateService, null, mock(ClusterService.class));
+        final Settings allocateSettings = Settings.builder().put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), true).build();
+
+        final ClusterService clusterServiceMock = mock(ClusterService.class);
+        when(clusterServiceMock.getSettings()).thenReturn(allocateSettings);
+
+        return new DanglingIndicesState(env, metaStateService, null, clusterServiceMock);
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/indices/recovery/DanglingIndicesIT.java b/server/src/test/java/org/elasticsearch/indices/recovery/DanglingIndicesIT.java
@@ -0,0 +1,95 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.indices.recovery;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
+import org.elasticsearch.test.InternalTestCluster;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.elasticsearch.cluster.metadata.IndexGraveyard.SETTING_MAX_TOMBSTONES;
+import static org.elasticsearch.gateway.DanglingIndicesState.AUTO_IMPORT_DANGLING_INDICES_SETTING;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+
+@ClusterScope(numDataNodes = 0, scope = ESIntegTestCase.Scope.TEST)
+public class DanglingIndicesIT extends ESIntegTestCase {
+    private static final String INDEX_NAME = "test-idx-1";
+
+    private Settings buildSettings(boolean importDanglingIndices) {
+        return Settings.builder()
+            // Don't keep any indices in the graveyard, so that when we delete an index,
+            // it's definitely considered to be dangling.
+            .put(SETTING_MAX_TOMBSTONES.getKey(), 0)
+            .put(AUTO_IMPORT_DANGLING_INDICES_SETTING.getKey(), importDanglingIndices)
+            .build();
+    }
+
+    /**
+     * Check that when dangling indices are discovered, then they are recovered into
+     * the cluster, so long as the recovery setting is enabled.
+     */
+    public void testDanglingIndicesAreRecoveredWhenSettingIsEnabled() throws Exception {
+        final Settings settings = buildSettings(true);
+        internalCluster().startNodes(3, settings);
+
+        createIndex(INDEX_NAME, Settings.builder().put("number_of_replicas", 2).build());
+
+        // Restart node, deleting the index in its absence, so that there is a dangling index to recover
+        internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() {
+
+            @Override
+            public Settings onNodeStopped(String nodeName) throws Exception {
+                assertAcked(client().admin().indices().prepareDelete(INDEX_NAME));
+                return super.onNodeStopped(nodeName);
+            }
+        });
+
+        assertBusy(() -> assertTrue("Expected dangling index " + INDEX_NAME + " to be recovered", indexExists(INDEX_NAME)));
+    }
+
+    /**
+     * Check that when dangling indices are discovered, then they are not recovered into
+     * the cluster when the recovery setting is disabled.
+     */
+    public void testDanglingIndicesAreNotRecoveredWhenSettingIsDisabled() throws Exception {
+        internalCluster().startNodes(3, buildSettings(false));
+
+        createIndex(INDEX_NAME, Settings.builder().put("number_of_replicas", 2).build());
+
+        // Restart node, deleting the index in its absence, so that there is a dangling index to recover
+        internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() {
+
+            @Override
+            public Settings onNodeStopped(String nodeName) throws Exception {
+                assertAcked(client().admin().indices().prepareDelete(INDEX_NAME));
+                return super.onNodeStopped(nodeName);
+            }
+        });
+
+        // Since index recovery is async, we can't prove index recovery will never occur, just that it doesn't occur within some reasonable
+        // amount of time
+        assertFalse(
+            "Did not expect dangling index " + INDEX_NAME + " to be recovered",
+            waitUntil(() -> indexExists(INDEX_NAME), 1, TimeUnit.SECONDS)
+        );
+    }
+}
diff --git a/server/src/test/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java b/server/src/test/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java
@@ -142,7 +142,6 @@
 public class IndexRecoveryIT extends ESIntegTestCase {
 
     private static final String INDEX_NAME = "test-idx-1";
-    private static final String INDEX_TYPE = "test-type-1";
     private static final String REPO_NAME = "test-repo-1";
     private static final String SNAP_NAME = "test-snap-1";
 
diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
@@ -1487,7 +1487,7 @@ public synchronized boolean stopRandomDataNode() throws IOException {
     }
 
     /**
-     * Stops a random node in the cluster that applies to the given filter or non if the non of the nodes applies to the
+     * Stops a random node in the cluster that applies to the given filter. Does nothing if none of the nodes match the
      * filter.
      */
     public synchronized void stopRandomNode(final Predicate<Settings> filter) throws IOException {

Original file line number	Diff line number	Diff line change
`@@ -1487,7 +1487,7 @@ public synchronized boolean stopRandomDataNode() throws IOException {`
`1487`	`1487`	`}`
`1488`	`1488`
`1489`	`1489`	`/**`
`1490`		`- * Stops a random node in the cluster that applies to the given filter or non if the non of the nodes applies to the`
	`1490`	`+ * Stops a random node in the cluster that applies to the given filter. Does nothing if none of the nodes match the`
`1491`	`1491`	`* filter.`
`1492`	`1492`	`*/`
`1493`	`1493`	`public synchronized void stopRandomNode(final Predicate<Settings> filter) throws IOException {`