From c4ba7eb39de684a9c3d03c1b0dda007f527e2d8a Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 24 Jan 2019 15:35:46 +0100 Subject: [PATCH 1/6] Ignore obsolete dangling indices For a non-data, non-master node we now warn about dangling indices and will otherwise ignore them. This avoids import of old indices with a following inevitable red cluster status. Issue #27073 --- .../gateway/DanglingIndicesState.java | 12 ++++- .../gateway/DanglingIndicesStateTests.java | 51 ++++++++++++++++++- .../gateway/GatewayIndexStateIT.java | 35 +++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java b/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java index 4d7949cdf4de8..8f1fa5017e547 100644 --- a/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java +++ b/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java @@ -27,8 +27,10 @@ import org.elasticsearch.cluster.metadata.IndexGraveyard; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.Index; @@ -55,6 +57,7 @@ public class DanglingIndicesState implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(DanglingIndicesState.class); + private final Settings settings; private final NodeEnvironment nodeEnv; private final MetaStateService metaStateService; private final LocalAllocateDangledIndices allocateDangledIndices; @@ -62,8 +65,9 @@ public class DanglingIndicesState implements ClusterStateListener { private final Map danglingIndices = ConcurrentCollections.newConcurrentMap(); @Inject - public DanglingIndicesState(NodeEnvironment nodeEnv, MetaStateService metaStateService, + public DanglingIndicesState(Settings settings, NodeEnvironment nodeEnv, MetaStateService metaStateService, LocalAllocateDangledIndices allocateDangledIndices, ClusterService clusterService) { + this.settings = settings; this.nodeEnv = nodeEnv; this.metaStateService = metaStateService; this.allocateDangledIndices = allocateDangledIndices; @@ -132,8 +136,12 @@ Map findNewDanglingIndices(final MetaData metaData) { final List indexMetaDataList = metaStateService.loadIndicesStates(excludeIndexPathIds::contains); Map newIndices = new HashMap<>(indexMetaDataList.size()); final IndexGraveyard graveyard = metaData.indexGraveyard(); + boolean coordinatingOnly = DiscoveryNode.isDataNode(settings) == false + && DiscoveryNode.isMasterNode(settings) == false; for (IndexMetaData indexMetaData : indexMetaDataList) { - if (metaData.hasIndex(indexMetaData.getIndex().getName())) { + if (coordinatingOnly) { + logger.warn("[{}] dangling index ignored for non-data, non-master node", indexMetaData.getIndex()); + } else if (metaData.hasIndex(indexMetaData.getIndex().getName())) { logger.warn("[{}] can not be imported as a dangling index, as index with same name already exists in cluster metadata", indexMetaData.getIndex()); } else if (graveyard.containsIndex(indexMetaData.getIndex())) { diff --git a/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java b/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java index 9593b58eae97c..b4d4d6cf0d264 100644 --- a/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java @@ -26,9 +26,11 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.Index; +import org.elasticsearch.node.Node; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -158,7 +160,54 @@ public void testDanglingIndicesNotImportedWhenTombstonePresent() throws Exceptio } } + public void testDanglingIndicesIgnoredWhenObsolete() throws IOException { + Settings noDataNoMasterSettings = Settings.builder() + .put(Node.NODE_DATA_SETTING.getKey(), false) + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .build(); + + Settings noDataSettings = Settings.builder() + .put(Node.NODE_DATA_SETTING.getKey(), false) + .build(); + + Settings noMasterSettings = Settings.builder() + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .build(); + + verifyDanglingIndicesIgnoredWhenObsolete(noDataNoMasterSettings, 0, + "node.data=false and node.master=false nodes should not detect any dangling indices"); + verifyDanglingIndicesIgnoredWhenObsolete(noDataSettings, 1, + "node.data=false and node.master=true nodes should detect dangling indices"); + verifyDanglingIndicesIgnoredWhenObsolete(noMasterSettings, 1, + "node.data=true and node.master=false nodes should detect dangling indices"); + // also validated by #testDanglingIndicesDiscovery, included for completeness. + verifyDanglingIndicesIgnoredWhenObsolete(Settings.EMPTY, 1, + "node.data=true and node.master=true nodes should detect dangling indices"); + } + + private void verifyDanglingIndicesIgnoredWhenObsolete(Settings settings, int expected, String reason) throws IOException { + try (NodeEnvironment env = newNodeEnvironment(settings)) { + MetaStateService metaStateService = new MetaStateService(env, xContentRegistry()); + DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, settings); + + final Settings.Builder testIndexSettings = Settings.builder().put(indexSettings) + .put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID"); + IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(testIndexSettings).build(); + metaStateService.writeIndex("test_write", dangledIndex); + + assertThat(reason, + danglingState.findNewDanglingIndices(MetaData.builder().build()).size(), + equalTo(expected)); + } + } + private DanglingIndicesState createDanglingIndicesState(NodeEnvironment env, MetaStateService metaStateService) { - return new DanglingIndicesState(env, metaStateService, null, mock(ClusterService.class)); + return new DanglingIndicesState(Settings.EMPTY, env, metaStateService, null, mock(ClusterService.class)); + } + + private DanglingIndicesState createDanglingIndicesState(NodeEnvironment env, + MetaStateService metaStateService, + Settings settings) { + return new DanglingIndicesState(settings, env, metaStateService, null, mock(ClusterService.class)); } } diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java index ff8393b659d14..56dae77e14599 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.discovery.DiscoverySettings; import org.elasticsearch.discovery.zen.ElectMasterService; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.mapper.MapperParsingException; @@ -318,6 +319,40 @@ public boolean clearData(String nodeName) { assertThat(client().prepareGet("test", "type1", "1").execute().actionGet().isExists(), equalTo(true)); } + public void testDanglingIndicesIgnoredWhenObsolete() throws Exception { + logger.info("--> starting first node"); + internalCluster().startNode(); + + logger.info("--> indexing a simple document"); + client().prepareIndex("test", "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); + + internalCluster().fullRestart(new RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) throws Exception { + return Settings.builder() + .put(Node.NODE_DATA_SETTING.getKey(), false) + .put(Node.NODE_MASTER_SETTING.getKey(), false) + // avoid waiting for discovery. + .put(DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.getKey(), 0) + .build(); + } + + @Override + public boolean validateClusterForming() { + return false; + } + }); + + logger.info("--> starting second node (master)"); + internalCluster().startNode(); + + logger.info("--> verify green status"); + ensureGreen(); + + logger.info("--> verify that the dangling index does not exists"); + assertThat(client().admin().indices().prepareExists("test").execute().actionGet().isExists(), equalTo(false)); + } + /** * This test ensures that when an index deletion takes place while a node is offline, when that * node rejoins the cluster, it deletes the index locally instead of importing it as a dangling index. From 69f2f4bcdae029c0870da34e1348c0851d851a7d Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 31 Jan 2019 08:34:18 +0100 Subject: [PATCH 2/6] Warn about left-behind data or metadata Now warn about both left-behind data and metadata for non-data or non-data and non-master nodes. Disable dangling indices check completely for coordinating only nodes (non-data and non-master). --- .../elasticsearch/env/NodeEnvironment.java | 74 ++++++++ .../gateway/DanglingIndicesState.java | 13 +- .../env/NodeEnvironmentTests.java | 159 ++++++++++++++++++ .../gateway/DanglingIndicesStateTests.java | 28 ++- 4 files changed, 250 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 15458b1d70b31..5b8a0029c2892 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -59,6 +59,7 @@ import org.elasticsearch.monitor.fs.FsInfo; import org.elasticsearch.monitor.fs.FsProbe; import org.elasticsearch.monitor.jvm.JvmInfo; +import org.elasticsearch.node.Node; import java.io.Closeable; import java.io.IOException; @@ -311,6 +312,24 @@ public NodeEnvironment(Settings settings, Environment environment, Consumer shardDataPaths = collectIndexSubPaths(nodePaths, this::isShardPath); + if (shardDataPaths.isEmpty() == false) { + throw new IllegalStateException("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false, but has shard data: " + + shardDataPaths); + } + } + + private void ensureNoIndexMetaData(final NodePath[] nodePaths) throws IOException { + List indexMetaDataPaths = collectIndexSubPaths(nodePaths, this::isIndexMetaDataPath); + if (indexMetaDataPaths.isEmpty() == false) { + throw new IllegalStateException("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false and " + + Node.NODE_MASTER_SETTING.getKey() + + "=false, but has index metadata: " + + indexMetaDataPaths); + } + } + + private List collectIndexSubPaths(NodePath[] nodePaths, Predicate subPathPredicate) throws IOException { + List indexSubPaths = new ArrayList<>(); + for (NodePath nodePath : nodePaths) { + Path indicesPath = nodePath.indicesPath; + if (Files.isDirectory(indicesPath)) { + try (DirectoryStream indexStream = Files.newDirectoryStream(indicesPath)) { + for (Path indexPath : indexStream) { + if (Files.isDirectory(indexPath)) { + try (Stream shardStream = Files.list(indexPath)) { + shardStream.filter(subPathPredicate) + .map(Path::toAbsolutePath) + .forEach(indexSubPaths::add); + } + } + } + } + } + } + + return indexSubPaths; + } + + private boolean isShardPath(Path path) { + return Files.isDirectory(path) + && path.getFileName().toString().chars().allMatch(Character::isDigit); + } + + private boolean isIndexMetaDataPath(Path path) { + return Files.isDirectory(path) + && path.getFileName().toString().equals(MetaDataStateFormat.STATE_DIR_NAME); + } + /** * Resolve the custom path for a index's shard. * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine diff --git a/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java b/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java index 8f1fa5017e547..0aa853a60608f 100644 --- a/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java +++ b/server/src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java @@ -57,7 +57,6 @@ public class DanglingIndicesState implements ClusterStateListener { private static final Logger logger = LogManager.getLogger(DanglingIndicesState.class); - private final Settings settings; private final NodeEnvironment nodeEnv; private final MetaStateService metaStateService; private final LocalAllocateDangledIndices allocateDangledIndices; @@ -67,11 +66,13 @@ public class DanglingIndicesState implements ClusterStateListener { @Inject public DanglingIndicesState(Settings settings, NodeEnvironment nodeEnv, MetaStateService metaStateService, LocalAllocateDangledIndices allocateDangledIndices, ClusterService clusterService) { - this.settings = settings; this.nodeEnv = nodeEnv; this.metaStateService = metaStateService; this.allocateDangledIndices = allocateDangledIndices; - clusterService.addListener(this); + if (DiscoveryNode.isDataNode(settings) + || DiscoveryNode.isMasterNode(settings)) { + clusterService.addListener(this); + } } /** @@ -136,12 +137,8 @@ Map findNewDanglingIndices(final MetaData metaData) { final List indexMetaDataList = metaStateService.loadIndicesStates(excludeIndexPathIds::contains); Map newIndices = new HashMap<>(indexMetaDataList.size()); final IndexGraveyard graveyard = metaData.indexGraveyard(); - boolean coordinatingOnly = DiscoveryNode.isDataNode(settings) == false - && DiscoveryNode.isMasterNode(settings) == false; for (IndexMetaData indexMetaData : indexMetaDataList) { - if (coordinatingOnly) { - logger.warn("[{}] dangling index ignored for non-data, non-master node", indexMetaData.getIndex()); - } else if (metaData.hasIndex(indexMetaData.getIndex().getName())) { + if (metaData.hasIndex(indexMetaData.getIndex().getName())) { logger.warn("[{}] can not be imported as a dangling index, as index with same name already exists in cluster metadata", indexMetaData.getIndex()); } else if (graveyard.containsIndex(indexMetaData.getIndex())) { diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index af5d589b1dc45..dce6e14b2a3f9 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -18,7 +18,17 @@ */ package org.elasticsearch.env; +import junit.framework.AssertionFailedError; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LoggingException; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.ErrorHandler; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.lucene.index.SegmentInfos; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.internal.io.IOUtils; import org.apache.lucene.util.LuceneTestCase; @@ -31,6 +41,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.node.Node; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; @@ -53,6 +64,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; @LuceneTestCase.SuppressFileSystems("ExtrasFS") // TODO: fix test to allow extras public class NodeEnvironmentTests extends ESTestCase { @@ -475,6 +487,153 @@ public void testExistingTempFiles() throws IOException { } } + // backported from 7.0, but in 6.x this only prints warnings. We keep the original test as is to ease further backports and ensure + // that log messages convert into exceptions. + public void testEnsureNoShardDataOrIndexMetaData6x() throws IOException, IllegalAccessException { + // Convert warn log messages into exceptions and call original test case. + Appender appender = new AbstractAppender("convertToException", null, null, false) { + @Override + public void append(LogEvent event) { + if (event.getLevel() == Level.WARN + && event.getMessage().getFormattedMessage().endsWith(", this needs to be cleaned up (will refuse to start in 7.0)")) { + throw new LoggingException(new IllegalStateException(event.getMessage().getFormattedMessage())); + } + } + }; + appender.setHandler(new ErrorHandler() { + @Override + public void error(String msg) { + } + + @Override + public void error(String msg, Throwable t) { + } + + @Override + public void error(String msg, LogEvent event, Throwable t) { + } + }); + appender.start(); + Logger nodeEnvironmentLogger = LogManager.getLogger(NodeEnvironment.class); + Loggers.addAppender(nodeEnvironmentLogger, appender); + try { + testEnsureNoShardDataOrIndexMetaData(); + } finally { + Loggers.removeAppender(nodeEnvironmentLogger, appender); + appender.stop(); + } + } + + private static T expectLoggingThrows(Class expectedType, String noExceptionMessage, ThrowingRunnable runnable) { + try { + runnable.run(); + } catch (Throwable e) { + if (e instanceof LoggingException) { + e = e.getCause(); + } + if (expectedType.isInstance(e)) { + return expectedType.cast(e); + } + AssertionFailedError assertion = new AssertionFailedError("Unexpected exception type, expected " + expectedType.getSimpleName() + " but got " + e); + assertion.initCause(e); + throw assertion; + } + throw new AssertionFailedError(noExceptionMessage); + } + + // exact 7.0 copy (except private on purpose to disable test and expectLoggingThrows used) + private void testEnsureNoShardDataOrIndexMetaData() throws IOException { + Settings settings = buildEnvSettings(Settings.EMPTY); + Index index = new Index("test", "testUUID"); + + // build settings using same path.data as original but with node.data=false and node.master=false + Settings noDataNoMasterSettings = Settings.builder() + .put(settings) + .put(Node.NODE_DATA_SETTING.getKey(), false) + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .build(); + + // test that we can create data=false and master=false with no meta information + newNodeEnvironment(noDataNoMasterSettings).close(); + + Path indexPath; + try (NodeEnvironment env = newNodeEnvironment(settings)) { + for (Path path : env.indexPaths(index)) { + Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); + } + indexPath = env.indexPaths(index)[0]; + } + + verifyFailsOnMetaData(noDataNoMasterSettings, indexPath); + + // build settings using same path.data as original but with node.data=false + Settings noDataSettings = Settings.builder() + .put(settings) + .put(Node.NODE_DATA_SETTING.getKey(), false).build(); + + String shardDataDirName = Integer.toString(randomInt(10)); + + // test that we can create data=false env with only meta information. Also create shard data for following asserts + try (NodeEnvironment env = newNodeEnvironment(noDataSettings)) { + for (Path path : env.indexPaths(index)) { + Files.createDirectories(path.resolve(shardDataDirName)); + } + } + + verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName); + + // assert that we get the stricter message on meta-data when both conditions fail + verifyFailsOnMetaData(noDataNoMasterSettings, indexPath); + + // build settings using same path.data as original but with node.master=false + Settings noMasterSettings = Settings.builder() + .put(settings) + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .build(); + + // test that we can create master=false env regardless of data. + newNodeEnvironment(noMasterSettings).close(); + + // test that we can create data=true, master=true env. Also remove state dir to leave only shard data for following asserts + try (NodeEnvironment env = newNodeEnvironment(settings)) { + for (Path path : env.indexPaths(index)) { + Files.delete(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); + } + } + + // assert that we fail on shard data even without the metadata dir. + verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName); + verifyFailsOnShardData(noDataNoMasterSettings, indexPath, shardDataDirName); + } + + private void verifyFailsOnShardData(Settings settings, Path indexPath, String shardDataDirName) { + IllegalStateException ex = expectLoggingThrows(IllegalStateException.class, + "Must fail creating NodeEnvironment on a data path that has shard data if node.data=false", + () -> newNodeEnvironment(settings).close()); + + assertThat(ex.getMessage(), + containsString(indexPath.resolve(shardDataDirName).toAbsolutePath().toString())); + assertThat(ex.getMessage(), + startsWith("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false, but has shard data")); + } + + private void verifyFailsOnMetaData(Settings settings, Path indexPath) { + IllegalStateException ex = expectLoggingThrows(IllegalStateException.class, + "Must fail creating NodeEnvironment on a data path that has index meta-data if node.data=false and node.master=false", + () -> newNodeEnvironment(settings).close()); + + assertThat(ex.getMessage(), + containsString(indexPath.resolve(MetaDataStateFormat.STATE_DIR_NAME).toAbsolutePath().toString())); + assertThat(ex.getMessage(), + startsWith("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false and " + + Node.NODE_MASTER_SETTING.getKey() + + "=false, but has index metadata")); + } + /** Converts an array of Strings to an array of Paths, adding an additional child if specified */ private Path[] stringsToPaths(String[] strings, String additional) { Path[] locations = new Path[strings.length]; diff --git a/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java b/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java index b4d4d6cf0d264..ea4aa2da08862 100644 --- a/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/DanglingIndicesStateTests.java @@ -37,7 +37,10 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class DanglingIndicesStateTests extends ESTestCase { @@ -160,7 +163,7 @@ public void testDanglingIndicesNotImportedWhenTombstonePresent() throws Exceptio } } - public void testDanglingIndicesIgnoredWhenObsolete() throws IOException { + public void testDanglingIndicesDisabledForCoordinatorOnly() throws IOException { Settings noDataNoMasterSettings = Settings.builder() .put(Node.NODE_DATA_SETTING.getKey(), false) .put(Node.NODE_MASTER_SETTING.getKey(), false) @@ -174,30 +177,23 @@ public void testDanglingIndicesIgnoredWhenObsolete() throws IOException { .put(Node.NODE_MASTER_SETTING.getKey(), false) .build(); - verifyDanglingIndicesIgnoredWhenObsolete(noDataNoMasterSettings, 0, + verifyDanglingIndicesDisabled(noDataNoMasterSettings, 0, "node.data=false and node.master=false nodes should not detect any dangling indices"); - verifyDanglingIndicesIgnoredWhenObsolete(noDataSettings, 1, + verifyDanglingIndicesDisabled(noDataSettings, 1, "node.data=false and node.master=true nodes should detect dangling indices"); - verifyDanglingIndicesIgnoredWhenObsolete(noMasterSettings, 1, + verifyDanglingIndicesDisabled(noMasterSettings, 1, "node.data=true and node.master=false nodes should detect dangling indices"); // also validated by #testDanglingIndicesDiscovery, included for completeness. - verifyDanglingIndicesIgnoredWhenObsolete(Settings.EMPTY, 1, + verifyDanglingIndicesDisabled(Settings.EMPTY, 1, "node.data=true and node.master=true nodes should detect dangling indices"); } - private void verifyDanglingIndicesIgnoredWhenObsolete(Settings settings, int expected, String reason) throws IOException { + private void verifyDanglingIndicesDisabled(Settings settings, int expected, String reason) throws IOException { try (NodeEnvironment env = newNodeEnvironment(settings)) { MetaStateService metaStateService = new MetaStateService(env, xContentRegistry()); - DanglingIndicesState danglingState = createDanglingIndicesState(env, metaStateService, settings); - - final Settings.Builder testIndexSettings = Settings.builder().put(indexSettings) - .put(IndexMetaData.SETTING_INDEX_UUID, "test1UUID"); - IndexMetaData dangledIndex = IndexMetaData.builder("test1").settings(testIndexSettings).build(); - metaStateService.writeIndex("test_write", dangledIndex); - - assertThat(reason, - danglingState.findNewDanglingIndices(MetaData.builder().build()).size(), - equalTo(expected)); + ClusterService clusterService = mock(ClusterService.class); + new DanglingIndicesState(settings, env, metaStateService, null, clusterService); + verify(clusterService, times(expected)).addListener(any()); } } From bf0b3f2cfeba69aecf619d4dea9dcbd7cf17b734 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 31 Jan 2019 08:39:19 +0100 Subject: [PATCH 3/6] Warn about left-behind data or metadata Fix checkstyle issues. --- .../java/org/elasticsearch/env/NodeEnvironmentTests.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index dce6e14b2a3f9..ba5a61880d384 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -524,7 +524,9 @@ public void error(String msg, LogEvent event, Throwable t) { } } - private static T expectLoggingThrows(Class expectedType, String noExceptionMessage, ThrowingRunnable runnable) { + private static T expectLoggingThrows(Class expectedType, + String noExceptionMessage, + ThrowingRunnable runnable) { try { runnable.run(); } catch (Throwable e) { @@ -534,7 +536,8 @@ private static T expectLoggingThrows(Class expectedType if (expectedType.isInstance(e)) { return expectedType.cast(e); } - AssertionFailedError assertion = new AssertionFailedError("Unexpected exception type, expected " + expectedType.getSimpleName() + " but got " + e); + AssertionFailedError assertion = + new AssertionFailedError("Unexpected exception type, expected " + expectedType.getSimpleName() + " but got " + e); assertion.initCause(e); throw assertion; } From cf14983a1b229c02e78d90f68bf2cc2600a1de1a Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 31 Jan 2019 14:54:45 +0100 Subject: [PATCH 4/6] Warn about left-behind data or metadata Improved messaging. --- .../src/main/java/org/elasticsearch/env/NodeEnvironment.java | 4 ++-- .../test/java/org/elasticsearch/env/NodeEnvironmentTests.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 5b8a0029c2892..c3d16468ddecd 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -319,14 +319,14 @@ public NodeEnvironment(Settings settings, Environment environment, Consumer Date: Thu, 31 Jan 2019 15:19:49 +0100 Subject: [PATCH 5/6] Warn about left-behind data or metadata Fix checkstyle issue. --- .../test/java/org/elasticsearch/env/NodeEnvironmentTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index a6b3002777a91..b2499a968d597 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -495,7 +495,8 @@ public void testEnsureNoShardDataOrIndexMetaData6x() throws IOException, Illegal @Override public void append(LogEvent event) { if (event.getLevel() == Level.WARN - && event.getMessage().getFormattedMessage().endsWith(", this should be cleaned up (will refuse to start in 7.0). Beware of data-loss.")) { + && event.getMessage().getFormattedMessage() + .endsWith(", this should be cleaned up (will refuse to start in 7.0). Beware of data-loss.")) { throw new LoggingException(new IllegalStateException(event.getMessage().getFormattedMessage())); } } From fbbb295b7606cdd63b8719c6dc369504c1d6b46c Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Fri, 1 Feb 2019 15:45:11 +0100 Subject: [PATCH 6/6] Warn about left-behind data or metadata Improved messaging and log to the deprecation logger. --- .../main/java/org/elasticsearch/env/NodeEnvironment.java | 8 ++++++-- .../java/org/elasticsearch/env/NodeEnvironmentTests.java | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index c3d16468ddecd..8a84af9fc3fcb 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -36,6 +36,7 @@ import org.apache.lucene.store.SimpleFSDirectory; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.lease.Releasable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -146,6 +147,7 @@ public String toString() { } private final Logger logger = LogManager.getLogger(NodeEnvironment.class); + private final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); private final NodePath[] nodePaths; private final Path sharedDataPath; private final Lock[] locks; @@ -319,14 +321,16 @@ public NodeEnvironment(Settings settings, Environment environment, Consumer