Skip to content

Commit 32e3ae4

Browse files
joegallojoegallo
authored
Searchable snapshots mounted in the 'hot' phase should be pinned to hot nodes (#72696) (#73184)
1 parent db21442 commit 32e3ae4

File tree

4 files changed

+88
-106
lines changed

4 files changed

+88
-106
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.apache.logging.log4j.LogManager;
1010
import org.apache.logging.log4j.Logger;
1111
import org.elasticsearch.client.Client;
12-
import org.elasticsearch.cluster.metadata.IndexMetadata;
1312
import org.elasticsearch.common.ParseField;
1413
import org.elasticsearch.common.Strings;
1514
import org.elasticsearch.common.io.stream.StreamInput;
@@ -21,15 +20,13 @@
2120
import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider;
2221
import org.elasticsearch.xpack.core.DataTier;
2322
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
23+
import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants;
2424

2525
import java.io.IOException;
26-
import java.util.Arrays;
2726
import java.util.List;
2827
import java.util.Objects;
2928
import java.util.stream.Collectors;
3029

31-
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.FROZEN_PHASE;
32-
3330
/**
3431
* A {@link LifecycleAction} which enables or disables the automatic migration of data between
3532
* {@link org.elasticsearch.xpack.core.DataTier}s.
@@ -103,40 +100,34 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
103100
StepKey migrationKey = new StepKey(phase, NAME, NAME);
104101
StepKey migrationRoutedKey = new StepKey(phase, NAME, DataTierMigrationRoutedStep.NAME);
105102

106-
Settings.Builder migrationSettings = Settings.builder();
107103
String targetTier = "data_" + phase;
108104
assert DataTier.validTierName(targetTier) : "invalid data tier name:" + targetTier;
109105

110106
BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey,
111107
(index, clusterState) -> {
112-
if (skipMigrateAction(phase, clusterState.metadata().index(index))) {
113-
String policyName =
114-
LifecycleSettings.LIFECYCLE_NAME_SETTING.get(clusterState.metadata().index(index).getSettings());
115-
logger.debug("[{}] action is configured for index [{}] in policy [{}] which is already mounted as a searchable " +
116-
"snapshot. skipping this action", MigrateAction.NAME, index.getName(), policyName);
108+
Settings indexSettings = clusterState.metadata().index(index).getSettings();
109+
110+
// partially mounted indices will already have data_frozen, and we don't want to change that if they do
111+
if (SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexSettings)) {
112+
String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexSettings);
113+
logger.debug("[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. " +
114+
"skipping this action", MigrateAction.NAME, policyName, index.getName());
117115
return true;
118116
}
119117

120-
// don't skip the migrate action as the index is not mounted as searchable snapshot or we're in the frozen phase
121118
return false;
122119
});
123-
migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(targetTier));
124120
UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client,
125-
migrationSettings.build());
121+
Settings.builder()
122+
.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(targetTier))
123+
.build());
126124
DataTierMigrationRoutedStep migrationRoutedStep = new DataTierMigrationRoutedStep(migrationRoutedKey, nextStepKey);
127-
return Arrays.asList(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep);
125+
return org.elasticsearch.common.collect.List.of(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep);
128126
} else {
129127
return org.elasticsearch.common.collect.List.of();
130128
}
131129
}
132130

133-
static boolean skipMigrateAction(String phase, IndexMetadata indexMetadata) {
134-
// if the index is a searchable snapshot we skip the migrate action (as mounting an index as searchable snapshot
135-
// configures the tier allocation preference), unless we're in the frozen phase
136-
return (indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME) != null)
137-
&& (phase.equals(FROZEN_PHASE) == false);
138-
}
139-
140131
/**
141132
* Based on the provided target tier it will return a comma separated list of preferred tiers.
142133
* ie. if `data_cold` is the target tier, it will return `data_cold,data_warm,data_hot`

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import org.elasticsearch.common.settings.Settings;
1818
import org.elasticsearch.index.IndexSettings;
1919
import org.elasticsearch.rest.RestStatus;
20+
import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider;
21+
import org.elasticsearch.xpack.core.DataTier;
2022
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction;
2123
import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest;
2224

@@ -101,10 +103,14 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl
101103
indexName = snapshotIndexName;
102104
}
103105

106+
Settings.Builder settingsBuilder = Settings.builder();
107+
settingsBuilder.put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), Boolean.FALSE.toString());
108+
// if we are mounting a searchable snapshot in the hot phase, then the index should be pinned to the hot nodes
109+
if (TimeseriesLifecycleType.HOT_PHASE.equals(this.getKey().getPhase())) {
110+
settingsBuilder.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_HOT);
111+
}
104112
final MountSearchableSnapshotRequest mountSearchableSnapshotRequest = new MountSearchableSnapshotRequest(mountedIndexName,
105-
snapshotRepository, snapshotName, indexName, Settings.builder()
106-
.put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), Boolean.FALSE.toString())
107-
.build(),
113+
snapshotRepository, snapshotName, indexName, settingsBuilder.build(),
108114
// we captured the index metadata when we took the snapshot. the index likely had the ILM execution state in the metadata.
109115
// if we were to restore the lifecycle.name setting, the restored index would be captured by the ILM runner and,
110116
// depending on what ILM execution state was captured at snapshot time, make it's way forward from _that_ step forward in

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MigrateActionTests.java‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,29 @@
77
package org.elasticsearch.xpack.core.ilm;
88

99
import org.elasticsearch.Version;
10+
import org.elasticsearch.cluster.ClusterName;
11+
import org.elasticsearch.cluster.ClusterState;
1012
import org.elasticsearch.cluster.metadata.IndexMetadata;
13+
import org.elasticsearch.cluster.metadata.Metadata;
1114
import org.elasticsearch.common.io.stream.Writeable.Reader;
12-
import org.elasticsearch.common.settings.Settings;
1315
import org.elasticsearch.common.xcontent.XContentParser;
1416
import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider;
1517
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
1618

1719
import java.io.IOException;
18-
import java.util.Arrays;
1920
import java.util.List;
2021

21-
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
22-
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
23-
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
22+
import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
2423
import static org.elasticsearch.xpack.core.DataTier.DATA_COLD;
2524
import static org.elasticsearch.xpack.core.DataTier.DATA_HOT;
2625
import static org.elasticsearch.xpack.core.DataTier.DATA_WARM;
2726
import static org.elasticsearch.xpack.core.ilm.MigrateAction.getPreferredTiersConfiguration;
28-
import static org.elasticsearch.xpack.core.ilm.MigrateAction.skipMigrateAction;
2927
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.COLD_PHASE;
3028
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.DELETE_PHASE;
31-
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.FROZEN_PHASE;
3229
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.HOT_PHASE;
3330
import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.WARM_PHASE;
31+
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY;
32+
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING;
3433
import static org.hamcrest.CoreMatchers.is;
3534

3635
public class MigrateActionTests extends AbstractActionTestCase<MigrateAction> {
@@ -116,43 +115,50 @@ public void testMigrateActionsConfiguresTierPreference() {
116115
}
117116
}
118117

119-
public void testSkipMigrateAction() {
120-
IndexMetadata snappedIndex = IndexMetadata.builder("snapped_index")
121-
.settings(
122-
Settings.builder()
123-
.put(LifecycleSettings.SNAPSHOT_INDEX_NAME, "snapped")
124-
.put(SETTING_VERSION_CREATED, Version.CURRENT)
125-
.put(SETTING_NUMBER_OF_SHARDS, 1)
126-
.put(SETTING_NUMBER_OF_REPLICAS, 0)
127-
)
128-
.build();
129-
130-
IndexMetadata regularIndex = IndexMetadata.builder("regular_index")
131-
.settings(
132-
Settings.builder()
133-
.put(SETTING_VERSION_CREATED, Version.CURRENT)
134-
.put(SETTING_NUMBER_OF_SHARDS, 1)
135-
.put(SETTING_NUMBER_OF_REPLICAS, 0)
136-
)
137-
.build();
118+
public void testMigrateActionWillSkipAPartiallyMountedIndex() {
119+
StepKey nextStepKey = new StepKey(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 10),
120+
randomAlphaOfLengthBetween(1, 10));
121+
MigrateAction action = new MigrateAction();
138122

123+
// does not skip an ordinary index
139124
{
140-
// migrate action is not skipped if the index is not a searchable snapshot
141-
Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE, FROZEN_PHASE)
142-
.forEach(phase -> assertThat(skipMigrateAction(phase, regularIndex), is(false)));
143-
}
125+
IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(5))
126+
.settings(settings(Version.CURRENT))
127+
.numberOfShards(1)
128+
.numberOfReplicas(2)
129+
.build();
144130

145-
{
146-
// migrate action is skipped if the index is a searchable snapshot for phases hot -> cold
147-
Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE)
148-
.forEach(phase -> assertThat(skipMigrateAction(phase, snappedIndex), is(true)));
131+
ClusterState clusterState = ClusterState.builder(new ClusterName("_name"))
132+
.metadata(Metadata.builder().put(indexMetadata, true).build())
133+
.build();
134+
135+
List<Step> steps = action.toSteps(null, HOT_PHASE, nextStepKey);
136+
BranchingStep firstStep = (BranchingStep) steps.get(0);
137+
UpdateSettingsStep secondStep = (UpdateSettingsStep) steps.get(1);
138+
firstStep.performAction(indexMetadata.getIndex(), clusterState);
139+
140+
assertEquals(secondStep.getKey(), firstStep.getNextStepKey());
149141
}
150142

143+
// does skip a partially mounted
151144
{
152-
// migrate action is never skipped for the frozen phase
153-
assertThat(skipMigrateAction(FROZEN_PHASE, snappedIndex), is(false));
154-
assertThat(skipMigrateAction(FROZEN_PHASE, regularIndex), is(false));
145+
IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(5))
146+
.settings(settings(Version.CURRENT)
147+
.put(INDEX_STORE_TYPE_SETTING.getKey(), SNAPSHOT_DIRECTORY_FACTORY_KEY)
148+
.put(SNAPSHOT_PARTIAL_SETTING.getKey(), true))
149+
.numberOfShards(1)
150+
.numberOfReplicas(2)
151+
.build();
152+
153+
ClusterState clusterState = ClusterState.builder(new ClusterName("_name"))
154+
.metadata(Metadata.builder().put(indexMetadata, true).build())
155+
.build();
156+
157+
List<Step> steps = action.toSteps(null, HOT_PHASE, nextStepKey);
158+
BranchingStep firstStep = (BranchingStep) steps.get(0);
159+
firstStep.performAction(indexMetadata.getIndex(), clusterState);
160+
161+
assertEquals(nextStepKey, firstStep.getNextStepKey());
155162
}
156163
}
157-
158164
}

‎x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java‎

Lines changed: 23 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.elasticsearch.xpack.core.ilm.LifecycleAction;
2929
import org.elasticsearch.xpack.core.ilm.LifecyclePolicy;
3030
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
31-
import org.elasticsearch.xpack.core.ilm.MigrateAction;
3231
import org.elasticsearch.xpack.core.ilm.Phase;
3332
import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
3433
import org.elasticsearch.xpack.core.ilm.RolloverAction;
@@ -40,7 +39,6 @@
4039

4140
import java.io.IOException;
4241
import java.io.InputStream;
43-
import java.time.ZonedDateTime;
4442
import java.util.HashMap;
4543
import java.util.List;
4644
import java.util.Locale;
@@ -154,8 +152,8 @@ public void testSearchableSnapshotForceMergesIndexToOneSegment() throws Exceptio
154152
TimeUnit.SECONDS);
155153
}
156154

157-
@SuppressWarnings("unchecked")
158-
public void testDeleteActionDeletesSearchableSnapshot() throws Exception {
155+
@SuppressWarnings("unchecked")
156+
public void testDeleteActionDeletesSearchableSnapshot() throws Exception {
159157
createSnapshotRepo(client(), snapshotRepo, randomBoolean());
160158

161159
// create policy with cold and delete phases
@@ -191,20 +189,20 @@ public void testDeleteActionDeletesSearchableSnapshot() throws Exception {
191189
assertBusy(() -> assertFalse(indexExists(restoredIndexName)), 60, TimeUnit.SECONDS);
192190

193191
assertTrue("the snapshot we generate in the cold phase should be deleted by the delete phase", waitUntil(() -> {
194-
try {
195-
Request getSnapshotsRequest = new Request("GET", "_snapshot/" + snapshotRepo + "/_all");
196-
Response getSnapshotsResponse = client().performRequest(getSnapshotsRequest);
197-
198-
Map<String, Object> responseMap;
199-
try (InputStream is = getSnapshotsResponse.getEntity().getContent()) {
200-
responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true);
201-
}
202-
List<Map<String, Object>> snapshots = (List<Map<String, Object>>) responseMap.get("snapshots");
203-
return snapshots.size() == 0;
204-
} catch (Exception e) {
205-
logger.error(e.getMessage(), e);
206-
return false;
207-
}
192+
try {
193+
Request getSnapshotsRequest = new Request("GET", "_snapshot/" + snapshotRepo + "/_all");
194+
Response getSnapshotsResponse = client().performRequest(getSnapshotsRequest);
195+
196+
Map<String, Object> responseMap;
197+
try (InputStream is = getSnapshotsResponse.getEntity().getContent()) {
198+
responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true);
199+
}
200+
List<Map<String, Object>> snapshots = (List<Map<String, Object>>) responseMap.get("snapshots");
201+
return snapshots.size() == 0;
202+
} catch (Exception e) {
203+
logger.error(e.getMessage(), e);
204+
return false;
205+
}
208206
}, 30, TimeUnit.SECONDS));
209207
}
210208

@@ -473,21 +471,15 @@ public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Except
473471
containsString("policy specifies [searchable_snapshot] action multiple times with differing repositories"));
474472
}
475473

476-
public void testSearchableSnapshotActionOverridesMigrateAction() throws Exception {
474+
public void testSearchableSnapshotsInHotPhasePinnedToHotNodes() throws Exception {
477475
createSnapshotRepo(client(), snapshotRepo, randomBoolean());
478476
createPolicy(client(), policy,
479477
new Phase("hot", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(RolloverAction.NAME,
480478
new RolloverAction(null, null, null, 1L),
481479
SearchableSnapshotAction.NAME, new SearchableSnapshotAction(
482480
snapshotRepo, randomBoolean()))
483481
),
484-
new Phase("warm", TimeValue.ZERO, org.elasticsearch.common.collect.Map.of(MigrateAction.NAME, new MigrateAction(true))),
485-
// this time transition condition will make sure we catch ILM in the warm phase so we can assert the warm migrate action
486-
// didn't re-configure the tier allocation settings set by the searchable_snapshot action in the hot phase
487-
// we'll use the origination date to kick off ILM to complete the policy
488-
new Phase("cold", TimeValue.timeValueDays(5L),
489-
org.elasticsearch.common.collect.Map.of(MigrateAction.NAME, new MigrateAction(true))),
490-
null, null
482+
null, null, null, null
491483
);
492484

493485
createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream,
@@ -510,25 +502,12 @@ snapshotRepo, randomBoolean()))
510502
logger.info("--> waiting for [{}] to exist...", restoredIndex);
511503
assertTrue(indexExists(restoredIndex));
512504
}, 30, TimeUnit.SECONDS);
513-
assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("warm").getKey())),
505+
assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("hot").getKey())),
514506
30, TimeUnit.SECONDS);
515507

516-
Map<String, Object> warmIndexSettings = getIndexSettingsAsMap(restoredIndex);
517-
// the warm phase shouldn't have changed the data_cold -> data_hot configuration
518-
assertThat(warmIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER),
519-
is("data_cold,data_warm,data_hot"));
520-
521-
// make the index 100 days old so the cold phase transition timing passes
522-
updateIndexSettings(restoredIndex, Settings.builder().put(LifecycleSettings.LIFECYCLE_ORIGINATION_DATE,
523-
ZonedDateTime.now().toInstant().toEpochMilli() - TimeValue.timeValueDays(100).getMillis()));
524-
525-
// let's wait for ILM to finish
526-
assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("cold").getKey())));
527-
528-
Map<String, Object> coldIndexSettings = getIndexSettingsAsMap(restoredIndex);
529-
// the frozen phase should've reconfigured the allocation preference
530-
assertThat(coldIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER),
531-
is("data_cold,data_warm,data_hot"));
508+
Map<String, Object> hotIndexSettings = getIndexSettingsAsMap(restoredIndex);
509+
// searchable snapshots mounted in the hot phase should be pinned to hot nodes
510+
assertThat(hotIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER),
511+
is("data_hot"));
532512
}
533-
534513
}

0 commit comments

Comments
 (0)