-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Store DataTier Preference directly on IndexMetadata #78668
Changes from 12 commits
0e02b49
66735e1
cacac54
0c69999
3d3bcaa
e758bf3
c13e2db
5fd6f4e
d9b2363
3f381bb
ecb5f1f
59adab9
9ae2ca7
e4ba16e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
import org.elasticsearch.cluster.block.ClusterBlockLevel; | ||
import org.elasticsearch.cluster.node.DiscoveryNodeFilters; | ||
import org.elasticsearch.cluster.routing.IndexRouting; | ||
import org.elasticsearch.cluster.routing.allocation.DataTier; | ||
import org.elasticsearch.cluster.routing.allocation.IndexMetadataUpdater; | ||
import org.elasticsearch.cluster.routing.allocation.decider.DiskThresholdDecider; | ||
import org.elasticsearch.common.collect.ImmutableOpenIntMap; | ||
|
@@ -400,6 +401,9 @@ public static APIBlock readFrom(StreamInput input) throws IOException { | |
|
||
private final boolean ignoreDiskWatermarks; | ||
|
||
@Nullable // since we store null if DataTier.TIER_PREFERENCE_SETTING failed validation | ||
private final List<String> tierPreference; | ||
|
||
private IndexMetadata( | ||
final Index index, | ||
final long version, | ||
|
@@ -429,7 +433,8 @@ private IndexMetadata( | |
final IndexLongFieldRange timestampRange, | ||
final int priority, | ||
final long creationDate, | ||
final boolean ignoreDiskWatermarks | ||
final boolean ignoreDiskWatermarks, | ||
@Nullable final List<String> tierPreference | ||
) { | ||
|
||
this.index = index; | ||
|
@@ -468,6 +473,7 @@ private IndexMetadata( | |
this.priority = priority; | ||
this.creationDate = creationDate; | ||
this.ignoreDiskWatermarks = ignoreDiskWatermarks; | ||
this.tierPreference = tierPreference; | ||
assert numberOfShards * routingFactor == routingNumShards : routingNumShards + " must be a multiple of " + numberOfShards; | ||
} | ||
|
||
|
@@ -574,6 +580,15 @@ public ImmutableOpenMap<String, AliasMetadata> getAliases() { | |
return this.aliases; | ||
} | ||
|
||
public List<String> getTierPreference() { | ||
if (tierPreference == null) { | ||
final List<String> parsed = DataTier.parseTierList(DataTier.TIER_PREFERENCE_SETTING.get(settings)); | ||
assert false : "the setting parsing should always throw if we didn't store a tier preference when building this instance"; | ||
return parsed; | ||
} | ||
return tierPreference; | ||
} | ||
|
||
/** | ||
* Return the concrete mapping for this index or {@code null} if this index has no mappings at all. | ||
*/ | ||
|
@@ -1311,6 +1326,16 @@ public IndexMetadata build() { | |
|
||
final String uuid = settings.get(SETTING_INDEX_UUID, INDEX_UUID_NA_VALUE); | ||
|
||
List<String> tierPreference; | ||
try { | ||
tierPreference = DataTier.parseTierList(DataTier.TIER_PREFERENCE_SETTING.get(settings)); | ||
} catch (Exception e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this has to be an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically yes, but we also call some other settings. I don't know if we want to bank on no changes to that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, can we then assert that it is an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ++ aded |
||
// BwC hack: the setting failed validation but it will be fixed in | ||
// #IndexMetadataVerifier#convertSharedCacheTierPreference(IndexMetadata)} later so we just store a null | ||
// to be able to build a temporary instance | ||
tierPreference = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test that we can build an |
||
} | ||
|
||
return new IndexMetadata( | ||
new Index(index, uuid), | ||
version, | ||
|
@@ -1340,7 +1365,8 @@ public IndexMetadata build() { | |
timestampRange, | ||
IndexMetadata.INDEX_PRIORITY_SETTING.get(settings), | ||
settings.getAsLong(SETTING_CREATION_DATE, -1L), | ||
DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.get(settings) | ||
DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.get(settings), | ||
tierPreference | ||
); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,38 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.xpack.core; | ||
package org.elasticsearch.cluster.routing.allocation; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import org.elasticsearch.cluster.metadata.IndexMetadata; | ||
import org.elasticsearch.cluster.node.DiscoveryNode; | ||
import org.elasticsearch.cluster.node.DiscoveryNodeRole; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.settings.Setting; | ||
import org.elasticsearch.common.settings.Settings; | ||
import org.elasticsearch.index.IndexModule; | ||
import org.elasticsearch.index.shard.IndexSettingProvider; | ||
import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; | ||
import org.elasticsearch.snapshots.SearchableSnapshotsSettings; | ||
|
||
import java.util.Collection; | ||
import java.util.Iterator; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.function.Function; | ||
import java.util.stream.Collectors; | ||
|
||
/** | ||
* The {@code DataTier} class encapsulates the formalization of the "content", | ||
* "hot", "warm", and "cold" tiers as node roles. In contains the | ||
* roles themselves as well as helpers for validation and determining if a node | ||
* has a tier configured. | ||
* | ||
* Related: | ||
* {@link org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider} | ||
*/ | ||
public class DataTier { | ||
|
||
|
@@ -39,6 +44,17 @@ public class DataTier { | |
|
||
public static final Set<String> ALL_DATA_TIERS = Set.of(DATA_CONTENT, DATA_HOT, DATA_WARM, DATA_COLD, DATA_FROZEN); | ||
|
||
public static final String TIER_PREFERENCE = "index.routing.allocation.include._tier_preference"; | ||
|
||
public static final Setting<String> TIER_PREFERENCE_SETTING = new Setting<>( | ||
new Setting.SimpleKey(TIER_PREFERENCE), | ||
DataTierSettingValidator::getDefaultTierPreference, | ||
Function.identity(), | ||
new DataTierSettingValidator(), | ||
Setting.Property.Dynamic, | ||
Setting.Property.IndexScope | ||
); | ||
|
||
static { | ||
for (String tier : ALL_DATA_TIERS) { | ||
assert tier.equals(DATA_FROZEN) || tier.contains(DATA_FROZEN) == false | ||
|
@@ -59,8 +75,8 @@ public static boolean validTierName(String tierName) { | |
|
||
/** | ||
* Based on the provided target tier it will return a comma separated list of preferred tiers. | ||
* ie. if `data_cold` is the target tier, it will return `data_cold,data_warm,data_hot` | ||
* This is usually used in conjunction with {@link DataTierAllocationDecider#TIER_PREFERENCE_SETTING} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think we can keep this, the setting is still in scope here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ++ brought this back There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. u sure? I don't see it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now :) Sorry accidentally put this on the top level class. |
||
* ie. if `data_cold` is the target tier, it will return `data_cold,data_warm,data_hot`. | ||
* This is usually used in conjunction with {@link #TIER_PREFERENCE_SETTING}. | ||
*/ | ||
public static String getPreferredTiersConfiguration(String targetTier) { | ||
int indexOfTargetTier = ORDERED_FROZEN_TO_HOT_TIERS.indexOf(targetTier); | ||
|
@@ -115,6 +131,15 @@ public static boolean isFrozenNode(final Set<DiscoveryNodeRole> roles) { | |
return roles.contains(DiscoveryNodeRole.DATA_FROZEN_NODE_ROLE) || roles.contains(DiscoveryNodeRole.DATA_ROLE); | ||
} | ||
|
||
public static List<String> parseTierList(String tiers) { | ||
if (Strings.hasText(tiers) == false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bit weird that we treat all-whitespace as empty but otherwise we're whitespace-sensitive. (Acking that this is how it was before too, no action required) |
||
// avoid parsing overhead in the null/empty string case | ||
return List.of(); | ||
} else { | ||
return List.of(tiers.split(",")); | ||
} | ||
} | ||
|
||
/** | ||
* This setting provider injects the setting allocating all newly created indices with | ||
* {@code index.routing.allocation.include._tier_preference: "data_hot"} for a data stream index | ||
|
@@ -128,9 +153,9 @@ public static class DefaultHotAllocationSettingProvider implements IndexSettingP | |
@Override | ||
public Settings getAdditionalIndexSettings(String indexName, boolean isDataStreamIndex, Settings indexSettings) { | ||
Set<String> settings = indexSettings.keySet(); | ||
if (settings.contains(DataTierAllocationDecider.TIER_PREFERENCE)) { | ||
if (settings.contains(TIER_PREFERENCE)) { | ||
// just a marker -- this null value will be removed or overridden by the template/request settings | ||
return Settings.builder().putNull(DataTierAllocationDecider.TIER_PREFERENCE).build(); | ||
return Settings.builder().putNull(TIER_PREFERENCE).build(); | ||
} else if (settings.stream().anyMatch(s -> s.startsWith(IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".")) || | ||
settings.stream().anyMatch(s -> s.startsWith(IndexMetadata.INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".")) || | ||
settings.stream().anyMatch(s -> s.startsWith(IndexMetadata.INDEX_ROUTING_INCLUDE_GROUP_PREFIX + "."))) { | ||
|
@@ -142,11 +167,60 @@ public Settings getAdditionalIndexSettings(String indexName, boolean isDataStrea | |
// tier if the index is part of a data stream, the "content" | ||
// tier if it is not. | ||
if (isDataStreamIndex) { | ||
return Settings.builder().put(DataTierAllocationDecider.TIER_PREFERENCE, DATA_HOT).build(); | ||
return Settings.builder().put(TIER_PREFERENCE, DATA_HOT).build(); | ||
} else { | ||
return Settings.builder().put(TIER_PREFERENCE, DATA_CONTENT).build(); | ||
} | ||
} | ||
} | ||
} | ||
|
||
private static final class DataTierSettingValidator implements Setting.Validator<String> { | ||
|
||
private static final Collection<Setting<?>> dependencies = List.of( | ||
IndexModule.INDEX_STORE_TYPE_SETTING, | ||
SearchableSnapshotsSettings.SNAPSHOT_PARTIAL_SETTING | ||
); | ||
|
||
public static String getDefaultTierPreference(Settings settings) { | ||
if (SearchableSnapshotsSettings.isPartialSearchableSnapshotIndex(settings)) { | ||
return DATA_FROZEN; | ||
} else { | ||
return ""; | ||
} | ||
} | ||
|
||
@Override | ||
public void validate(String value) { | ||
if (Strings.hasText(value)) { | ||
for (String s : parseTierList(value)) { | ||
if (validTierName(s) == false) { | ||
throw new IllegalArgumentException( | ||
"invalid tier names found in [" + value + "] allowed values are " + ALL_DATA_TIERS); | ||
} | ||
} | ||
} | ||
} | ||
|
||
@Override | ||
public void validate(String value, Map<Setting<?>, Object> settings, boolean exists) { | ||
if (exists && value != null) { | ||
if (SearchableSnapshotsSettings.isPartialSearchableSnapshotIndex(settings)) { | ||
if (value.equals(DATA_FROZEN) == false) { | ||
throw new IllegalArgumentException("only the [" + DATA_FROZEN + | ||
"] tier preference may be used for partial searchable snapshots (got: [" + value + "])"); | ||
} | ||
} else { | ||
return Settings.builder().put(DataTierAllocationDecider.TIER_PREFERENCE, DATA_CONTENT).build(); | ||
if (value.contains(DATA_FROZEN)) { | ||
throw new IllegalArgumentException("[" + DATA_FROZEN + "] tier can only be used for partial searchable snapshots"); | ||
} | ||
} | ||
} | ||
} | ||
|
||
@Override | ||
public Iterator<Setting<?>> settings() { | ||
return dependencies.iterator(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a simple test that
getTierPreference
delivers the right output?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ added both tests