Skip to content

Commit

Permalink
Remove the node local storage setting (#54381)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jasontedor
jasontedor authored Mar 30, 2020
1 parent 6f84652 commit 4a3688d
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 132 deletions.
5 changes: 3 additions & 2 deletions docs/reference/migration/migrate_8_0/settings.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
12 changes: 0 additions & 12 deletions server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 7 additions & 20 deletions server/src/main/java/org/elasticsearch/env/Environment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -118,22 +114,13 @@ public Environment(final Settings settings, final Path configPath, final boolean

List<String> 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();
Expand Down
7 changes: 0 additions & 7 deletions server/src/main/java/org/elasticsearch/env/NodeEnvironment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
35 changes: 15 additions & 20 deletions server/src/main/java/org/elasticsearch/indices/IndicesService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand All @@ -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;
}
}

Expand Down
4 changes: 0 additions & 4 deletions server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 1 addition & 10 deletions server/src/main/java/org/elasticsearch/node/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,6 @@ public class Node implements Closeable {
public static final Setting<Boolean> 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<Boolean> NODE_LOCAL_STORAGE_SETTING =
Setting.boolSetting("node.local_storage", true, Property.Deprecated, Property.NodeScope);
public static final Setting<String> NODE_NAME_SETTING = Setting.simpleString("node.name", Property.NodeScope);
public static final Setting.AffixSetting<String> NODE_ATTRIBUTES = Setting.prefixKeySetting("node.attr.", (key) ->
new Setting<>(key, "", (value) -> {
Expand Down Expand Up @@ -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<ExecutorBuilder<?>> executorBuilders = pluginsService.getExecutorBuilders(settings);
Expand Down
35 changes: 3 additions & 32 deletions server/src/test/java/org/elasticsearch/env/EnvironmentTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"));
Expand All @@ -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"));
Expand All @@ -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"));

Expand Down
24 changes: 0 additions & 24 deletions server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4a3688d

Please sign in to comment.