From b1ff74f6521176a5cccbc984dbc9de5f13874761 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Fri, 29 Nov 2019 11:39:51 +0000 Subject: [PATCH] New setting to prevent automatically importing dangling indices (#49174) Introduce a new static setting, `gateway.auto_import_dangling_indices`, which prevents dangling indices from being automatically imported. Part of #48366. --- .../common/settings/ClusterSettings.java | 2 + .../gateway/DanglingIndicesState.java | 24 ++++- .../gateway/DanglingIndicesStateTests.java | 71 +++++++++++++- .../indices/recovery/DanglingIndicesIT.java | 95 +++++++++++++++++++ .../test/InternalTestCluster.java | 2 +- 5 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/indices/recovery/DanglingIndicesIT.java diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 752035d938e..a48c996b80b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -77,6 +77,7 @@ import org.elasticsearch.discovery.zen.FaultDetection; import org.elasticsearch.discovery.zen.ZenDiscovery; 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; @@ -199,6 +200,7 @@ public final class ClusterSettings extends AbstractScopedSettings { 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 index 5f76177735c..16bd23399e0 100644 --- 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.IndexMetaData; 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; @@ -56,9 +57,17 @@ public class DanglingIndicesState implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(DanglingIndicesState.class); + public static final Setting 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 danglingIndices = ConcurrentCollections.newConcurrentMap(); @@ -68,7 +77,18 @@ public class DanglingIndicesState implements ClusterStateListener { 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; } /** @@ -172,7 +192,7 @@ public class DanglingIndicesState implements ClusterStateListener { * 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 index e7dfbadeeda..42c546e1928 100644 --- 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.Path; 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 class DanglingIndicesStateTests extends ESTestCase { 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 class DanglingIndicesStateTests extends ESTestCase { 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 class DanglingIndicesStateTests extends ESTestCase { } } + 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 new file mode 100644 index 00000000000..31afef73ad4 --- /dev/null +++ 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/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index c774959d3a6..9841f060de8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -1635,7 +1635,7 @@ public final class InternalTestCluster extends TestCluster { } /** - * 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 filter) throws IOException {