Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the node local storage setting #54381

Merged
merged 2 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/reference/migration/migrate_8_0/settings.asciidoc
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.
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
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
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
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
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
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
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
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
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