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

Allocate newly created indices on data_hot tier nodes #61342

Merged
merged 18 commits into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
8 changes: 8 additions & 0 deletions docs/reference/api-conventions.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ Returns:
"index.creation_date": "1474389951325",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably want to update https://www.elastic.co/guide/en/elasticsearch/reference/7.9/shard-allocation-filtering.html with or soon after this PR. (also https://www.elastic.co/guide/en/elasticsearch/reference/7.9/modules-node.html#data-node could use an update, but is outside the scope of this PR)

Copy link
Member Author

@dakrone dakrone Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I am leaving documentation for a subsequent PR though, in the chance that these end up having a name change. I will make sure they are handled in the documentation PR.

"index.uuid": "n6gzFZTgS664GUfx0Xrpjw",
"index.version.created": ...,
"index.routing.allocation.include._tier" : "data_hot",
"index.provided_name" : "my-index-000001"
}
}
Expand Down Expand Up @@ -424,6 +425,13 @@ Returns:
"version": {
"created": ...
},
"routing": {
"allocation": {
"include": {
"_tier": "data_hot"
}
}
},
"provided_name" : "my-index-000001"
}
}
Expand Down
7 changes: 6 additions & 1 deletion docs/reference/cluster/allocation-explain.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,12 @@ DELETE my-index-000001
[source,console]
--------------------------------------------------
PUT /my-index-000001?master_timeout=1s&timeout=1s
{"settings": {"index.routing.allocation.include._name": "non_existent_node"} }
{
"settings": {
"index.routing.allocation.include._name": "non_existent_node",
"index.routing.allocation.include._tier": null
}
}

GET /_cluster/allocation/explain
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ public void testRelocationWithConcurrentIndexing() throws Exception {
// but the recovering copy will be seen as invalid and the cluster health won't return to GREEN
// before timing out
.put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "100ms")
.put("index.routing.allocation.include._tier", "")
.put(SETTING_ALLOCATION_MAX_RETRY.getKey(), "0"); // fail faster
createIndex(index, settings.build());
indexDocs(index, 0, 10);
Expand All @@ -240,6 +241,7 @@ public void testRelocationWithConcurrentIndexing() throws Exception {
.put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 0)
.put(INDEX_ROUTING_ALLOCATION_ENABLE_SETTING.getKey(), (String)null)
.put("index.routing.allocation.include._id", oldNode)
.putNull("index.routing.allocation.include._tier")
);
ensureGreen(index); // wait for the primary to be assigned
ensureNoInitializingShards(); // wait for all other shard activity to finish
Expand All @@ -262,6 +264,7 @@ public void testRelocationWithConcurrentIndexing() throws Exception {
updateIndexSettings(index, Settings.builder()
.put(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 2)
.put("index.routing.allocation.include._id", (String)null)
.putNull("index.routing.allocation.include._tier")
);
asyncIndexDocs(index, 60, 45).get();
ensureGreen(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MapperService.MergeReason;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.shard.IndexSettingProvider;
import org.elasticsearch.indices.IndexCreationException;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.InvalidIndexNameException;
Expand All @@ -83,6 +84,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -127,6 +129,7 @@ public class MetadataCreateIndexService {
private final SystemIndices systemIndices;
private final ShardLimitValidator shardLimitValidator;
private final boolean forbidPrivateIndexSettings;
private final Set<IndexSettingProvider> indexSettingProviders = new HashSet<>();

public MetadataCreateIndexService(
final Settings settings,
Expand Down Expand Up @@ -155,6 +158,19 @@ public MetadataCreateIndexService(
this.shardLimitValidator = shardLimitValidator;
}

/**
* Add a provider to be invoked to get additional index settings prior to an index being created
*/
public void addAdditionalIndexSettingProvider(IndexSettingProvider provider) {
if (provider == null) {
throw new IllegalArgumentException("provider must not be null");
}
if (indexSettingProviders.contains(provider)) {
throw new IllegalArgumentException("provider already added");
}
this.indexSettingProviders.add(provider);
}

/**
* Validate the name for an index against some static rules and a cluster state.
*/
Expand Down Expand Up @@ -465,7 +481,7 @@ private ClusterState applyCreateIndexRequestWithV1Templates(final ClusterState c

final Settings aggregatedIndexSettings =
aggregateIndexSettings(currentState, request, MetadataIndexTemplateService.resolveSettings(templates),
null, settings, indexScopedSettings, shardLimitValidator);
null, settings, indexScopedSettings, shardLimitValidator, indexSettingProviders);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);

Expand All @@ -491,7 +507,7 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu
final Settings aggregatedIndexSettings =
aggregateIndexSettings(currentState, request,
MetadataIndexTemplateService.resolveSettings(currentState.metadata(), templateName),
null, settings, indexScopedSettings, shardLimitValidator);
null, settings, indexScopedSettings, shardLimitValidator, indexSettingProviders);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);

Expand Down Expand Up @@ -537,7 +553,7 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(final ClusterSt
}

final Settings aggregatedIndexSettings = aggregateIndexSettings(currentState, request, Settings.EMPTY,
sourceMetadata, settings, indexScopedSettings, shardLimitValidator);
sourceMetadata, settings, indexScopedSettings, shardLimitValidator, indexSettingProviders);
final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);

Expand Down Expand Up @@ -596,14 +612,64 @@ static Map<String, Object> parseV1Mappings(String mappingsJson, List<CompressedX
* @return the aggregated settings for the new index
*/
static Settings aggregateIndexSettings(ClusterState currentState, CreateIndexClusterStateUpdateRequest request,
Settings templateSettings, @Nullable IndexMetadata sourceMetadata, Settings settings,
IndexScopedSettings indexScopedSettings, ShardLimitValidator shardLimitValidator) {
Settings.Builder indexSettingsBuilder = Settings.builder();
Settings combinedTemplateSettings, @Nullable IndexMetadata sourceMetadata, Settings settings,
IndexScopedSettings indexScopedSettings, ShardLimitValidator shardLimitValidator,
Set<IndexSettingProvider> indexSettingProviders) {
// Create builders for the template and request settings. We transform these into builders
// because we may want settings to be "removed" from these prior to being set on the new
// index (see more comments below)
final Settings.Builder templateSettings = Settings.builder().put(combinedTemplateSettings);
final Settings.Builder requestSettings = Settings.builder().put(request.settings());

final Settings.Builder indexSettingsBuilder = Settings.builder();
if (sourceMetadata == null) {
indexSettingsBuilder.put(templateSettings);
final Settings.Builder additionalIndexSettings = Settings.builder();
final Settings templateAndRequestSettings = Settings.builder()
.put(combinedTemplateSettings)
.put(request.settings())
.build();

// Loop through all the explicit index setting providers, adding them to the
// additionalIndexSettings map
for (IndexSettingProvider provider : indexSettingProviders) {
additionalIndexSettings.put(provider.getAdditionalIndexSettings(request.index(), templateAndRequestSettings));
}

// For all the explicit settings, we go through the template and request level settings
// and see if either a template or the request has "cancelled out" an explicit default
// setting. For example, if a plugin had as an explicit setting:
// "index.mysetting": "blah
// And either a template or create index request had:
// "index.mysetting": null
// We want to remove the explicit setting not only from the explicitly set settings, but
// also from the template and request settings, so that from the newly create index's
// perspective it is as though the setting has not been set at all (using the default
// value).
for (String explicitSetting : additionalIndexSettings.keys()) {
if (templateSettings.keys().contains(explicitSetting) && templateSettings.get(explicitSetting) == null) {
logger.debug("removing default [{}] setting as it in set to null in a template for [{}] creation",
explicitSetting, request.index());
additionalIndexSettings.remove(explicitSetting);
templateSettings.remove(explicitSetting);
dakrone marked this conversation as resolved.
Show resolved Hide resolved
}
if (requestSettings.keys().contains(explicitSetting) && requestSettings.get(explicitSetting) == null) {
logger.debug("removing default [{}] setting as it in set to null in the request for [{}] creation",
explicitSetting, request.index());
additionalIndexSettings.remove(explicitSetting);
requestSettings.remove(explicitSetting);
dakrone marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Finally, we actually add the explicit defaults prior to the template settings and the
// request settings, so that the precedence goes:
// Explicit Defaults -> Template -> Request -> Necessary Settings (# of shards, uuid, etc)
indexSettingsBuilder.put(additionalIndexSettings.build());
indexSettingsBuilder.put(templateSettings.build());
}

// now, put the request settings, so they override templates
indexSettingsBuilder.put(request.settings());
indexSettingsBuilder.put(requestSettings.build());

if (indexSettingsBuilder.get(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey()) == null) {
final DiscoveryNodes nodes = currentState.nodes();
final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ public boolean match(DiscoveryNode node) {
}
}
}
} else if ("_tier".equals(attr)) {
// Always allow _tier as an attribute, will be handled elsewhere
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outside the scope of this PR, but can you use _tier with the node specification ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you can, I added tests for that in #60994

return true;
} else {
String nodeAttributeValue = node.getAttributes().get(attr);
if (nodeAttributeValue == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.index.shard;

import org.elasticsearch.common.settings.Settings;

/**
* An {@link IndexSettingProvider} is a provider for index level settings that can be set
* explicitly as a default value (so they show up as "set" for newly created indices)
*/
public interface IndexSettingProvider {
/**
* Returns explicitly set default index {@link Settings} for the given index. This should not
* return null.
*/
default Settings getAdditionalIndexSettings(String indexName, Settings templateAndRequestSettings) {
return Settings.EMPTY;
}
}
3 changes: 3 additions & 0 deletions server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,9 @@ protected Node(final Environment initialEnvironment,
systemIndices,
forbidPrivateIndexSettings
);
pluginsService.filterPlugins(Plugin.class)
.forEach(p -> p.getAdditionalIndexSettingProviders()
.forEach(metadataCreateIndexService::addAdditionalIndexSettingProvider));

final MetadataCreateDataStreamService metadataCreateDataStreamService =
new MetadataCreateDataStreamService(threadPool, clusterService, metadataCreateIndexService);
Expand Down
11 changes: 11 additions & 0 deletions server/src/main/java/org/elasticsearch/plugins/Plugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.shard.IndexSettingProvider;
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.ExecutorBuilder;
Expand Down Expand Up @@ -197,4 +198,14 @@ public Set<DiscoveryNodeRole> getRoles() {
public void close() throws IOException {

}

/**
* An {@link IndexSettingProvider} allows hooking in to parts of an index
* lifecycle to provide explicit default settings for newly created indices. Rather than changing
* the default values for an index-level setting, these act as though the setting has been set
* explicitly, but still allow the setting to be overridden by a template or creation request body.
*/
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders() {
return Collections.emptyList();
}
}
Loading