From 4a3688df2fc441faf5ed9edbc42303829d311a32 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 30 Mar 2020 07:03:25 -0400 Subject: [PATCH] Remove the node local storage setting (#54381) In 7.8.0 the node.local_storage setting was deprecated in favor of requiring all nodes to have a form of persistent storage. This commit removes the node.local_storage setting. --- .../migration/migrate_8_0/settings.asciidoc | 5 +-- .../cluster/node/DiscoveryNode.java | 12 ------- .../common/settings/ClusterSettings.java | 1 - .../org/elasticsearch/env/Environment.java | 27 ++++---------- .../elasticsearch/env/NodeEnvironment.java | 7 ---- .../elasticsearch/indices/IndicesService.java | 35 ++++++++----------- .../indices/store/IndicesStore.java | 4 --- .../java/org/elasticsearch/node/Node.java | 11 +----- .../elasticsearch/env/EnvironmentTests.java | 35 ++----------------- .../env/NodeEnvironmentTests.java | 24 ------------- 10 files changed, 29 insertions(+), 132 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/settings.asciidoc b/docs/reference/migration/migrate_8_0/settings.asciidoc index a093b68759223..eec5c4add28e7 100644 --- a/docs/reference/migration/migrate_8_0/settings.asciidoc +++ b/docs/reference/migration/migrate_8_0/settings.asciidoc @@ -46,7 +46,8 @@ favor of setting `node.remote_cluster_client`. In Elasticsearch 8.0.0, the setting `cluster.remote.connect` is removed. [float] -==== `node.local_storage` is deprecated +==== `node.local_storage` is removed In Elasticsearch 7.8.0, the setting `node.local_storage` was deprecated and -beginning in Elasticsearch 8.0.0 all nodes will require local storage. +beginning in Elasticsearch 8.0.0 all nodes will require local storage. Therefore, +the `node.local_storage` setting has been removed. diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java index 153c734e82c71..b2d45cd85fa91 100644 --- a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java @@ -51,18 +51,6 @@ public class DiscoveryNode implements Writeable, ToXContentFragment { static final String COORDINATING_ONLY = "coordinating_only"; - public static boolean nodeRequiresLocalStorage(Settings settings) { - boolean localStorageEnable = Node.NODE_LOCAL_STORAGE_SETTING.get(settings); - if (localStorageEnable == false && - (Node.NODE_DATA_SETTING.get(settings) || - Node.NODE_MASTER_SETTING.get(settings)) - ) { - // TODO: make this a proper setting validation logic, requiring multi-settings validation - throw new IllegalArgumentException("storage can not be disabled for master and data nodes"); - } - return localStorageEnable; - } - public static boolean isMasterNode(Settings settings) { return Node.NODE_MASTER_SETTING.get(settings); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 719da2b89ff39..dcd8cc959a314 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -403,7 +403,6 @@ public void apply(Settings value, Settings current, Settings previous) { Node.NODE_INGEST_SETTING, Node.NODE_REMOTE_CLUSTER_CLIENT, Node.NODE_ATTRIBUTES, - Node.NODE_LOCAL_STORAGE_SETTING, AutoCreateIndex.AUTO_CREATE_INDEX_SETTING, BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX, ClusterName.CLUSTER_NAME_SETTING, diff --git a/server/src/main/java/org/elasticsearch/env/Environment.java b/server/src/main/java/org/elasticsearch/env/Environment.java index 7856aff225907..ce10e9bff7032 100644 --- a/server/src/main/java/org/elasticsearch/env/Environment.java +++ b/server/src/main/java/org/elasticsearch/env/Environment.java @@ -90,15 +90,11 @@ public class Environment { private final Path tmpFile; public Environment(final Settings settings, final Path configPath) { - this(settings, configPath, true); - } - - public Environment(final Settings settings, final Path configPath, final boolean nodeLocalStorage) { - this(settings, configPath, nodeLocalStorage, PathUtils.get(System.getProperty("java.io.tmpdir"))); + this(settings, configPath, PathUtils.get(System.getProperty("java.io.tmpdir"))); } // Should only be called directly by this class's unit tests - Environment(final Settings settings, final Path configPath, final boolean nodeLocalStorage, final Path tmpPath) { + Environment(final Settings settings, final Path configPath, final Path tmpPath) { final Path homeFile; if (PATH_HOME_SETTING.exists(settings)) { homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).toAbsolutePath().normalize(); @@ -118,22 +114,13 @@ public Environment(final Settings settings, final Path configPath, final boolean List dataPaths = PATH_DATA_SETTING.get(settings); final ClusterName clusterName = ClusterName.CLUSTER_NAME_SETTING.get(settings); - if (nodeLocalStorage) { - if (dataPaths.isEmpty() == false) { - dataFiles = new Path[dataPaths.size()]; - for (int i = 0; i < dataPaths.size(); i++) { - dataFiles[i] = PathUtils.get(dataPaths.get(i)).toAbsolutePath().normalize(); - } - } else { - dataFiles = new Path[]{homeFile.resolve("data")}; + if (dataPaths.isEmpty() == false) { + dataFiles = new Path[dataPaths.size()]; + for (int i = 0; i < dataPaths.size(); i++) { + dataFiles[i] = PathUtils.get(dataPaths.get(i)).toAbsolutePath().normalize(); } } else { - if (dataPaths.isEmpty()) { - dataFiles = EMPTY_PATH_ARRAY; - } else { - final String paths = String.join(",", dataPaths); - throw new IllegalStateException("node does not require local storage yet path.data is set to [" + paths + "]"); - } + dataFiles = new Path[]{homeFile.resolve("data")}; } if (PATH_SHARED_DATA_SETTING.exists(settings)) { sharedDataFile = PathUtils.get(PATH_SHARED_DATA_SETTING.get(settings)).toAbsolutePath().normalize(); diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 9164f6f23a8cd..2893e81ce458b 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -247,13 +247,6 @@ public void close() { * @param settings settings from elasticsearch.yml */ public NodeEnvironment(Settings settings, Environment environment) throws IOException { - if (!DiscoveryNode.nodeRequiresLocalStorage(settings)) { - nodePaths = null; - sharedDataPath = null; - locks = null; - nodeMetaData = new NodeMetaData(generateNodeId(settings), Version.CURRENT); - return; - } boolean success = false; try { diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index cd68658522126..6055efedc7dd0 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1029,8 +1029,7 @@ public IndexMetaData verifyIndexIsDeleted(final Index index, final ClusterState public enum ShardDeletionCheckResult { FOLDER_FOUND_CAN_DELETE, // shard data exists and can be deleted STILL_ALLOCATED, // the shard is still allocated / active on this node - NO_FOLDER_FOUND, // the shards data locations do not exist - NO_LOCAL_STORAGE // node does not have local storage (see DiscoveryNode.nodeRequiresLocalStorage) + NO_FOLDER_FOUND // the shards data locations do not exist } /** @@ -1042,25 +1041,21 @@ public enum ShardDeletionCheckResult { public ShardDeletionCheckResult canDeleteShardContent(ShardId shardId, IndexSettings indexSettings) { assert shardId.getIndex().equals(indexSettings.getIndex()); final IndexService indexService = indexService(shardId.getIndex()); - if (nodeEnv.hasNodeFile()) { - final boolean isAllocated = indexService != null && indexService.hasShard(shardId.id()); - if (isAllocated) { - return ShardDeletionCheckResult.STILL_ALLOCATED; // we are allocated - can't delete the shard - } else if (indexSettings.hasCustomDataPath()) { - // lets see if it's on a custom path (return false if the shared doesn't exist) - // we don't need to delete anything that is not there - return Files.exists(nodeEnv.resolveCustomLocation(indexSettings.customDataPath(), shardId)) ? - ShardDeletionCheckResult.FOLDER_FOUND_CAN_DELETE : - ShardDeletionCheckResult.NO_FOLDER_FOUND; - } else { - // lets see if it's path is available (return false if the shared doesn't exist) - // we don't need to delete anything that is not there - return FileSystemUtils.exists(nodeEnv.availableShardPaths(shardId)) ? - ShardDeletionCheckResult.FOLDER_FOUND_CAN_DELETE : - ShardDeletionCheckResult.NO_FOLDER_FOUND; - } + final boolean isAllocated = indexService != null && indexService.hasShard(shardId.id()); + if (isAllocated) { + return ShardDeletionCheckResult.STILL_ALLOCATED; // we are allocated - can't delete the shard + } else if (indexSettings.hasCustomDataPath()) { + // lets see if it's on a custom path (return false if the shared doesn't exist) + // we don't need to delete anything that is not there + return Files.exists(nodeEnv.resolveCustomLocation(indexSettings.customDataPath(), shardId)) ? + ShardDeletionCheckResult.FOLDER_FOUND_CAN_DELETE : + ShardDeletionCheckResult.NO_FOLDER_FOUND; } else { - return ShardDeletionCheckResult.NO_LOCAL_STORAGE; + // lets see if it's path is available (return false if the shared doesn't exist) + // we don't need to delete anything that is not there + return FileSystemUtils.exists(nodeEnv.availableShardPaths(shardId)) ? + ShardDeletionCheckResult.FOLDER_FOUND_CAN_DELETE : + ShardDeletionCheckResult.NO_FOLDER_FOUND; } } diff --git a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java index 0c04b3ecbbf04..4457b7c818104 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java +++ b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java @@ -166,10 +166,6 @@ public void clusterChanged(ClusterChangedEvent event) { case NO_FOLDER_FOUND: folderNotFoundCache.add(shardId); break; - case NO_LOCAL_STORAGE: - assert false : "shard deletion only runs on data nodes which always have local storage"; - // nothing to do - break; case STILL_ALLOCATED: // nothing to do break; diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 3f3af6171acdc..ec53ebbef0395 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -203,15 +203,6 @@ public class Node implements Closeable { public static final Setting NODE_REMOTE_CLUSTER_CLIENT = Setting.boolSetting("node.remote_cluster_client", true, Property.NodeScope); - /** - * controls whether the node is allowed to persist things like metadata to disk - * Note that this does not control whether the node stores actual indices (see - * {@link #NODE_DATA_SETTING}). However, if this is false, {@link #NODE_DATA_SETTING} - * and {@link #NODE_MASTER_SETTING} must also be false. - * - */ - public static final Setting NODE_LOCAL_STORAGE_SETTING = - Setting.boolSetting("node.local_storage", true, Property.Deprecated, Property.NodeScope); public static final Setting NODE_NAME_SETTING = Setting.simpleString("node.name", Property.NodeScope); public static final Setting.AffixSetting NODE_ATTRIBUTES = Setting.prefixKeySetting("node.attr.", (key) -> new Setting<>(key, "", (value) -> { @@ -333,7 +324,7 @@ protected Node(final Environment initialEnvironment, // create the environment based on the finalized (processed) view of the settings // this is just to makes sure that people get the same settings, no matter where they ask them from - this.environment = new Environment(settings, initialEnvironment.configFile(), Node.NODE_LOCAL_STORAGE_SETTING.get(settings)); + this.environment = new Environment(settings, initialEnvironment.configFile()); Environment.assertEquivalent(initialEnvironment, this.environment); final List> executorBuilders = pluginsService.getExecutorBuilders(settings); diff --git a/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java index b5976e0c08f17..e784e307fdd76 100644 --- a/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/EnvironmentTests.java @@ -32,11 +32,8 @@ import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.CoreMatchers.startsWith; -import static org.hamcrest.Matchers.arrayWithSize; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.hasToString; /** * Simple unit-tests for Environment.java @@ -124,37 +121,11 @@ public void testConfigPathWhenNotSet() { assertThat(environment.configFile(), equalTo(pathHome.resolve("config"))); } - public void testNodeDoesNotRequireLocalStorage() { - final Path pathHome = createTempDir().toAbsolutePath(); - final Settings settings = - Settings.builder() - .put("path.home", pathHome) - .put("node.master", false) - .put("node.data", false) - .build(); - final Environment environment = new Environment(settings, null, false); - assertThat(environment.dataFiles(), arrayWithSize(0)); - } - - public void testNodeDoesNotRequireLocalStorageButHasPathData() { - final Path pathHome = createTempDir().toAbsolutePath(); - final Path pathData = pathHome.resolve("data"); - final Settings settings = - Settings.builder() - .put("path.home", pathHome) - .put("path.data", pathData) - .put("node.master", false) - .put("node.data", false) - .build(); - final IllegalStateException e = expectThrows(IllegalStateException.class, () -> new Environment(settings, null, false)); - assertThat(e, hasToString(containsString("node does not require local storage yet path.data is set to [" + pathData + "]"))); - } - public void testNonExistentTempPathValidation() { Settings build = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) .build(); - Environment environment = new Environment(build, null, true, createTempDir().resolve("this_does_not_exist")); + Environment environment = new Environment(build, null, createTempDir().resolve("this_does_not_exist")); FileNotFoundException e = expectThrows(FileNotFoundException.class, environment::validateTmpFile); assertThat(e.getMessage(), startsWith("Temporary file directory [")); assertThat(e.getMessage(), endsWith("this_does_not_exist] does not exist or is not accessible")); @@ -164,7 +135,7 @@ public void testTempPathValidationWhenRegularFile() throws IOException { Settings build = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) .build(); - Environment environment = new Environment(build, null, true, createTempFile("something", ".test")); + Environment environment = new Environment(build, null, createTempFile("something", ".test")); IOException e = expectThrows(IOException.class, environment::validateTmpFile); assertThat(e.getMessage(), startsWith("Configured temporary file directory [")); assertThat(e.getMessage(), endsWith(".test] is not a directory")); @@ -184,7 +155,7 @@ public void testPathNormalization() throws IOException { // the above paths will be treated as relative to the working directory final Path workingDirectory = PathUtils.get(System.getProperty("user.dir")); - final Environment environment = new Environment(settings, null, true, createTempDir()); + final Environment environment = new Environment(settings, null, createTempDir()); final String homePath = Environment.PATH_HOME_SETTING.get(environment.settings()); assertPath(homePath, workingDirectory.resolve("home")); diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 6e13202464fe4..94c8f7814a75d 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -22,7 +22,6 @@ import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.PathUtils; -import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.set.Sets; @@ -52,7 +51,6 @@ import static org.hamcrest.Matchers.arrayWithSize; 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 @@ -385,28 +383,6 @@ public void testCustomDataPaths() throws Exception { env.close(); } - public void testNodeIdNotPersistedAtInitialization() throws IOException { - NodeEnvironment env = newNodeEnvironment(new String[0], Settings.builder() - .put("node.local_storage", false) - .put("node.master", false) - .put("node.data", false) - .build()); - String nodeID = env.nodeId(); - env.close(); - final String[] paths = tmpPaths(); - env = newNodeEnvironment(paths, Settings.EMPTY); - assertThat("previous node didn't have local storage enabled, id should change", env.nodeId(), not(equalTo(nodeID))); - nodeID = env.nodeId(); - env.close(); - env = newNodeEnvironment(paths, Settings.EMPTY); - assertThat(env.nodeId(), not(equalTo(nodeID))); - env.close(); - env = newNodeEnvironment(Settings.EMPTY); - assertThat(env.nodeId(), not(equalTo(nodeID))); - env.close(); - assertSettingDeprecationsAndWarnings(new Setting[]{Node.NODE_LOCAL_STORAGE_SETTING}); - } - public void testExistingTempFiles() throws IOException { String[] paths = tmpPaths(); // simulate some previous left over temp files