From fa5f551b8034da55b30a736d555c9d3777b97e6d Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Thu, 29 Nov 2018 11:11:20 -0800 Subject: [PATCH] [ILM] make alias swapping atomic (#35972) In the current implementation, there is a time between the ShrinkStep and the ShrinkSetAliasStep that both the source and target indices will be present with the same aliases. This means that queries to during this time will query both and return duplicates. This fixes that scenario by moving the alias inheritance to the same aliases update request as is done in ShrinkSetAliasStep --- .../indexlifecycle/ShrinkSetAliasStep.java | 18 ++++++++++--- .../xpack/core/indexlifecycle/ShrinkStep.java | 5 ---- .../ShrinkSetAliasStepTests.java | 25 ++++++++++++++++--- .../core/indexlifecycle/ShrinkStepTests.java | 3 +-- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStep.java index 52fc955a232..da59b16ded9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStep.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import java.util.Objects; @@ -36,11 +37,20 @@ public class ShrinkSetAliasStep extends AsyncActionStep { String index = indexMetaData.getIndex().getName(); // get target shrink index String targetIndexName = shrunkIndexPrefix + index; - IndicesAliasesRequest aliasesRequest = new IndicesAliasesRequest() .addAliasAction(IndicesAliasesRequest.AliasActions.removeIndex().index(index)) .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(targetIndexName).alias(index)); - + // copy over other aliases from original index + indexMetaData.getAliases().values().spliterator().forEachRemaining(aliasMetaDataObjectCursor -> { + AliasMetaData aliasMetaDataToAdd = aliasMetaDataObjectCursor.value; + // inherit all alias properties except `is_write_index` + aliasesRequest.addAliasAction(IndicesAliasesRequest.AliasActions.add() + .index(targetIndexName).alias(aliasMetaDataToAdd.alias()) + .indexRouting(aliasMetaDataToAdd.indexRouting()) + .searchRouting(aliasMetaDataToAdd.searchRouting()) + .filter(aliasMetaDataToAdd.filter() == null ? null : aliasMetaDataToAdd.filter().string()) + .writeIndex(null)); + }); getClient().admin().indices().aliases(aliasesRequest, ActionListener.wrap(response -> listener.onResponse(true), listener::onFailure)); } @@ -54,7 +64,7 @@ public class ShrinkSetAliasStep extends AsyncActionStep { public int hashCode() { return Objects.hash(super.hashCode(), shrunkIndexPrefix); } - + @Override public boolean equals(Object obj) { if (obj == null) { @@ -64,7 +74,7 @@ public class ShrinkSetAliasStep extends AsyncActionStep { return false; } ShrinkSetAliasStep other = (ShrinkSetAliasStep) obj; - return super.equals(obj) && + return super.equals(obj) && Objects.equals(shrunkIndexPrefix, other.shrunkIndexPrefix); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java index 4847cd07a20..19ddbfe04cf 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.core.indexlifecycle; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; @@ -58,13 +57,9 @@ public class ShrinkStep extends AsyncActionStep { String shrunkenIndexName = shrunkIndexPrefix + indexMetaData.getIndex().getName(); ResizeRequest resizeRequest = new ResizeRequest(shrunkenIndexName, indexMetaData.getIndex().getName()); - indexMetaData.getAliases().values().spliterator().forEachRemaining(aliasMetaDataObjectCursor -> { - resizeRequest.getTargetIndexRequest().alias(new Alias(aliasMetaDataObjectCursor.value.alias())); - }); resizeRequest.getTargetIndexRequest().settings(relevantTargetSettings); getClient().admin().indices().resizeIndex(resizeRequest, ActionListener.wrap(response -> { - // TODO(talevy): when is this not acknowledged? listener.onResponse(response.isAcknowledged()); }, listener::onFailure)); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStepTests.java index 5fcfcdeea38..a5c0e4d7146 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkSetAliasStepTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.AdminClient; import org.elasticsearch.client.Client; import org.elasticsearch.client.IndicesAdminClient; +import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.xpack.core.indexlifecycle.AsyncActionStep.Listener; import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey; @@ -71,15 +72,33 @@ public class ShrinkSetAliasStepTests extends AbstractStepTestCase expectedAliasActions = Arrays.asList( IndicesAliasesRequest.AliasActions.removeIndex().index(sourceIndex), - IndicesAliasesRequest.AliasActions.add().index(shrunkenIndex).alias(sourceIndex)); + IndicesAliasesRequest.AliasActions.add().index(shrunkenIndex).alias(sourceIndex), + IndicesAliasesRequest.AliasActions.add().index(shrunkenIndex).alias(aliasMetaData.alias()) + .searchRouting(aliasMetaData.searchRouting()).indexRouting(aliasMetaData.indexRouting()) + .filter(aliasMetaDataFilter).writeIndex(null)); AdminClient adminClient = Mockito.mock(AdminClient.class); IndicesAdminClient indicesClient = Mockito.mock(IndicesAdminClient.class); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java index 472c22025e1..0cd655cb9d6 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.core.indexlifecycle; import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.rollover.RolloverResponse; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeResponse; @@ -112,7 +111,7 @@ public class ShrinkStepTests extends AbstractStepTestCase { @SuppressWarnings("unchecked") ActionListener listener = (ActionListener) invocation.getArguments()[1]; assertThat(request.getSourceIndex(), equalTo(sourceIndexMetaData.getIndex().getName())); - assertThat(request.getTargetIndexRequest().aliases(), equalTo(Collections.singleton(new Alias("my_alias")))); + assertThat(request.getTargetIndexRequest().aliases(), equalTo(Collections.emptySet())); assertThat(request.getTargetIndexRequest().settings(), equalTo(Settings.builder() .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, step.getNumberOfShards()) .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, sourceIndexMetaData.getNumberOfReplicas())