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.

Sign up for GitHub

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

Avoid race condition in ILMHistorySotre #53039

Merged
merged 4 commits into from
Mar 4, 2020
Merged

Avoid race condition in ILMHistorySotre #53039

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ccc6d16
Avoid race condition in ILMHistorySotre
probakowski Mar 2, 2020
ad432c9
Merge branch 'master' into fix-ilmhistorystore
elasticmachine Mar 3, 2020
7a86390
Review comment
probakowski Mar 3, 2020
acde520
Merge branch 'master' into fix-ilmhistorystore
elasticmachine Mar 3, 2020
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
58 changes: 8 additions & 50 deletions ...qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1358,10 +1358,7 @@ public void testWaitForActiveShardsStep() throws Exception {
assertBusy(() -> assertThat(getStepKeyForIndex(originalIndex), equalTo(PhaseCompleteStep.finalStep("hot").getKey())));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353")
public void testHistoryIsWrittenWithSuccess() throws Exception {
String index = "success-index";

createNewSingletonPolicy("hot", new RolloverAction(null, null, 1L));
Request createIndexTemplate = new Request("PUT", "_template/rolling_indexes");
createIndexTemplate.setJsonEntity("{" +
Expand All @@ -1375,10 +1372,7 @@ public void testHistoryIsWrittenWithSuccess() throws Exception {
"}");
client().performRequest(createIndexTemplate);

createIndexWithSettings(index + "-1",
Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0),
true);
createIndexWithSettings(index + "-1", Settings.builder(), true);

// Index a document
index(client(), index + "-1", "1", "foo", "bar");
Expand All @@ -1396,69 +1390,34 @@ public void testHistoryIsWrittenWithSuccess() throws Exception {
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "wait-for-yellow-step"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "check-rollover-ready"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "attempt-rollover"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "update-rollover-lifecycle-date"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "set-indexing-complete"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "completed"), 30, TimeUnit.SECONDS);
assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", true, "complete"), 30, TimeUnit.SECONDS);

assertBusy(() -> assertHistoryIsPresent(policy, index + "-000002", true, "check-rollover-ready"), 30, TimeUnit.SECONDS);
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353")
public void testHistoryIsWrittenWithFailure() throws Exception {
String index = "failure-index";

createIndexWithSettings(index + "-1", Settings.builder(), false);
createNewSingletonPolicy("hot", new RolloverAction(null, null, 1L));
Request createIndexTemplate = new Request("PUT", "_template/rolling_indexes");
createIndexTemplate.setJsonEntity("{" +
"\"index_patterns\": [\""+ index + "-*\"], \n" +
" \"settings\": {\n" +
" \"number_of_shards\": 1,\n" +
" \"number_of_replicas\": 0,\n" +
" \"index.lifecycle.name\": \"" + policy+ "\"\n" +
" }\n" +
"}");
client().performRequest(createIndexTemplate);

createIndexWithSettings(index + "-1",
Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0),
false);
updatePolicy(index + "-1", policy);

// Index a document
index(client(), index + "-1", "1", "foo", "bar");
Request refreshIndex = new Request("POST", "/" + index + "-1/_refresh");
client().performRequest(refreshIndex);

assertBusy(() -> assertThat(getStepKeyForIndex(index + "-1").getName(), equalTo(ErrorStep.NAME)));
assertBusy(() -> assertThat(getStepKeyForIndex(index + "-1").getName(), equalTo(ErrorStep.NAME)), 30, TimeUnit.SECONDS);

assertBusy(() -> assertHistoryIsPresent(policy, index + "-1", false, "ERROR"), 30, TimeUnit.SECONDS);
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/50353")
public void testHistoryIsWrittenWithDeletion() throws Exception {
String index = "delete-index";

createNewSingletonPolicy("delete", new DeleteAction());
Request createIndexTemplate = new Request("PUT", "_template/delete_indexes");
createIndexTemplate.setJsonEntity("{" +
"\"index_patterns\": [\""+ index + "\"], \n" +
" \"settings\": {\n" +
" \"number_of_shards\": 1,\n" +
" \"number_of_replicas\": 0,\n" +
" \"index.lifecycle.name\": \"" + policy+ "\"\n" +
" }\n" +
"}");
client().performRequest(createIndexTemplate);

// Index should be created and then deleted by ILM
createIndexWithSettings(index, Settings.builder(), false);
createNewSingletonPolicy("delete", new DeleteAction());
updatePolicy(index, policy);

assertBusy(() -> {
logger.info("--> checking for index deletion...");
Request existCheck = new Request("HEAD", "/" + index);
Response resp = client().performRequest(existCheck);
assertThat(resp.getStatusLine().getStatusCode(), equalTo(404));
});
assertBusy(() -> assertFalse(indexExists(index)));

assertBusy(() -> {
assertHistoryIsPresent(policy, index, true, "delete", "delete", "wait-for-shard-history-leases");
Expand Down Expand Up @@ -1593,7 +1552,6 @@ private void assertHistoryIsPresent(String policyName, String indexName, boolean
// This method should be called inside an assertBusy, it has no retry logic of its own
private void assertHistoryIsPresent(String policyName, String indexName, boolean success,
@Nullable String phase, @Nullable String action, String stepName) throws IOException {
assertOK(client().performRequest(new Request("POST", indexName + "/_refresh")));
logger.info("--> checking for history item [{}], [{}], success: [{}], phase: [{}], action: [{}], step: [{}]",
policyName, indexName, success, phase, action, stepName);
final Request historySearchRequest = new Request("GET", "ilm-history*/_search?expand_wildcards=all");
Expand Down
19 changes: 16 additions & 3 deletions x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.threadpool.ThreadPool;

import java.io.Closeable;
Expand All @@ -46,6 +49,7 @@
import static org.elasticsearch.xpack.core.ClientHelper.INDEX_LIFECYCLE_ORIGIN;
import static org.elasticsearch.xpack.core.ilm.LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING;
import static org.elasticsearch.xpack.ilm.history.ILMHistoryTemplateRegistry.INDEX_TEMPLATE_VERSION;
import static org.elasticsearch.xpack.ilm.history.ILMHistoryTemplateRegistry.TEMPLATE_ILM_HISTORY;

/**
* The {@link ILMHistoryStore} handles indexing {@link ILMHistoryItem} documents into the
Expand Down Expand Up @@ -175,6 +179,7 @@ public void putAsync(ILMHistoryItem item) {
* @param listener Called after the index has been created. `onResponse` called with `true` if the index was created,
* `false` if it already existed.
*/
@SuppressWarnings("unchecked")
static void ensureHistoryIndex(Client client, ClusterState state, ActionListener<Boolean> listener) {
final String initialHistoryIndexName = ILM_HISTORY_INDEX_PREFIX + "000001";
final AliasOrIndex ilmHistory = state.metaData().getAliasAndIndexLookup().get(ILM_HISTORY_ALIAS);
Expand All @@ -183,11 +188,19 @@ static void ensureHistoryIndex(Client client, ClusterState state, ActionListener
if (ilmHistory == null && initialHistoryIndex == null) {
// No alias or index exists with the expected names, so create the index with appropriate alias
logger.debug("creating ILM history index [{}]", initialHistoryIndexName);

// Template below should be already defined as real index template but it can be deleted. To avoid race condition with its
// recreation we apply settings and mappings ourselves
byte[] templateBytes = TEMPLATE_ILM_HISTORY.loadBytes();
Map<String, Object> map = XContentHelper.convertToMap(new BytesArray(templateBytes, 0, templateBytes.length),
probakowski marked this conversation as resolved.
Show resolved Hide resolved
false, XContentType.JSON).v2();

client.admin().indices().prepareCreate(initialHistoryIndexName)
.setSettings((Map<String, ?>) map.get("settings"))
.setMapping((Map<String, Object>) map.get("mappings"))
.setWaitForActiveShards(1)
.addAlias(new Alias(ILM_HISTORY_ALIAS)
.writeIndex(true))
.execute(new ActionListener<CreateIndexResponse>() {
.addAlias(new Alias(ILM_HISTORY_ALIAS).writeIndex(true))
.execute(new ActionListener<>() {
@Override
public void onResponse(CreateIndexResponse response) {
listener.onResponse(true);
Expand Down