Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
6 changes: 6 additions & 0 deletions docs/changelog/83603.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 83603
summary: Cache ILM policy name on `IndexMetadata`
area: ILM+SLM
type: enhancement
issues:
- 83582
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,9 @@ public static APIBlock readFrom(StreamInput input) throws IOException {

private final int shardsPerNodeLimit;

@Nullable // if an index isn't managed by ilm, it won't have a policy
private final String lifecyclePolicyName;

private final LifecycleExecutionState lifecycleExecutionState;

private final AutoExpandReplicas autoExpandReplicas;
Expand Down Expand Up @@ -544,6 +547,7 @@ private IndexMetadata(
final boolean ignoreDiskWatermarks,
@Nullable final List<String> tierPreference,
final int shardsPerNodeLimit,
final String lifecyclePolicyName,
final LifecycleExecutionState lifecycleExecutionState,
final AutoExpandReplicas autoExpandReplicas,
final boolean isSearchableSnapshot,
Expand Down Expand Up @@ -588,6 +592,7 @@ private IndexMetadata(
this.ignoreDiskWatermarks = ignoreDiskWatermarks;
this.tierPreference = tierPreference;
this.shardsPerNodeLimit = shardsPerNodeLimit;
this.lifecyclePolicyName = lifecyclePolicyName;
this.lifecycleExecutionState = lifecycleExecutionState;
this.autoExpandReplicas = autoExpandReplicas;
this.isSearchableSnapshot = isSearchableSnapshot;
Expand Down Expand Up @@ -632,6 +637,7 @@ IndexMetadata withMappingMetadata(MappingMetadata mapping) {
this.ignoreDiskWatermarks,
this.tierPreference,
this.shardsPerNodeLimit,
this.lifecyclePolicyName,
this.lifecycleExecutionState,
this.autoExpandReplicas,
this.isSearchableSnapshot,
Expand Down Expand Up @@ -759,6 +765,10 @@ public List<String> getTierPreference() {
return tierPreference;
}

public String getLifecyclePolicyName() {
return lifecyclePolicyName;
}

public LifecycleExecutionState getLifecycleExecutionState() {
return lifecycleExecutionState;
}
Expand Down Expand Up @@ -807,6 +817,10 @@ public Index getResizeSourceIndex() {
Property.PrivateIndex
);

// LIFECYCLE_NAME is here an as optimization, see LifecycleSettings.LIFECYCLE_NAME and
// LifecycleSettings.LIFECYCLE_NAME_SETTING for the 'real' version
public static final String LIFECYCLE_NAME = "index.lifecycle.name";

ImmutableOpenMap<String, DiffableStringMap> getCustomData() {
return this.customData;
}
Expand Down Expand Up @@ -1642,6 +1656,7 @@ public IndexMetadata build() {
DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS.get(settings),
tierPreference,
ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.get(settings),
settings.get(IndexMetadata.LIFECYCLE_NAME),
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems mildly hacky, should we just move the full Setting instance in here so we can just use LIFECYCLE_NAME_SETTING.get(settings) the standard way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has the null semantics I want, but the standard way doesn't -- SOME_SETTING.get(settings) returns the default value for the setting if the setting is absent, and in this case that's an empty string (because you can't define the default value for a setting as being null).

Of course, that's not impossible to work around, we could do a hasText check or something like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see. This is mildly scary because we seem to have been mixing both patterns for this setting before and now we're only using one that has null in it.
But I guess we use if (Strings.isNullOrEmpty(policyName) == false) { or the return as the right hand side of an equals so it should be good.

lifecycleExecutionState,
AutoExpandReplicas.SETTING.get(settings),
isSearchableSnapshot,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,24 @@ public void testBuildsWithBrokenTierPreference() {
expectThrows(IllegalArgumentException.class, indexMetadata::getTierPreference);
}

public void testLifeCyclePolicyName() {
Settings.Builder settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 8))
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT);

IndexMetadata idxMeta1 = IndexMetadata.builder("test").settings(settings).build();

// null means no policy
assertNull(idxMeta1.getLifecyclePolicyName());

IndexMetadata idxMeta2 = IndexMetadata.builder(idxMeta1)
.settings(settings.put(IndexMetadata.LIFECYCLE_NAME, "some_policy").build())
.build();

assertThat(idxMeta2.getLifecyclePolicyName(), equalTo("some_policy"));
}

private static Settings indexSettingsWithDataTier(String dataTier) {
return Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) {
return new Result(false, new Info(errorMessage));
}

String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
IndexAbstraction indexAbstraction = clusterState.metadata().getIndicesLookup().get(indexName);
assert indexAbstraction != null : "invalid cluster metadata. index [" + indexName + "] was not found";
IndexAbstraction.DataStream dataStream = indexAbstraction.getParentDataStream();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) {
if (numberOfShards != null) {
int sourceNumberOfShards = indexMetadata.getNumberOfShards();
if (sourceNumberOfShards % numberOfShards != 0) {
String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
String errorMessage = String.format(
Locale.ROOT,
"lifecycle action of policy [%s] for index [%s] cannot make progress "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl
if (currentClusterState.metadata().index(shrunkenIndexSource) == null) {
// if the source index does not exist, we'll skip deleting the
// (managed) shrunk index as that will cause data loss
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
String policyName = indexMetadata.getLifecyclePolicyName();
logger.warn(
"managed index [{}] as part of policy [{}] is a shrunk index and the source index [{}] does not exist "
+ "anymore. will skip the [{}] step",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl
@Override
public void onResponse(AcknowledgedResponse acknowledgedResponse) {
if (acknowledgedResponse.isAcknowledged() == false) {
String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
throw new ElasticsearchException(
"cleanup snapshot step request for repository ["
+ repositoryName
Expand All @@ -82,7 +82,7 @@ public void onFailure(Exception e) {
listener.onResponse(null);
} else {
if (e instanceof RepositoryMissingException) {
String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
listener.onFailure(
new IllegalStateException(
"repository ["
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void createSnapshot(IndexMetadata indexMetadata, ActionListener<Boolean> listene

final LifecycleExecutionState lifecycleState = indexMetadata.getLifecycleExecutionState();

final String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
final String policyName = indexMetadata.getLifecyclePolicyName();
final String snapshotRepository = lifecycleState.snapshotRepository();
if (Strings.hasText(snapshotRepository) == false) {
listener.onFailure(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public DeleteStep(StepKey key, StepKey nextStepKey, Client client) {

@Override
public void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentState, ActionListener<Void> listener) {
String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
String indexName = indexMetadata.getIndex().getName();
IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(indexName);
assert indexAbstraction != null : "invalid cluster metadata. index [" + indexName + "] was not found";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
IndexMetadata indexMetadata = clusterState.metadata().index(index);
assert indexMetadata != null : "index " + index.getName() + " must exist in the cluster state";
if (indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME) != null) {
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
String policyName = indexMetadata.getLifecyclePolicyName();
logger.warn(
"[{}] action is configured for index [{}] in policy [{}] which is mounted as searchable snapshot. "
+ "Skipping this action",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void performAction(
listener.onResponse(null);
} else {
DefaultShardOperationFailedException[] failures = response.getShardFailures();
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
String policyName = indexMetadata.getLifecyclePolicyName();
String errorMessage = String.format(
Locale.ROOT,
"index [%s] in policy [%s] encountered failures [%s] on step [%s]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
(index, clusterState) -> {
IndexMetadata indexMetadata = clusterState.getMetadata().index(index);
assert indexMetadata != null : "index " + index.getName() + " must exist in the cluster state";
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
String policyName = indexMetadata.getLifecyclePolicyName();
if (indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME) != null) {
logger.warn(
"[{}] action is configured for index [{}] in policy [{}] which is mounted as searchable snapshot. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
return clusterState;
}

String policy = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
LifecycleExecutionState lifecycleState = indexMetadata.getLifecycleExecutionState();

// validate that the snapshot repository exists -- because policies are refreshed on later retries, and because
Expand All @@ -70,7 +70,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
"repository ["
+ snapshotRepository
+ "] is missing. ["
+ policy
+ policyName
+ "] policy for "
+ "index ["
+ index.getName()
Expand All @@ -83,13 +83,13 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
newCustomData.setSnapshotRepository(snapshotRepository);
if (lifecycleState.snapshotName() == null) {
// generate and validate the snapshotName
String snapshotNamePrefix = ("<{now/d}-" + index.getName() + "-" + policy + ">").toLowerCase(Locale.ROOT);
String snapshotNamePrefix = ("<{now/d}-" + index.getName() + "-" + policyName + ">").toLowerCase(Locale.ROOT);
String snapshotName = generateSnapshotName(snapshotNamePrefix);
ActionRequestValidationException validationException = validateGeneratedSnapshotName(snapshotNamePrefix, snapshotName);
if (validationException != null) {
logger.warn(
"unable to generate a snapshot name as part of policy [{}] for index [{}] due to [{}]",
policy,
policyName,
index.getName(),
validationException.getMessage()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
LifecycleExecutionState lifecycleState = indexMetadata.getLifecycleExecutionState();

Builder newCustomData = LifecycleExecutionState.builder(lifecycleState);
String policy = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
String generatedIndexName = generateValidIndexName(prefix, index.getName());
ActionRequestValidationException validationException = validateGeneratedIndexName(generatedIndexName, clusterState);
if (validationException != null) {
logger.warn(
"unable to generate a valid index name as part of policy [{}] for index [{}] due to [{}]",
policy,
policyName,
index.getName(),
validationException.getMessage()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
);
}
} catch (Exception e) {
String policy = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
throw new InitializePolicyException(policy, index.getName(), e);
String policyName = indexMetadata.getLifecyclePolicyName();
throw new InitializePolicyException(policyName, index.getName(), e);
}

ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static ItemUsage calculateUsage(
.indices()
.values()
.stream()
.filter(indexMetadata -> policyName.equals(LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings())))
.filter(indexMetadata -> policyName.equals(indexMetadata.getLifecyclePolicyName()))
.map(indexMetadata -> indexMetadata.getIndex().getName())
.collect(Collectors.toList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,18 @@
*/
package org.elasticsearch.xpack.core.ilm;

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.xpack.core.scheduler.CronSchedule;

import static org.elasticsearch.common.settings.Setting.timeSetting;

/**
* Class encapsulating settings related to Index Lifecycle Management X-Pack Plugin
*/
public class LifecycleSettings {
public static final String LIFECYCLE_POLL_INTERVAL = "indices.lifecycle.poll_interval";
public static final String LIFECYCLE_NAME = "index.lifecycle.name";
public static final String LIFECYCLE_NAME = IndexMetadata.LIFECYCLE_NAME;
public static final String LIFECYCLE_INDEXING_COMPLETE = "index.lifecycle.indexing_complete";
public static final String LIFECYCLE_ORIGINATION_DATE = "index.lifecycle.origination_date";
public static final String LIFECYCLE_PARSE_ORIGINATION_DATE = "index.lifecycle.parse_origination_date";
Expand All @@ -35,7 +34,7 @@ public class LifecycleSettings {
// already mounted as a searchable snapshot. Those ILM actions will check if the index has this setting name configured.
public static final String SNAPSHOT_INDEX_NAME = "index.store.snapshot.index_name";

public static final Setting<TimeValue> LIFECYCLE_POLL_INTERVAL_SETTING = timeSetting(
public static final Setting<TimeValue> LIFECYCLE_POLL_INTERVAL_SETTING = Setting.timeSetting(
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a cleanup commit. "While I was here", so to speak.

LIFECYCLE_POLL_INTERVAL,
TimeValue.timeValueMinutes(10),
TimeValue.timeValueSeconds(1),
Expand Down Expand Up @@ -81,7 +80,7 @@ public class LifecycleSettings {
// This setting configures how much time since step_time should ILM wait for a condition to be met. After the threshold wait time has
// elapsed ILM will likely stop waiting and go to the next step.
// Also see {@link org.elasticsearch.xpack.core.ilm.ClusterStateWaitUntilThresholdStep}
public static final Setting<TimeValue> LIFECYCLE_STEP_WAIT_TIME_THRESHOLD_SETTING = timeSetting(
public static final Setting<TimeValue> LIFECYCLE_STEP_WAIT_TIME_THRESHOLD_SETTING = Setting.timeSetting(
LIFECYCLE_STEP_WAIT_TIME_THRESHOLD,
TimeValue.timeValueHours(12),
TimeValue.timeValueHours(1),
Expand Down Expand Up @@ -114,7 +113,7 @@ public class LifecycleSettings {
Setting.Property.Dynamic,
Setting.Property.NodeScope
);
public static final Setting<TimeValue> SLM_RETENTION_DURATION_SETTING = timeSetting(
public static final Setting<TimeValue> SLM_RETENTION_DURATION_SETTING = Setting.timeSetting(
SLM_RETENTION_DURATION,
TimeValue.timeValueHours(1),
TimeValue.timeValueMillis(500),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {

// partially mounted indices will already have data_frozen, and we don't want to change that if they do
if (indexMetadata.isPartialSearchableSnapshot()) {
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
String policyName = indexMetadata.getLifecyclePolicyName();
logger.debug(
"[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. "
+ "skipping this action",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl

LifecycleExecutionState lifecycleState = indexMetadata.getLifecycleExecutionState();

String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = indexMetadata.getLifecyclePolicyName();
final String snapshotRepository = lifecycleState.snapshotRepository();
if (Strings.hasText(snapshotRepository) == false) {
listener.onFailure(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public static boolean updateIndicesForPolicy(
.indices()
.values()
.stream()
.filter(meta -> newPolicy.getName().equals(LifecycleSettings.LIFECYCLE_NAME_SETTING.get(meta.getSettings())))
.filter(meta -> newPolicy.getName().equals(meta.getLifecyclePolicyName()))
.filter(meta -> isIndexPhaseDefinitionUpdatable(xContentRegistry, client, meta, newPolicy.getPolicy(), licenseState))
.collect(Collectors.toList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) {

String originalIndex = index.getName();
String targetIndexName = targetIndexNameSupplier.apply(originalIndex, originalIndexMetadata.getLifecycleExecutionState());
String policyName = originalIndexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
String policyName = originalIndexMetadata.getLifecyclePolicyName();
IndexAbstraction indexAbstraction = clusterState.metadata().getIndicesLookup().get(index.getName());
assert indexAbstraction != null : "invalid cluster metadata. index [" + index.getName() + "] was not found";
IndexAbstraction.DataStream dataStream = indexAbstraction.getParentDataStream();
Expand Down
Loading