From a14201d654ac094ce0f1c53db792b11c360954da Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Wed, 23 Jan 2019 09:02:35 +0100 Subject: [PATCH] Fail start on invalid index metadata Node started with node.data=false and node.master=false can no longer start if they have index metadata. This avoids resurrecting old indexes into the cluster and ensures metadata is cleaned out before re-purposing a node that was previously master or data node. Closes #27073 --- .../elasticsearch/env/NodeEnvironment.java | 44 +++++++++--- .../elasticsearch/env/NodeEnvironmentIT.java | 28 +++++++- .../env/NodeEnvironmentTests.java | 67 ++++++++++++++++--- 3 files changed, 119 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 2f676eb846770..397d1ee1763dd 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -312,6 +312,10 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce } if (DiscoveryNode.isDataNode(settings) == false) { + if (DiscoveryNode.isMasterNode(settings) == false) { + ensureNoIndexMetaData(nodePaths); + } + ensureNoShardData(nodePaths); } @@ -1037,7 +1041,29 @@ private static void ensureAtomicMoveSupported(final NodePath[] nodePaths) throws } private void ensureNoShardData(final NodePath[] nodePaths) throws IOException { - List shardDataPaths = new ArrayList<>(); + List 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)) { @@ -1045,9 +1071,9 @@ private void ensureNoShardData(final NodePath[] nodePaths) throws IOException { for (Path indexPath : indexStream) { if (Files.isDirectory(indexPath)) { try (Stream shardStream = Files.list(indexPath)) { - shardStream.filter(this::isShardPath) + shardStream.filter(subPathPredicate) .map(Path::toAbsolutePath) - .forEach(shardDataPaths::add); + .forEach(indexSubPaths::add); } } } @@ -1055,12 +1081,7 @@ private void ensureNoShardData(final NodePath[] nodePaths) throws IOException { } } - if (shardDataPaths.isEmpty() == false) { - throw new IllegalStateException("Node is started with " - + Node.NODE_DATA_SETTING.getKey() - + "=false, but has shard data: " - + shardDataPaths); - } + return indexSubPaths; } private boolean isShardPath(Path path) { @@ -1068,6 +1089,11 @@ private boolean isShardPath(Path 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/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index bdca86858701f..36f75c79a1792 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -42,15 +42,37 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { ).get(); final String indexUUID = resolveIndex(indexName).getUUID(); + logger.info("--> restarting the node with node.data=false and node.master=false"); + IllegalStateException ex = expectThrows(IllegalStateException.class, + "Node started with node.data=false and node.master=false while having existing index metadata must fail", + () -> + internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) { + return Settings.builder() + .put(Node.NODE_DATA_SETTING.getKey(), false) + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .build(); + } + })); + assertThat(ex.getMessage(), containsString(indexUUID)); + 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")); + + // client() also starts the node logger.info("--> indexing a simple document"); client().prepareIndex(indexName, "type1", "1").setSource("field1", "value1").get(); - logger.info("--> restarting the node with node.data=true"); + logger.info("--> restarting the node with node.data=true and node.master=true"); internalCluster().restartRandomDataNode(); logger.info("--> restarting the node with node.data=false"); - IllegalStateException ex = expectThrows(IllegalStateException.class, - "Node started with node.data=false and existing shard data must fail", + ex = expectThrows(IllegalStateException.class, + "Node started with node.data=false while having existing shard data must fail", () -> internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() { @Override diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 2771bc9f243ac..a667514fa7ef2 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -472,45 +472,96 @@ public void testExistingTempFiles() throws IOException { } } - public void testEnsureNoShardData() throws IOException { + public 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)); - Path shardPath; - // test that we can create data=false env with only meta information + // 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)); } - shardPath = env.indexPaths(index)[0]; } + 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 = expectThrows(IllegalStateException.class, "Must fail creating NodeEnvironment on a data path that has shard data if node.data=false", - () -> newNodeEnvironment(noDataSettings).close()); + () -> newNodeEnvironment(settings).close()); assertThat(ex.getMessage(), - containsString(shardPath.resolve(shardDataDirName).toAbsolutePath().toString())); + containsString(indexPath.resolve(shardDataDirName).toAbsolutePath().toString())); assertThat(ex.getMessage(), startsWith("Node is started with " + Node.NODE_DATA_SETTING.getKey() + "=false, but has shard data")); + } - // test that we can create data=true env - newNodeEnvironment(settings).close(); + private void verifyFailsOnMetaData(Settings settings, Path indexPath) { + IllegalStateException ex = expectThrows(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 */