From 0dc2ec3ec86728bebaa136f2e951cdf93ed0a00d Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 28 May 2017 12:40:27 +0300 Subject: [PATCH 1/5] rollover condition uses doc count from primary #24217 --- .../rollover/TransportRolloverAction.java | 2 +- .../indices.rollover/20_max_doc_condition.yml | 80 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index 3796529b8590b..7eb9d14b7ce4a 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -119,7 +119,7 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus @Override public void onResponse(IndicesStatsResponse statsResponse) { final Set conditionResults = evaluateConditions(rolloverRequest.getConditions(), - statsResponse.getTotal().getDocs(), metaData.index(sourceIndexName)); + statsResponse.getPrimaries().getDocs(), metaData.index(sourceIndexName)); if (rolloverRequest.isDryRun()) { listener.onResponse( diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml new file mode 100644 index 0000000000000..55d7889c2582b --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml @@ -0,0 +1,80 @@ +--- +"Rollover conditions matched with replica node": + # create index with alias and replica + - do: + indices.create: + index: logs-1 + wait_for_active_shards: all + body: + aliases: + logs_search: {} + settings: + index: + number_of_replicas: "1" + + # perform alias rollover on empty index + - do: + indices.rollover: + alias: "logs_search" + wait_for_active_shards: 1 + body: + conditions: + max_docs: 2 + + - match: { conditions: { "[max_docs: 2]": false } } + - match: { rolled_over: false } + + # index first document and wait for refresh + - do: + index: + index: logs-1 + type: test + id: "1" + body: { "foo": "hello world" } + refresh: true + + - do: + indices.stats: {} + + - match: { _all.primaries.docs.count: 1 } + - match: { _all.total.docs.count: 2 } + + # perform alias rollover with no result + - do: + indices.rollover: + alias: "logs_search" + wait_for_active_shards: 1 + body: + conditions: + max_docs: 2 + + - match: { conditions: { "[max_docs: 2]": false } } + - match: { rolled_over: false } + + # index second document and wait for refresh + - do: + index: + index: logs-1 + type: test + id: "2" + body: { "foo": "hello world" } + refresh: true + + - do: + indices.stats: {} + + - match: { _all.primaries.docs.count: 2 } + - match: { _all.total.docs.count: 4 } + + # perform alias rollover + - do: + indices.rollover: + alias: "logs_search" + wait_for_active_shards: 1 + body: + conditions: + max_docs: 2 + + - match: { conditions: { "[max_docs: 2]": true } } + - match: { rolled_over: true } + From a2ec8d60f05c098eb6e3a02edcfbd847ac6df63c Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 1 Jun 2017 16:09:59 +0300 Subject: [PATCH 2/5] unit test for max_docs condition evaluation in rollover action --- .../rollover/TransportRolloverAction.java | 7 ++- .../TransportRolloverActionTests.java | 59 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index 7eb9d14b7ce4a..2abe0dad74ee3 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -119,7 +119,7 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus @Override public void onResponse(IndicesStatsResponse statsResponse) { final Set conditionResults = evaluateConditions(rolloverRequest.getConditions(), - statsResponse.getPrimaries().getDocs(), metaData.index(sourceIndexName)); + metaData.index(sourceIndexName), statsResponse); if (rolloverRequest.isDryRun()) { listener.onResponse( @@ -201,6 +201,11 @@ static Set evaluateConditions(final Set conditions, .collect(Collectors.toSet()); } + static Set evaluateConditions(final Set conditions, final IndexMetaData metaData, + final IndicesStatsResponse statsResponse) { + return evaluateConditions(conditions, statsResponse.getPrimaries().getDocs(), metaData); + } + static void validate(MetaData metaData, RolloverRequest request) { final AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(request.getAlias()); if (aliasOrIndex == null) { diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java index 9d62bd825f3d5..34b4f073f5fd0 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -22,6 +22,8 @@ import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; +import org.elasticsearch.action.admin.indices.stats.CommonStats; +import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.cluster.metadata.AliasAction; import org.elasticsearch.cluster.metadata.AliasMetaData; @@ -43,9 +45,39 @@ import static org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.evaluateConditions; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class TransportRolloverActionTests extends ESTestCase { + public void testEvaluateMaxDocsConditionsSatisfied() throws Exception { + long maxDocs = 100; + long docsInPrimaryShards = 100; + long docsInShards = 200; + + final Condition.Result result = evaluateConditions( + Sets.newHashSet(new MaxDocsCondition(maxDocs)), + createMetaData(), + createIndecesStatResponse(docsInShards, docsInPrimaryShards) + ).iterator().next(); + + assertTrue(result.matched); + } + + public void testEvaluateMaxDocsConditionsNotSatisfied() throws Exception { + long maxDocs = 100; + long docsInPrimaryShards = 50; + long docsInShards = 100; + + final Condition.Result result = evaluateConditions( + Sets.newHashSet(new MaxDocsCondition(maxDocs)), + createMetaData(), + createIndecesStatResponse(docsInShards, docsInPrimaryShards) + ).iterator().next(); + + assertFalse(result.matched); + } + public void testEvaluateConditions() throws Exception { MaxDocsCondition maxDocsCondition = new MaxDocsCondition(100L); MaxAgeCondition maxAgeCondition = new MaxAgeCondition(TimeValue.timeValueHours(2)); @@ -190,4 +222,31 @@ public void testCreateIndexRequest() throws Exception { assertThat(createIndexRequest.index(), equalTo(rolloverIndex)); assertThat(createIndexRequest.cause(), equalTo("rollover_index")); } + + private IndicesStatsResponse createIndecesStatResponse(long totalDocs, long primaryDocs) { + final CommonStats primaryStats = mock(CommonStats.class); + when(primaryStats.getDocs()).thenReturn(new DocsStats(primaryDocs, 0)); + + final CommonStats totalStats = mock(CommonStats.class); + when(totalStats.getDocs()).thenReturn(new DocsStats(totalDocs, 0)); + + final IndicesStatsResponse response = mock(IndicesStatsResponse.class); + when(response.getPrimaries()).thenReturn(primaryStats); + when(response.getTotal()).thenReturn(totalStats); + + return response; + } + + private IndexMetaData createMetaData() { + final Settings settings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()) + .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .build(); + return IndexMetaData.builder(randomAlphaOfLength(10)) + .creationDate(System.currentTimeMillis() - TimeValue.timeValueHours(3).getMillis()) + .settings(settings) + .build(); + } } From 92c262224974e91a12d78b9d03061a382e70e6a2 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Wed, 7 Jun 2017 11:39:10 +0300 Subject: [PATCH 3/5] remove duplication in condition testing in TransportRolloverActionTests --- .../TransportRolloverActionTests.java | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java index 34b4f073f5fd0..d33987c92adbd 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -42,40 +42,28 @@ import java.util.Locale; import java.util.Set; +import org.mockito.ArgumentCaptor; import static org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.evaluateConditions; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; + public class TransportRolloverActionTests extends ESTestCase { - public void testEvaluateMaxDocsConditionsSatisfied() throws Exception { - long maxDocs = 100; + public void testDocStatsSelectionFromPrimariesOnly() throws Exception { long docsInPrimaryShards = 100; long docsInShards = 200; - final Condition.Result result = evaluateConditions( - Sets.newHashSet(new MaxDocsCondition(maxDocs)), - createMetaData(), - createIndecesStatResponse(docsInShards, docsInPrimaryShards) - ).iterator().next(); + final Condition condition = createTestCondition(); + evaluateConditions(Sets.newHashSet(condition), createMetaData(), createIndecesStatResponse(docsInShards, docsInPrimaryShards)); + final ArgumentCaptor argument = ArgumentCaptor.forClass(Condition.Stats.class); + verify(condition).evaluate(argument.capture()); - assertTrue(result.matched); - } - - public void testEvaluateMaxDocsConditionsNotSatisfied() throws Exception { - long maxDocs = 100; - long docsInPrimaryShards = 50; - long docsInShards = 100; - - final Condition.Result result = evaluateConditions( - Sets.newHashSet(new MaxDocsCondition(maxDocs)), - createMetaData(), - createIndecesStatResponse(docsInShards, docsInPrimaryShards) - ).iterator().next(); - - assertFalse(result.matched); + assertEquals(docsInPrimaryShards, argument.getValue().numDocs); } public void testEvaluateConditions() throws Exception { @@ -249,4 +237,10 @@ private IndexMetaData createMetaData() { .settings(settings) .build(); } + + private Condition createTestCondition() { + final Condition condition = mock(Condition.class); + when(condition.evaluate(any())).thenReturn(new Condition.Result(condition, true)); + return condition; + } } From f8137fbfcaac5ac63c113f2af35b6be9f5e58600 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 8 Jun 2017 19:15:08 +0300 Subject: [PATCH 4/5] make max_doc condition index rollover yml test pass for both 1 node and multi nodes --- .../indices.rollover/20_max_doc_condition.yml | 29 +------------------ 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml index 55d7889c2582b..033a509c576da 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml @@ -4,25 +4,10 @@ - do: indices.create: index: logs-1 - wait_for_active_shards: all + wait_for_active_shards: 1 body: aliases: logs_search: {} - settings: - index: - number_of_replicas: "1" - - # perform alias rollover on empty index - - do: - indices.rollover: - alias: "logs_search" - wait_for_active_shards: 1 - body: - conditions: - max_docs: 2 - - - match: { conditions: { "[max_docs: 2]": false } } - - match: { rolled_over: false } # index first document and wait for refresh - do: @@ -33,12 +18,6 @@ body: { "foo": "hello world" } refresh: true - - do: - indices.stats: {} - - - match: { _all.primaries.docs.count: 1 } - - match: { _all.total.docs.count: 2 } - # perform alias rollover with no result - do: indices.rollover: @@ -60,12 +39,6 @@ body: { "foo": "hello world" } refresh: true - - do: - indices.stats: {} - - - match: { _all.primaries.docs.count: 2 } - - match: { _all.total.docs.count: 4 } - # perform alias rollover - do: indices.rollover: From 11b11b13119446e071627565025d51967fef52be Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 11 Jun 2017 08:21:04 +0300 Subject: [PATCH 5/5] skip index rollover test with max_docs condition for previous versions of ES --- .../test/indices.rollover/20_max_doc_condition.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml index 033a509c576da..5f42206f6d50e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.rollover/20_max_doc_condition.yml @@ -1,5 +1,9 @@ --- -"Rollover conditions matched with replica node": +"Max docs rollover conditions matches only primary shards": + - skip: + version: "- 5.6.1" + reason: "matching docs changed from all shards to primary shards" + # create index with alias and replica - do: indices.create: