From 9a5204b49aa5657b8b91ef1f27ba34c2d4a190a9 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 23 May 2019 20:26:36 +0200 Subject: [PATCH 1/7] Remove "nodes/0" folder prefix from data path --- docs/reference/commands/shard-tool.asciidoc | 10 +- .../migration/migrate_8_0/node.asciidoc | 12 ++ .../elasticsearch/env/NodeEnvironment.java | 151 +++++++++++++++--- .../RemoveCorruptedShardDataCommand.java | 5 +- .../RecoveryWithUnsupportedIndicesIT.java | 17 +- .../elasticsearch/env/NodeEnvironmentIT.java | 78 +++++++++ .../env/NodeEnvironmentTests.java | 12 +- .../index/shard/NewPathForShardTests.java | 2 +- .../RemoveCorruptedShardDataCommandTests.java | 5 +- 9 files changed, 244 insertions(+), 48 deletions(-) diff --git a/docs/reference/commands/shard-tool.asciidoc b/docs/reference/commands/shard-tool.asciidoc index 6fca1355a27be..c13c8d3db6a36 100644 --- a/docs/reference/commands/shard-tool.asciidoc +++ b/docs/reference/commands/shard-tool.asciidoc @@ -51,14 +51,14 @@ $ bin/elasticsearch-shard remove-corrupted-data --index twitter --shard-id 0 Please make a complete backup of your index before using this tool. -Opening Lucene index at /var/lib/elasticsearchdata/nodes/0/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ +Opening Lucene index at /var/lib/elasticsearchdata/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ - >> Lucene index is corrupted at /var/lib/elasticsearchdata/nodes/0/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ + >> Lucene index is corrupted at /var/lib/elasticsearchdata/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ -Opening translog at /var/lib/elasticsearchdata/nodes/0/indices/P45vf_YQRhqjfwLMUvSqDw/0/translog/ +Opening translog at /var/lib/elasticsearchdata/indices/P45vf_YQRhqjfwLMUvSqDw/0/translog/ - >> Translog is clean at /var/lib/elasticsearchdata/nodes/0/indices/P45vf_YQRhqjfwLMUvSqDw/0/translog/ + >> Translog is clean at /var/lib/elasticsearchdata/indices/P45vf_YQRhqjfwLMUvSqDw/0/translog/ Corrupted Lucene index segments found - 32 documents will be lost. @@ -93,7 +93,7 @@ POST /_cluster/reroute You must accept the possibility of data loss by changing parameter `accept_data_loss` to `true`. -Deleted corrupt marker corrupted_FzTSBSuxT7i3Tls_TgwEag from /var/lib/elasticsearchdata/nodes/0/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ +Deleted corrupt marker corrupted_FzTSBSuxT7i3Tls_TgwEag from /var/lib/elasticsearchdata/indices/P45vf_YQRhqjfwLMUvSqDw/0/index/ -------------------------------------------------- diff --git a/docs/reference/migration/migrate_8_0/node.asciidoc b/docs/reference/migration/migrate_8_0/node.asciidoc index a1dcd654807e1..f3b15d086d769 100644 --- a/docs/reference/migration/migrate_8_0/node.asciidoc +++ b/docs/reference/migration/migrate_8_0/node.asciidoc @@ -14,3 +14,15 @@ The `node.max_local_storage_nodes` setting was deprecated in 7.x and has been removed in 8.0. Nodes should be run on separate data paths to ensure that each node is consistently assigned to the same data path. + +[float] +==== Change of data folder layout + +While data was previously stored in `$DATA_DIR/nodes/$nodeOrdinal`, it +has now, with the removal of the `node.max_local_storage_nodes` setting, +moved directly to `$DATA_DIR`. Upon startup, Elasticsearch will check +to see if there is data in the old location, and automatically move it +to the new location. This automatic migration only works if `$nodeOrdinal` +is 0, i.e., multiple node instances have not previously run on the same +data path, which required for `node.max_local_storage_nodes` to explicitly +be configured. diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 497c6a9e06459..19df107a32cfc 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -81,6 +81,7 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -90,9 +91,9 @@ */ public final class NodeEnvironment implements Closeable { public static class NodePath { - /* ${data.paths}/nodes/0 */ + /* ${data.paths} */ public final Path path; - /* ${data.paths}/nodes/0/indices */ + /* ${data.paths}/indices */ public final Path indicesPath; /** Cached FileStore from path */ public final FileStore fileStore; @@ -115,7 +116,7 @@ public NodePath(Path path) throws IOException { /** * Resolves the given shards directory against this NodePath - * ${data.paths}/nodes/{node.id}/indices/{index.uuid}/{shard.id} + * ${data.paths}/indices/{index.uuid}/{shard.id} */ public Path resolve(ShardId shardId) { return resolve(shardId.getIndex()).resolve(Integer.toString(shardId.id())); @@ -123,7 +124,7 @@ public Path resolve(ShardId shardId) { /** * Resolves index directory against this NodePath - * ${data.paths}/nodes/{node.id}/indices/{index.uuid} + * ${data.paths}/indices/{index.uuid} */ public Path resolve(Index index) { return resolve(index.getUUID()); @@ -170,7 +171,6 @@ public String toString() { public static final Setting ENABLE_LUCENE_SEGMENT_INFOS_TRACE_SETTING = Setting.boolSetting("node.enable_lucene_segment_infos_trace", false, Property.NodeScope); - public static final String NODES_FOLDER = "nodes"; public static final String INDICES_FOLDER = "indices"; public static final String NODE_LOCK_FILENAME = "node.lock"; @@ -179,20 +179,28 @@ public static class NodeLock implements Releasable { private final Lock[] locks; private final NodePath[] nodePaths; + + public NodeLock(final Logger logger, + final Environment environment, + final CheckedFunction pathFunction) throws IOException { + this(logger, environment, pathFunction, Function.identity()); + } + /** * Tries to acquire a node lock for a node id, throws {@code IOException} if it is unable to acquire it * @param pathFunction function to check node path before attempt of acquiring a node lock */ public NodeLock(final Logger logger, final Environment environment, - final CheckedFunction pathFunction) throws IOException { + final CheckedFunction pathFunction, + final Function subPathMapping) throws IOException { nodePaths = new NodePath[environment.dataFiles().length]; locks = new Lock[nodePaths.length]; try { final Path[] dataPaths = environment.dataFiles(); for (int dirIndex = 0; dirIndex < dataPaths.length; dirIndex++) { Path dataDir = dataPaths[dirIndex]; - Path dir = resolveNodePath(dataDir); + Path dir = subPathMapping.apply(dataDir); if (pathFunction.apply(dir) == false) { continue; } @@ -247,7 +255,7 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce sharedDataPath = environment.sharedDataFile(); for (Path path : environment.dataFiles()) { - Files.createDirectories(resolveNodePath(path)); + Files.createDirectories(path); } final NodeLock nodeLock; @@ -264,7 +272,6 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce this.locks = nodeLock.locks; this.nodePaths = nodeLock.nodePaths; - this.nodeMetaData = loadOrCreateNodeMetaData(settings, logger, nodePaths); logger.debug("using node location {}", Arrays.toString(nodePaths)); @@ -278,6 +285,10 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce ensureAtomicMoveSupported(nodePaths); } + if (upgradeLegacyNodeFolders(logger, environment, nodeLock)) { + assertCanWrite(); + } + if (DiscoveryNode.isDataNode(settings) == false) { if (DiscoveryNode.isMasterNode(settings) == false) { ensureNoIndexMetaData(nodePaths); @@ -286,6 +297,8 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce ensureNoShardData(nodePaths); } + this.nodeMetaData = loadOrCreateNodeMetaData(settings, logger, nodePaths); + success = true; } finally { if (success == false) { @@ -295,13 +308,111 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce } /** - * Resolve a specific nodes/{node.id} path for the specified path and node lock id. - * - * @param path the path - * @return the resolved path + * Upgrades all data paths that have been written to by an older ES version to the 8.0+ compatible folder layout, + * removing the "nodes/${lockId}" folder prefix */ - public static Path resolveNodePath(final Path path) { - return path.resolve(NODES_FOLDER).resolve("0"); + private static boolean upgradeLegacyNodeFolders(Logger logger, Environment environment, NodeLock nodeLock) throws IOException { + boolean upgradeNeeded = false; + + // check if we can do an auto-upgrade + for (Path path : environment.dataFiles()) { + final Path nodesFolderPath = path.resolve("nodes"); + if (Files.isDirectory(nodesFolderPath)) { + final List nodeLockIds = new ArrayList<>(); + + try (DirectoryStream stream = Files.newDirectoryStream(nodesFolderPath)) { + for (Path nodeLockIdPath : stream) { + String fileName = nodeLockIdPath.getFileName().toString(); + if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) { + int nodeLockId = Integer.parseInt(fileName); + nodeLockIds.add(nodeLockId); + } + } + } + + if (nodeLockIds.isEmpty() == false) { + upgradeNeeded = true; + + if (nodeLockIds.equals(Arrays.asList(0)) == false) { + throw new IllegalStateException("Cannot upgrade multiple path with multiple lock ids or lock id different to 0 " + + "(path: " + nodesFolderPath + " ids: " + nodeLockIds + ")"); + } + } + } + } + + if (upgradeNeeded == false) { + logger.trace("data folder upgrade not required"); + return false; + } + + logger.info("upgrading legacy data folders: {}", Arrays.toString(environment.dataFiles())); + + // acquire locks on legacy path for duration of upgrade (to ensure there is no older ES version running on this path) + final NodeLock legacyNodeLock; + try { + legacyNodeLock = new NodeLock(logger, environment, dir -> true, path -> path.resolve("nodes").resolve("0")); + } catch (IOException e) { + final String message = String.format( + Locale.ROOT, + "failed to obtain legacy node locks, tried %s;" + + " maybe these locations are not writable or multiple nodes were started on the same data path?", + Arrays.toString(environment.dataFiles())); + throw new IllegalStateException(message, e); + } + + // move contents from legacy path to new path + assert nodeLock.getNodePaths().length == legacyNodeLock.getNodePaths().length; + try { + for (int i = 0; i < legacyNodeLock.getNodePaths().length; i++) { + final NodePath legacyNodePath = legacyNodeLock.getNodePaths()[i]; + final NodePath nodePath = nodeLock.getNodePaths()[i]; + + // determine folders to move and check that there are no extra files/folders + final Set folderNames = new HashSet<>(); + + try (DirectoryStream stream = Files.newDirectoryStream(legacyNodePath.path)) { + for (Path subFolderPath : stream) { + final String fileName = subFolderPath.getFileName().toString(); + if (FileSystemUtils.isDesktopServicesStore(subFolderPath)) { + // ignore + } else if (FileSystemUtils.isAccessibleDirectory(subFolderPath, logger)) { + if (fileName.equals(INDICES_FOLDER) == false && // indices folder + fileName.equals(MetaDataStateFormat.STATE_DIR_NAME) == false) { // global metadata & node state folder + throw new IllegalStateException("unexpected folder encountered during data folder upgrade: " + + subFolderPath); + } + final Path targetSubFolderPath = nodePath.path.resolve(fileName); + if (Files.exists(targetSubFolderPath)) { + throw new IllegalStateException("target folder already exists during data folder upgrade: " + + targetSubFolderPath); + } + folderNames.add(fileName); + } else if (fileName.equals(NODE_LOCK_FILENAME) == false && + fileName.equals(TEMP_FILE_NAME) == false) { + throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " + + subFolderPath); + } + } + } + + // now do the actual move + for (String folderName : folderNames) { + final Path sourceSubFolderPath = legacyNodePath.path.resolve(folderName); + final Path targetSubFolderPath = nodePath.path.resolve(folderName); + Files.move(sourceSubFolderPath, targetSubFolderPath, StandardCopyOption.ATOMIC_MOVE); + logger.info("data folder upgrade: moved from [{}] to [{}]", sourceSubFolderPath, targetSubFolderPath); + } + IOUtils.fsync(nodePath.path, true); + } + } finally { + legacyNodeLock.close(); + } + + // upgrade successfully completed, remove legacy nodes folders + IOUtils.rm(Stream.of(environment.dataFiles()).map(path -> path.resolve("nodes")).toArray(Path[]::new)); + + return true; } private void maybeLogPathDetails() throws IOException { @@ -801,14 +912,14 @@ public Path[] availableShardPaths(ShardId shardId) { } /** - * Returns all folder names in ${data.paths}/nodes/{node.id}/indices folder + * Returns all folder names in ${data.paths}/indices folder */ public Set availableIndexFolders() throws IOException { return availableIndexFolders(p -> false); } /** - * Returns folder names in ${data.paths}/nodes/{node.id}/indices folder that don't match the given predicate. + * Returns folder names in ${data.paths}/indices folder that don't match the given predicate. * @param excludeIndexPathIdsPredicate folder names to exclude */ public Set availableIndexFolders(Predicate excludeIndexPathIdsPredicate) throws IOException { @@ -825,7 +936,7 @@ public Set availableIndexFolders(Predicate excludeIndexPathIdsPr } /** - * Return all directory names in the nodes/{node.id}/indices directory for the given node path. + * Return all directory names in the indices directory for the given node path. * * @param nodePath the path * @return all directories that could be indices for the given node path. @@ -836,7 +947,7 @@ public Set availableIndexFoldersForPath(final NodePath nodePath) throws } /** - * Return directory names in the nodes/{node.id}/indices directory for the given node path that don't match the given predicate. + * Return directory names in the indices directory for the given node path that don't match the given predicate. * * @param nodePath the path * @param excludeIndexPathIdsPredicate folder names to exclude @@ -865,7 +976,7 @@ public Set availableIndexFoldersForPath(final NodePath nodePath, Predica } /** - * Resolves all existing paths to indexFolderName in ${data.paths}/nodes/{node.id}/indices + * Resolves all existing paths to indexFolderName in ${data.paths}/indices */ public Path[] resolveIndexFolder(String indexFolderName) { if (nodePaths == null || locks == null) { diff --git a/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java b/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java index 16db596515b4c..5fc3ba57980bf 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java +++ b/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java @@ -140,17 +140,14 @@ protected void findAndProcessShardPath(OptionSet options, Environment environmen IndexMetaData.FORMAT.loadLatestState(logger, namedXContentRegistry, shardParent); final String shardIdFileName = path.getFileName().toString(); - final String nodeIdFileName = shardParentParent.getParent().getFileName().toString(); if (Files.isDirectory(path) && shardIdFileName.chars().allMatch(Character::isDigit) // SHARD-ID path element check && NodeEnvironment.INDICES_FOLDER.equals(shardParentParent.getFileName().toString()) // `indices` check - && nodeIdFileName.chars().allMatch(Character::isDigit) // NODE-ID check - && NodeEnvironment.NODES_FOLDER.equals(shardParentParent.getParent().getParent().getFileName().toString()) // `nodes` check ) { shardId = Integer.parseInt(shardIdFileName); indexName = indexMetaData.getIndex().getName(); } else { throw new ElasticsearchException("Unable to resolve shard id. Wrong folder structure at [ " + path.toString() - + " ], expected .../nodes/[NODE-ID]/indices/[INDEX-UUID]/[SHARD-ID]"); + + " ], expected .../indices/[INDEX-UUID]/[SHARD-ID]"); } } else { // otherwise resolve shardPath based on the index name and shard id diff --git a/server/src/test/java/org/elasticsearch/bwcompat/RecoveryWithUnsupportedIndicesIT.java b/server/src/test/java/org/elasticsearch/bwcompat/RecoveryWithUnsupportedIndicesIT.java index 53efeb393e4b4..720439768fabc 100644 --- a/server/src/test/java/org/elasticsearch/bwcompat/RecoveryWithUnsupportedIndicesIT.java +++ b/server/src/test/java/org/elasticsearch/bwcompat/RecoveryWithUnsupportedIndicesIT.java @@ -18,6 +18,12 @@ */ package org.elasticsearch.bwcompat; +import org.apache.lucene.util.LuceneTestCase; +import org.apache.lucene.util.TestUtil; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.test.ESIntegTestCase; + import java.io.IOException; import java.io.InputStream; import java.nio.file.DirectoryStream; @@ -26,13 +32,6 @@ import java.util.ArrayList; import java.util.List; -import org.apache.lucene.util.LuceneTestCase; -import org.apache.lucene.util.TestUtil; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.Environment; -import org.elasticsearch.env.NodeEnvironment; -import org.elasticsearch.test.ESIntegTestCase; - import static org.hamcrest.Matchers.containsString; @LuceneTestCase.SuppressCodecs("*") @@ -69,8 +68,8 @@ protected Settings prepareBackwardsDataDir(Path backwardsIndex) throws IOExcepti } throw new IllegalStateException(builder.toString()); } - Path src = list[0].resolve(NodeEnvironment.NODES_FOLDER); - Path dest = dataDir.resolve(NodeEnvironment.NODES_FOLDER); + Path src = list[0].resolve("nodes"); + Path dest = dataDir.resolve("nodes"); assertTrue(Files.exists(src)); Files.move(src, dest); assertFalse(Files.exists(src)); diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index 74de578426f2c..c5d7dd5bb61e1 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -21,13 +21,25 @@ import org.elasticsearch.Version; import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.node.Node; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalTestCluster; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.List; +import java.util.stream.Collectors; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; @@ -123,4 +135,70 @@ public void testFailsToStartIfUpgradedTooFar() { assertThat(illegalStateException.getMessage(), allOf(startsWith("cannot upgrade a node from version ["), endsWith("] directly to version [" + Version.CURRENT + "]"))); } + + public void testUpgradeDataFolder() throws IOException, InterruptedException { + String node = internalCluster().startNode(); + prepareCreate("test").get(); + indexRandom(true, client().prepareIndex("test", "type1", "1").setSource("{}", XContentType.JSON)); + String nodeId = client().admin().cluster().prepareState().get().getState().nodes().getMasterNodeId(); + + final Settings dataPathSettings = internalCluster().dataPathSettings(node); + internalCluster().stopRandomDataNode(); + + // simulate older data path layout by moving data under "nodes/0" folder + final List dataPaths = Environment.PATH_DATA_SETTING.get(dataPathSettings) + .stream().map(PathUtils::get).collect(Collectors.toList()); + dataPaths.forEach(path -> { + final Path targetPath = path.resolve("nodes").resolve("0"); + try { + Files.createDirectories(targetPath); + + try (DirectoryStream stream = Files.newDirectoryStream(path)) { + for (Path subPath : stream) { + String fileName = subPath.getFileName().toString(); + Path targetSubPath = targetPath.resolve(fileName); + if (fileName.equals("nodes") == false) { + Files.move(subPath, targetSubPath, StandardCopyOption.ATOMIC_MOVE); + } + } + } + IOUtils.fsync(targetPath, true); + IOUtils.fsync(path, true); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + + dataPaths.forEach(path -> assertTrue(Files.exists(path.resolve("nodes")))); + + // create extra file/folder, and check that upgrade fails + if (dataPaths.isEmpty() == false) { + final Path badFile = Files.createTempFile(randomFrom(dataPaths).resolve("nodes").resolve("0"), "bad", "file"); + IllegalStateException ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); + assertThat(ise.getMessage(), containsString("unexpected file/folder encountered during data folder upgrade")); + Files.delete(badFile); + + final Path badFolder = Files.createDirectories(randomFrom(dataPaths).resolve("nodes").resolve("0").resolve("bad-folder")); + ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); + assertThat(ise.getMessage(), containsString("unexpected folder encountered during data folder upgrad")); + Files.delete(badFolder); + + final Path conflictingFolder = randomFrom(dataPaths).resolve("indices"); + if (Files.exists(conflictingFolder) == false) { + Files.createDirectories(conflictingFolder); + ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); + assertThat(ise.getMessage(), containsString("target folder already exists during data folder upgrade")); + Files.delete(conflictingFolder); + } + } + + // check that upgrade works + dataPaths.forEach(path -> assertTrue(Files.exists(path.resolve("nodes")))); + internalCluster().startNode(dataPathSettings); + dataPaths.forEach(path -> assertFalse(Files.exists(path.resolve("nodes")))); + assertEquals(nodeId, client().admin().cluster().prepareState().get().getState().nodes().getMasterNodeId()); + assertTrue(client().admin().indices().prepareExists("test").get().isExists()); + ensureYellow("test"); + assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L); + } } diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index f21b55b9aee8f..5bb1152bcbe45 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -373,10 +373,10 @@ public void testCustomDataPaths() throws Exception { assertThat("shard paths with a custom data_path should contain only regular paths", env.availableShardPaths(sid), - equalTo(stringsToPaths(dataPaths, "nodes/0/indices/" + index.getUUID() + "/0"))); + equalTo(stringsToPaths(dataPaths, "indices/" + index.getUUID() + "/0"))); assertThat("index paths uses the regular template", - env.indexPaths(index), equalTo(stringsToPaths(dataPaths, "nodes/0/indices/" + index.getUUID()))); + env.indexPaths(index), equalTo(stringsToPaths(dataPaths, "indices/" + index.getUUID()))); IndexSettings s3 = new IndexSettings(s2.getIndexMetaData(), Settings.builder().build()); @@ -385,10 +385,10 @@ public void testCustomDataPaths() throws Exception { assertThat("shard paths with a custom data_path should contain only regular paths", env.availableShardPaths(sid), - equalTo(stringsToPaths(dataPaths, "nodes/0/indices/" + index.getUUID() + "/0"))); + equalTo(stringsToPaths(dataPaths, "indices/" + index.getUUID() + "/0"))); assertThat("index paths uses the regular template", - env.indexPaths(index), equalTo(stringsToPaths(dataPaths, "nodes/0/indices/" + index.getUUID()))); + env.indexPaths(index), equalTo(stringsToPaths(dataPaths, "indices/" + index.getUUID()))); env.close(); } @@ -418,7 +418,7 @@ public void testExistingTempFiles() throws IOException { String[] paths = tmpPaths(); // simulate some previous left over temp files for (String path : randomSubsetOf(randomIntBetween(1, paths.length), paths)) { - final Path nodePath = NodeEnvironment.resolveNodePath(PathUtils.get(path)); + final Path nodePath = PathUtils.get(path); Files.createDirectories(nodePath); Files.createFile(nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME)); if (randomBoolean()) { @@ -433,7 +433,7 @@ public void testExistingTempFiles() throws IOException { // check we clean up for (String path: paths) { - final Path nodePath = NodeEnvironment.resolveNodePath(PathUtils.get(path)); + final Path nodePath = PathUtils.get(path); final Path tempFile = nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME); assertFalse(tempFile + " should have been cleaned", Files.exists(tempFile)); final Path srcTempFile = nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME + ".src"); diff --git a/server/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java index 4e6e3036f4c40..73ae826d7211f 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/NewPathForShardTests.java @@ -90,7 +90,7 @@ static class MockUsableSpaceFileSystemProvider extends FilterFileSystemProvider @Override public FileStore getFileStore(Path path) throws IOException { - if (path.toString().contains(aPathPart)) { + if (path.toString().contains(aPathPart) || (path.toString() + path.getFileSystem().getSeparator()).contains(aPathPart)) { return aFileStore; } else { return bFileStore; diff --git a/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandTests.java b/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandTests.java index c9a7b236d9c8f..cb84f0233df33 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandTests.java @@ -91,8 +91,7 @@ public void setup() throws IOException { .putList(Environment.PATH_DATA_SETTING.getKey(), dataDir.toAbsolutePath().toString()).build()); // create same directory structure as prod does - final Path path = NodeEnvironment.resolveNodePath(dataDir); - Files.createDirectories(path); + Files.createDirectories(dataDir); settings = Settings.builder() .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) @@ -100,7 +99,7 @@ public void setup() throws IOException { .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) .build(); - final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(path); + final NodeEnvironment.NodePath nodePath = new NodeEnvironment.NodePath(dataDir); shardPath = new ShardPath(false, nodePath.resolve(shardId), nodePath.resolve(shardId), shardId); final IndexMetaData.Builder metaData = IndexMetaData.builder(routing.getIndexName()) .settings(settings) From b55b26afa1f302af2c0728c2fb6858042d8ab1a0 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Fri, 24 May 2019 09:22:43 +0200 Subject: [PATCH 2/7] fix tests --- .../org/elasticsearch/env/NodeEnvironmentEvilTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java index 44d3c2a88a55b..49e30ac4b5ed3 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java @@ -51,10 +51,11 @@ public void testMissingWritePermission() throws IOException { Settings build = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath().toString()) .putList(Environment.PATH_DATA_SETTING.getKey(), tempPaths).build(); - IOException exception = expectThrows(IOException.class, () -> { + IllegalStateException exception = expectThrows(IllegalStateException.class, () -> { new NodeEnvironment(build, TestEnvironment.newEnvironment(build)); }); - assertTrue(exception.getMessage(), exception.getMessage().startsWith(path.toString())); + assertTrue(exception.getCause().getCause().getMessage(), + exception.getCause().getCause().getMessage().startsWith(path.toString())); } } @@ -62,7 +63,7 @@ public void testMissingWritePermissionOnIndex() throws IOException { assumeTrue("posix filesystem", isPosix); final String[] tempPaths = tmpPaths(); Path path = PathUtils.get(randomFrom(tempPaths)); - Path fooIndex = path.resolve("nodes").resolve("0").resolve(NodeEnvironment.INDICES_FOLDER) + Path fooIndex = path.resolve(NodeEnvironment.INDICES_FOLDER) .resolve("foo"); Files.createDirectories(fooIndex); try (PosixPermissionsResetter attr = new PosixPermissionsResetter(fooIndex)) { @@ -82,7 +83,7 @@ public void testMissingWritePermissionOnShard() throws IOException { assumeTrue("posix filesystem", isPosix); final String[] tempPaths = tmpPaths(); Path path = PathUtils.get(randomFrom(tempPaths)); - Path fooIndex = path.resolve("nodes").resolve("0").resolve(NodeEnvironment.INDICES_FOLDER) + Path fooIndex = path.resolve(NodeEnvironment.INDICES_FOLDER) .resolve("foo"); Path fooShard = fooIndex.resolve("0"); Path fooShardIndex = fooShard.resolve("index"); From e22c2ee519ba74c0e67bfc7437ff9e2d23d4cb4f Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 27 May 2019 20:03:06 +0200 Subject: [PATCH 3/7] review comments --- .../migration/migrate_8_0/node.asciidoc | 7 +++- .../elasticsearch/env/NodeEnvironment.java | 40 +++++++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/node.asciidoc b/docs/reference/migration/migrate_8_0/node.asciidoc index f3b15d086d769..3fffb7706333a 100644 --- a/docs/reference/migration/migrate_8_0/node.asciidoc +++ b/docs/reference/migration/migrate_8_0/node.asciidoc @@ -25,4 +25,9 @@ to see if there is data in the old location, and automatically move it to the new location. This automatic migration only works if `$nodeOrdinal` is 0, i.e., multiple node instances have not previously run on the same data path, which required for `node.max_local_storage_nodes` to explicitly -be configured. +be configured. In case where the automatic migration cannot be done due to +ambiguity of `$nodeOrdinal` subfolders, the data path (i.e. `path.data` +setting) can either be adjusted for each node instance to one of the +`$nodeOrdinal` subfolders, or preferably the contents of the `$nodeOrdinal` +subfolders can be manually moved into separate new folders, with `path.data` +then set to one of these folders for each node instance. diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 19df107a32cfc..671eb8861c795 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -35,6 +35,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.CheckedRunnable; import org.elasticsearch.common.Randomness; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.UUIDs; @@ -45,6 +46,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.gateway.MetaDataStateFormat; @@ -285,7 +287,7 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce ensureAtomicMoveSupported(nodePaths); } - if (upgradeLegacyNodeFolders(logger, environment, nodeLock)) { + if (upgradeLegacyNodeFolders(logger, settings, environment, nodeLock)) { assertCanWrite(); } @@ -311,7 +313,8 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce * Upgrades all data paths that have been written to by an older ES version to the 8.0+ compatible folder layout, * removing the "nodes/${lockId}" folder prefix */ - private static boolean upgradeLegacyNodeFolders(Logger logger, Environment environment, NodeLock nodeLock) throws IOException { + private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings, Environment environment, + NodeLock nodeLock) throws IOException { boolean upgradeNeeded = false; // check if we can do an auto-upgrade @@ -334,8 +337,10 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Environment envir upgradeNeeded = true; if (nodeLockIds.equals(Arrays.asList(0)) == false) { - throw new IllegalStateException("Cannot upgrade multiple path with multiple lock ids or lock id different to 0 " + - "(path: " + nodesFolderPath + " ids: " + nodeLockIds + ")"); + throw new IllegalStateException("data path " + nodesFolderPath + " cannot be upgraded automatically because it " + + "contains data from nodes with ordinals " + nodeLockIds + ", due to previous use of the now obsolete " + + "[node.max_local_storage_nodes] setting. Please check the breaking changes docs for the current version of " + + "Elasticsearch to find an upgrade path"); } } } @@ -364,6 +369,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Environment envir // move contents from legacy path to new path assert nodeLock.getNodePaths().length == legacyNodeLock.getNodePaths().length; try { + final List> upgradeActions = new ArrayList<>(); for (int i = 0; i < legacyNodeLock.getNodePaths().length; i++) { final NodePath legacyNodePath = legacyNodeLock.getNodePaths()[i]; final NodePath nodePath = nodeLock.getNodePaths()[i]; @@ -396,14 +402,24 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Environment envir } } - // now do the actual move - for (String folderName : folderNames) { - final Path sourceSubFolderPath = legacyNodePath.path.resolve(folderName); - final Path targetSubFolderPath = nodePath.path.resolve(folderName); - Files.move(sourceSubFolderPath, targetSubFolderPath, StandardCopyOption.ATOMIC_MOVE); - logger.info("data folder upgrade: moved from [{}] to [{}]", sourceSubFolderPath, targetSubFolderPath); - } - IOUtils.fsync(nodePath.path, true); + assert Sets.difference(Sets.newHashSet(INDICES_FOLDER, MetaDataStateFormat.STATE_DIR_NAME), folderNames).isEmpty() : + "expected indices and/or state dir folder but was " + folderNames; + + upgradeActions.add(() -> { + for (String folderName : folderNames) { + final Path sourceSubFolderPath = legacyNodePath.path.resolve(folderName); + final Path targetSubFolderPath = nodePath.path.resolve(folderName); + Files.move(sourceSubFolderPath, targetSubFolderPath, StandardCopyOption.ATOMIC_MOVE); + logger.info("data folder upgrade: moved from [{}] to [{}]", sourceSubFolderPath, targetSubFolderPath); + } + IOUtils.fsync(nodePath.path, true); + }); + } + // now do the actual upgrade. start by upgrading the node metadata file before moving anything, since a downgrade in an + // intermediate state would be pretty disastrous + loadOrCreateNodeMetaData(settings, logger, legacyNodeLock.getNodePaths()); + for (CheckedRunnable upgradeAction : upgradeActions) { + upgradeAction.run(); } } finally { legacyNodeLock.close(); From 74d6bdbe776ab44eb8502291ef0a5dfd01e6a031 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 28 May 2019 14:15:42 +0200 Subject: [PATCH 4/7] docs --- .../migration/migrate_8_0/node.asciidoc | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/node.asciidoc b/docs/reference/migration/migrate_8_0/node.asciidoc index 3fffb7706333a..b1187e88b5d90 100644 --- a/docs/reference/migration/migrate_8_0/node.asciidoc +++ b/docs/reference/migration/migrate_8_0/node.asciidoc @@ -18,16 +18,21 @@ to ensure that each node is consistently assigned to the same data path. [float] ==== Change of data folder layout -While data was previously stored in `$DATA_DIR/nodes/$nodeOrdinal`, it -has now, with the removal of the `node.max_local_storage_nodes` setting, -moved directly to `$DATA_DIR`. Upon startup, Elasticsearch will check -to see if there is data in the old location, and automatically move it -to the new location. This automatic migration only works if `$nodeOrdinal` -is 0, i.e., multiple node instances have not previously run on the same -data path, which required for `node.max_local_storage_nodes` to explicitly -be configured. In case where the automatic migration cannot be done due to -ambiguity of `$nodeOrdinal` subfolders, the data path (i.e. `path.data` -setting) can either be adjusted for each node instance to one of the -`$nodeOrdinal` subfolders, or preferably the contents of the `$nodeOrdinal` -subfolders can be manually moved into separate new folders, with `path.data` -then set to one of these folders for each node instance. +Each node's data is now stored directly in the data directory set by the +`path.data` setting, rather than in `${path.data}/nodes/0`, because the removal +of the `node.max_local_storage_nodes` setting means that nodes may no longer +share a data path. At startup, Elasticsearch will automatically migrate the data +path to the new layout. This automatic migration will not proceed if the data +path contains data for more than one node. You should move to a configuration in +which each node has its own data path before upgrading. + +If you try to upgrade a configuration in which there is data for more than one +node in a data path then the automatic migration will fail and Elasticsearch +will refuse to start. To resolve this you will need to perform the migration +manually. The data for the extra nodes are stored in folders named +`${path.data}/nodes/1`, `${path.data}/nodes/2` and so on, and you should move +each of these folders to an appropriate location and then configure the +corresponding node to use this location for its data path. If your nodes each +have more than one data path in their `path.data` settings then you should move +all the corresponding subfolders in parallel. Each node uses the same subfolder +(e.g. `nodes/2`) across all its data paths. \ No newline at end of file From b139f14bb8514dfc0a661eda3b3e623f402bdc07 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 28 May 2019 14:23:24 +0200 Subject: [PATCH 5/7] Fail if any other file/dir in nodes dir --- .../org/elasticsearch/env/NodeEnvironment.java | 3 +++ .../org/elasticsearch/env/NodeEnvironmentIT.java | 16 ++++++++++++---- 2 files changed, 15 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 745820348a4ec..73ddbce5f82df 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -329,6 +329,9 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) { int nodeLockId = Integer.parseInt(fileName); nodeLockIds.add(nodeLockId); + } else if (FileSystemUtils.isDesktopServicesStore(nodeLockIdPath) == false) { + throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " + + nodeLockIdPath); } } } diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index c5d7dd5bb61e1..f49b3b45aad53 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -162,8 +162,6 @@ public void testUpgradeDataFolder() throws IOException, InterruptedException { } } } - IOUtils.fsync(targetPath, true); - IOUtils.fsync(path, true); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -173,14 +171,24 @@ public void testUpgradeDataFolder() throws IOException, InterruptedException { // create extra file/folder, and check that upgrade fails if (dataPaths.isEmpty() == false) { - final Path badFile = Files.createTempFile(randomFrom(dataPaths).resolve("nodes").resolve("0"), "bad", "file"); + final Path badFileInNodesDir = Files.createTempFile(randomFrom(dataPaths).resolve("nodes"), "bad", "file"); IllegalStateException ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); assertThat(ise.getMessage(), containsString("unexpected file/folder encountered during data folder upgrade")); + Files.delete(badFileInNodesDir); + + final Path badFolderInNodesDir = Files.createDirectories(randomFrom(dataPaths).resolve("nodes").resolve("bad-folder")); + ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); + assertThat(ise.getMessage(), containsString("unexpected file/folder encountered during data folder upgrade")); + Files.delete(badFolderInNodesDir); + + final Path badFile = Files.createTempFile(randomFrom(dataPaths).resolve("nodes").resolve("0"), "bad", "file"); + ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); + assertThat(ise.getMessage(), containsString("unexpected file/folder encountered during data folder upgrade")); Files.delete(badFile); final Path badFolder = Files.createDirectories(randomFrom(dataPaths).resolve("nodes").resolve("0").resolve("bad-folder")); ise = expectThrows(IllegalStateException.class, () -> internalCluster().startNode(dataPathSettings)); - assertThat(ise.getMessage(), containsString("unexpected folder encountered during data folder upgrad")); + assertThat(ise.getMessage(), containsString("unexpected folder encountered during data folder upgrade")); Files.delete(badFolder); final Path conflictingFolder = randomFrom(dataPaths).resolve("indices"); From 37695153fc3cc35983c24e5fe98a68de466e42e0 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 28 May 2019 14:52:43 +0200 Subject: [PATCH 6/7] sigh --- server/src/main/java/org/elasticsearch/env/NodeEnvironment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 73ddbce5f82df..75f39e70cfc7b 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -405,7 +405,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings } } - assert Sets.difference(Sets.newHashSet(INDICES_FOLDER, MetaDataStateFormat.STATE_DIR_NAME), folderNames).isEmpty() : + assert Sets.difference(folderNames, Sets.newHashSet(INDICES_FOLDER, MetaDataStateFormat.STATE_DIR_NAME)).isEmpty() : "expected indices and/or state dir folder but was " + folderNames; upgradeActions.add(() -> { From 102d21fff41c9477dc9109921cd2c87fdaf78f44 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 28 May 2019 16:05:44 +0200 Subject: [PATCH 7/7] checkstyle --- .../src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index f49b3b45aad53..4d1848428e5a7 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.node.Node; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalTestCluster;