-
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 9 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; | ||
|
@@ -574,6 +575,26 @@ public ImmutableOpenMap<String, AliasMetadata> getAliases() { | |
return this.aliases; | ||
} | ||
|
||
/** | ||
* Lazy loaded cache for tier preference setting. We can't eager load this setting because | ||
* {@link IndexMetadataVerifier#convertSharedCacheTierPreference(IndexMetadata)} might not have acted on this index yet and thus the | ||
* setting validation for this setting could fail for metadata loaded from a snapshot or disk after an upgrade. | ||
* Note: this field needs no synchronization since its a pure function of the immutable {@link #settings}, similar to how | ||
* {@link String#hashCode()} works. | ||
*/ | ||
@Nullable // since lazy-loaded | ||
private List<String> tierPreference; | ||
DaveCTurner marked this conversation as resolved.
Show resolved
Hide resolved
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 we should this still mark this volatile to avoid unsafe publishing. Neither The difference to |
||
|
||
public List<String> getTierPreference() { | ||
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 simple test 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. ++ added both tests |
||
List<String> tierPreference = this.tierPreference; | ||
if (tierPreference != null) { | ||
return tierPreference; | ||
} | ||
tierPreference = DataTier.parseTierList(DataTier.TIER_PREFERENCE_SETTING.get(settings)); | ||
this.tierPreference = tierPreference; | ||
return tierPreference; | ||
} | ||
|
||
/** | ||
* Return the concrete mapping for this index or {@code null} if this index has no mappings at all. | ||
*/ | ||
|
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.
An alternative to lazy loading would be to store a null here if parsing the field fails when building and then parse the field every time when
getTierPreference
is called (or parse it to throw the exception and then if it succeeds throw another exception). That seems slightly better to me, since then the field is final.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.
++ I think I like this option best
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.
Implemented :)