From 47095c1debc241057305a9177e42c8523220f41b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 4 May 2021 10:49:43 -0700 Subject: [PATCH 1/2] Remove multiple paths from ShardPath --- .../RemoveCorruptedShardDataCommand.java | 2 +- .../elasticsearch/index/shard/ShardPath.java | 43 +++++++------------ 2 files changed, 16 insertions(+), 29 deletions(-) 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 372a3f5641178..50c1e65f08c39 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java +++ b/server/src/main/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommand.java @@ -160,7 +160,7 @@ protected void findAndProcessShardPath(OptionSet options, Environment environmen .resolve(Integer.toString(shId.id())); if (Files.exists(shardPathLocation)) { final ShardPath shardPath = ShardPath.loadShardPath(logger, shId, indexSettings.customDataPath(), - new Path[]{shardPathLocation}, dataPath); + shardPathLocation, dataPath); if (shardPath != null) { consumer.accept(shardPath); } diff --git a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java index ea8203556ccb2..6d5a5d6290161 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java +++ b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java @@ -107,7 +107,7 @@ public static ShardPath loadShardPath(Logger logger, NodeEnvironment env, ShardId shardId, String customDataPath) throws IOException { final Path shardPath = env.availableShardPath(shardId); final Path sharedDataPath = env.sharedDataPath(); - return loadShardPath(logger, shardId, customDataPath, new Path[] { shardPath }, sharedDataPath); + return loadShardPath(logger, shardId, customDataPath, shardPath, sharedDataPath); } /** @@ -115,43 +115,30 @@ public static ShardPath loadShardPath(Logger logger, NodeEnvironment env, * directories with a valid shard state exist the one with the highest version will be used. * Note: this method resolves custom data locations for the shard. */ - public static ShardPath loadShardPath(Logger logger, ShardId shardId, String customDataPath, Path[] availableShardPaths, + public static ShardPath loadShardPath(Logger logger, ShardId shardId, String customDataPath, Path availableShardPath, Path sharedDataPath) throws IOException { final String indexUUID = shardId.getIndex().getUUID(); - Path loadedPath = null; - for (Path path : availableShardPaths) { - // EMPTY is safe here because we never call namedObject - ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, path); - if (load != null) { - if (load.indexUUID.equals(indexUUID) == false && IndexMetadata.INDEX_UUID_NA_VALUE.equals(load.indexUUID) == false) { - logger.warn("{} found shard on path: [{}] with a different index UUID - this " - + "shard seems to be leftover from a different index with the same name. " - + "Remove the leftover shard in order to reuse the path with the current index", shardId, path); - throw new IllegalStateException(shardId + " index UUID in shard state was: " + load.indexUUID - + " expected: " + indexUUID + " on shard path: " + path); - } - if (loadedPath == null) { - loadedPath = path; - } else{ - throw new IllegalStateException(shardId + " more than one shard state found"); - } + // EMPTY is safe here because we never call namedObject + ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, availableShardPath); + if (load != null) { + if (load.indexUUID.equals(indexUUID) == false && IndexMetadata.INDEX_UUID_NA_VALUE.equals(load.indexUUID) == false) { + logger.warn("{} found shard on path: [{}] with a different index UUID - this " + + "shard seems to be leftover from a different index with the same name. " + + "Remove the leftover shard in order to reuse the path with the current index", shardId, availableShardPath); + throw new IllegalStateException(shardId + " index UUID in shard state was: " + load.indexUUID + + " expected: " + indexUUID + " on shard path: " + availableShardPath); } - - } - if (loadedPath == null) { - return null; - } else { final Path dataPath; - final Path statePath = loadedPath; final boolean hasCustomDataPath = Strings.isNotEmpty(customDataPath); if (hasCustomDataPath) { dataPath = NodeEnvironment.resolveCustomLocation(customDataPath, shardId, sharedDataPath); } else { - dataPath = statePath; + dataPath = availableShardPath; } - logger.debug("{} loaded data path [{}], state path [{}]", shardId, dataPath, statePath); - return new ShardPath(hasCustomDataPath, dataPath, statePath, shardId); + logger.debug("{} loaded data path [{}], state path [{}]", shardId, dataPath, availableShardPath); + return new ShardPath(hasCustomDataPath, dataPath, availableShardPath, shardId); } + return null; } /** From 16975ec28836352a7c5009cd68270be894b42513 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 5 May 2021 11:45:57 -0700 Subject: [PATCH 2/2] address feedback --- .../elasticsearch/index/shard/ShardPath.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java index 6d5a5d6290161..12172d09fe0af 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java +++ b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java @@ -99,8 +99,7 @@ public boolean isCustomDataPath() { } /** - * This method walks through the nodes shard paths to find the data and state path for the given shard. If multiple - * directories with a valid shard state exist the one with the highest version will be used. + * This method resolves the node's shard path using the given {@link NodeEnvironment}. * Note: this method resolves custom data locations for the shard if such a custom data path is provided. */ public static ShardPath loadShardPath(Logger logger, NodeEnvironment env, @@ -111,32 +110,31 @@ public static ShardPath loadShardPath(Logger logger, NodeEnvironment env, } /** - * This method walks through the nodes shard paths to find the data and state path for the given shard. If multiple - * directories with a valid shard state exist the one with the highest version will be used. + * This method resolves the node's shard path using the given data paths. * Note: this method resolves custom data locations for the shard. */ - public static ShardPath loadShardPath(Logger logger, ShardId shardId, String customDataPath, Path availableShardPath, + public static ShardPath loadShardPath(Logger logger, ShardId shardId, String customDataPath, Path shardPath, Path sharedDataPath) throws IOException { final String indexUUID = shardId.getIndex().getUUID(); // EMPTY is safe here because we never call namedObject - ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, availableShardPath); + ShardStateMetadata load = ShardStateMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, shardPath); if (load != null) { if (load.indexUUID.equals(indexUUID) == false && IndexMetadata.INDEX_UUID_NA_VALUE.equals(load.indexUUID) == false) { logger.warn("{} found shard on path: [{}] with a different index UUID - this " + "shard seems to be leftover from a different index with the same name. " - + "Remove the leftover shard in order to reuse the path with the current index", shardId, availableShardPath); + + "Remove the leftover shard in order to reuse the path with the current index", shardId, shardPath); throw new IllegalStateException(shardId + " index UUID in shard state was: " + load.indexUUID - + " expected: " + indexUUID + " on shard path: " + availableShardPath); + + " expected: " + indexUUID + " on shard path: " + shardPath); } final Path dataPath; final boolean hasCustomDataPath = Strings.isNotEmpty(customDataPath); if (hasCustomDataPath) { dataPath = NodeEnvironment.resolveCustomLocation(customDataPath, shardId, sharedDataPath); } else { - dataPath = availableShardPath; + dataPath = shardPath; } - logger.debug("{} loaded data path [{}], state path [{}]", shardId, dataPath, availableShardPath); - return new ShardPath(hasCustomDataPath, dataPath, availableShardPath, shardId); + logger.debug("{} loaded data path [{}], state path [{}]", shardId, dataPath, shardPath); + return new ShardPath(hasCustomDataPath, dataPath, shardPath, shardId); } return null; }