From b9331d3dcbca34f0262a7dbb624d20e97a7efc66 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 8 Jan 2019 13:07:23 +0100 Subject: [PATCH 1/6] [CCR] Follow stats api should return a 404 when requesting stats for non existing indices. Currently it returns an empty response with a 200 response code. Closes #37021 --- .../resources/rest-api-spec/test/ccr/follow_stats.yml | 10 ++++++++++ .../xpack/ccr/action/TransportFollowStatsAction.java | 11 ++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/ccr/qa/rest/src/test/resources/rest-api-spec/test/ccr/follow_stats.yml b/x-pack/plugin/ccr/qa/rest/src/test/resources/rest-api-spec/test/ccr/follow_stats.yml index aa63c804aba21..5b3e6c18ef29b 100644 --- a/x-pack/plugin/ccr/qa/rest/src/test/resources/rest-api-spec/test/ccr/follow_stats.yml +++ b/x-pack/plugin/ccr/qa/rest/src/test/resources/rest-api-spec/test/ccr/follow_stats.yml @@ -43,6 +43,12 @@ - is_true: follow_index_shards_acked - is_true: index_following_started + - do: + ccr.follow_stats: + index: _all + - length: { indices: 1 } + - match: { indices.0.index: "bar" } + # we can not reliably wait for replication to occur so we test the endpoint without indexing any documents - do: ccr.follow_stats: @@ -77,3 +83,7 @@ index: bar - is_true: acknowledged + - do: + catch: missing + ccr.follow_stats: + index: unknown diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java index 8ab66aec8e80b..8a3a16afb33a5 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java @@ -10,8 +10,10 @@ import org.elasticsearch.action.FailedNodeException; import org.elasticsearch.action.TaskOperationFailure; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.tasks.TransportTasksAction; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.license.LicenseUtils; @@ -37,13 +39,15 @@ public class TransportFollowStatsAction extends TransportTasksAction< FollowStatsAction.StatsResponses, FollowStatsAction.StatsResponse> { private final CcrLicenseChecker ccrLicenseChecker; + private final IndexNameExpressionResolver resolver; @Inject public TransportFollowStatsAction( final ClusterService clusterService, final TransportService transportService, final ActionFilters actionFilters, - final CcrLicenseChecker ccrLicenseChecker) { + final CcrLicenseChecker ccrLicenseChecker, + final IndexNameExpressionResolver resolver) { super( FollowStatsAction.NAME, clusterService, @@ -54,6 +58,7 @@ public TransportFollowStatsAction( FollowStatsAction.StatsResponse::new, Ccr.CCR_THREAD_POOL_NAME); this.ccrLicenseChecker = Objects.requireNonNull(ccrLicenseChecker); + this.resolver = resolver; } @Override @@ -65,6 +70,10 @@ protected void doExecute( listener.onFailure(LicenseUtils.newComplianceException("ccr")); return; } + + String[] concreteIndices = + resolver.concreteIndexNames(clusterService.state(), IndicesOptions.strictExpandOpenAndForbidClosed(), request.indices()); + request.setIndices(concreteIndices); super.doExecute(task, request, listener); } From 81f2e325ef970625d0ed4d24e9a70573238ed4d1 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 14 Jan 2019 11:50:07 +0100 Subject: [PATCH 2/6] avoid 404 when querying for stats --- .../documentation/CCRDocumentationIT.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CCRDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CCRDocumentationIT.java index b05c7a0dde368..1f6373aff6a8c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CCRDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CCRDocumentationIT.java @@ -630,6 +630,22 @@ public void onFailure(Exception e) { public void testGetFollowStats() throws Exception { RestHighLevelClient client = highLevelClient(); + { + // Create leader index: + CreateIndexRequest createIndexRequest = new CreateIndexRequest("leader"); + createIndexRequest.settings(Collections.singletonMap("index.soft_deletes.enabled", true)); + CreateIndexResponse response = client.indices().create(createIndexRequest, RequestOptions.DEFAULT); + assertThat(response.isAcknowledged(), is(true)); + } + { + // Follow index, so that we can query for follow stats: + PutFollowRequest putFollowRequest = new PutFollowRequest("local", "leader", "follower"); + PutFollowResponse putFollowResponse = client.ccr().putFollow(putFollowRequest, RequestOptions.DEFAULT); + assertThat(putFollowResponse.isFollowIndexCreated(), is(true)); + assertThat(putFollowResponse.isFollowIndexShardsAcked(), is(true)); + assertThat(putFollowResponse.isIndexFollowingStarted(), is(true)); + } + // tag::ccr-get-follow-stats-request FollowStatsRequest request = new FollowStatsRequest("follower"); // <1> @@ -671,6 +687,12 @@ public void onFailure(Exception e) { // end::ccr-get-follow-stats-execute-async assertTrue(latch.await(30L, TimeUnit.SECONDS)); + + { + PauseFollowRequest pauseFollowRequest = new PauseFollowRequest("follower"); + AcknowledgedResponse pauseFollowResponse = client.ccr().pauseFollow(pauseFollowRequest, RequestOptions.DEFAULT); + assertThat(pauseFollowResponse.isAcknowledged(), is(true)); + } } static Map toMap(Response response) throws IOException { From e99ccfb8c76c9ef8ef1f1912800bf12dfe242fde Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 14 Jan 2019 12:45:08 +0100 Subject: [PATCH 3/6] take into account that there may be shard follow tasks where the follower index has been removed. --- .../action/TransportFollowStatsAction.java | 61 ++++++++++----- .../xpack/ccr/action/FollowStatsIT.java | 74 +++++++++++++++++++ .../TransportFollowStatsActionTests.java | 66 +++++++++++++++++ 3 files changed, 183 insertions(+), 18 deletions(-) create mode 100644 x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsActionTests.java diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java index 8a3a16afb33a5..674e4356bd9ee 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java @@ -15,7 +15,9 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.tasks.Task; @@ -71,9 +73,28 @@ protected void doExecute( return; } - String[] concreteIndices = - resolver.concreteIndexNames(clusterService.state(), IndicesOptions.strictExpandOpenAndForbidClosed(), request.indices()); - request.setIndices(concreteIndices); + final ClusterState state = clusterService.state(); + try { + String[] concreteIndices = resolver.concreteIndexNames(state, IndicesOptions.strictExpand(), request.indices()); + + // Also include matching shard follow task's follower indices: + // A follower index may be removed and therefor concreteIndexNames(...) does not include it. + // Shard follow tasks for these removed follower indices do exist (with a fatal error set). + Set shardFollowTaskFollowerIndices = new HashSet<>(Arrays.asList(concreteIndices)); + shardFollowTaskFollowerIndices.addAll(findFollowerIndicesFromShardFollowTasks(state, request.indices())); + + request.setIndices(shardFollowTaskFollowerIndices.toArray(new String[0])); + } catch (IndexNotFoundException e) { + // It is possible that there are failed shard follow tasks (with fatal error set) for a none existing index + // (in case the follower index has been removed) + // we should also include the stats for these shard follow tasks: + final Set followerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); + if (followerIndices.size() != 0) { + request.setIndices(followerIndices.toArray(new String[0])); + } else { + throw e; + } + } super.doExecute(task, request, listener); } @@ -89,21 +110,7 @@ protected FollowStatsAction.StatsResponses newResponse( @Override protected void processTasks(final FollowStatsAction.StatsRequest request, final Consumer operation) { final ClusterState state = clusterService.state(); - final PersistentTasksCustomMetaData persistentTasksMetaData = state.metaData().custom(PersistentTasksCustomMetaData.TYPE); - if (persistentTasksMetaData == null) { - return; - } - - final Set requestedFollowerIndices = request.indices() != null ? - new HashSet<>(Arrays.asList(request.indices())) : Collections.emptySet(); - final Set followerIndices = persistentTasksMetaData.tasks().stream() - .filter(persistentTask -> persistentTask.getTaskName().equals(ShardFollowTask.NAME)) - .map(persistentTask -> { - ShardFollowTask shardFollowTask = (ShardFollowTask) persistentTask.getParams(); - return shardFollowTask.getFollowShardId().getIndexName(); - }) - .filter(followerIndex -> requestedFollowerIndices.isEmpty() || requestedFollowerIndices.contains(followerIndex)) - .collect(Collectors.toSet()); + final Set followerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); for (final Task task : taskManager.getTasks().values()) { if (task instanceof ShardFollowNodeTask) { @@ -123,4 +130,22 @@ protected void taskOperation( listener.onResponse(new FollowStatsAction.StatsResponse(task.getStatus())); } + static Set findFollowerIndicesFromShardFollowTasks(ClusterState state, String[] indices) { + final PersistentTasksCustomMetaData persistentTasksMetaData = state.metaData().custom(PersistentTasksCustomMetaData.TYPE); + if (persistentTasksMetaData == null) { + return Collections.emptySet(); + } + + final Set requestedFollowerIndices = indices != null ? + new HashSet<>(Arrays.asList(indices)) : Collections.emptySet(); + return persistentTasksMetaData.tasks().stream() + .filter(persistentTask -> persistentTask.getTaskName().equals(ShardFollowTask.NAME)) + .map(persistentTask -> { + ShardFollowTask shardFollowTask = (ShardFollowTask) persistentTask.getParams(); + return shardFollowTask.getFollowShardId().getIndexName(); + }) + .filter(followerIndex -> Strings.isAllOrWildcard(indices) || requestedFollowerIndices.contains(followerIndex)) + .collect(Collectors.toSet()); + } + } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java index bf6f080099088..b2f38202607ea 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java @@ -9,6 +9,8 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; +import org.elasticsearch.action.admin.indices.close.CloseIndexRequest; +import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; @@ -116,4 +118,76 @@ public void testFollowStatsApiFollowerIndexFiltering() throws Exception { }); } + public void testFollowStatsApiIncludeShardFollowStatsWithRemovedFollowerIndex() throws Exception { + final String leaderIndexSettings = getIndexSettings(1, 0, + singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")); + assertAcked(client().admin().indices().prepareCreate("leader1").setSource(leaderIndexSettings, XContentType.JSON)); + ensureGreen("leader1"); + + PutFollowAction.Request followRequest = getPutFollowRequest("leader1", "follower1"); + client().execute(PutFollowAction.INSTANCE, followRequest).get(); + + FollowStatsAction.StatsRequest statsRequest = new FollowStatsAction.StatsRequest(); + FollowStatsAction.StatsResponses response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + statsRequest = new FollowStatsAction.StatsRequest(); + statsRequest.setIndices(new String[] {"follower1"}); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + assertAcked(client().admin().indices().delete(new DeleteIndexRequest("follower1")).actionGet()); + + statsRequest = new FollowStatsAction.StatsRequest(); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + statsRequest = new FollowStatsAction.StatsRequest(); + statsRequest.setIndices(new String[] {"follower1"}); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + assertAcked(client().execute(PauseFollowAction.INSTANCE, new PauseFollowAction.Request("follower1")).actionGet()); + } + + public void testFollowStatsApiIncludeShardFollowStatsWithClosedFollowerIndex() throws Exception { + final String leaderIndexSettings = getIndexSettings(1, 0, + singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")); + assertAcked(client().admin().indices().prepareCreate("leader1").setSource(leaderIndexSettings, XContentType.JSON)); + ensureGreen("leader1"); + + PutFollowAction.Request followRequest = getPutFollowRequest("leader1", "follower1"); + client().execute(PutFollowAction.INSTANCE, followRequest).get(); + + FollowStatsAction.StatsRequest statsRequest = new FollowStatsAction.StatsRequest(); + FollowStatsAction.StatsResponses response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + statsRequest = new FollowStatsAction.StatsRequest(); + statsRequest.setIndices(new String[] {"follower1"}); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + assertAcked(client().admin().indices().close(new CloseIndexRequest("follower1")).actionGet()); + + statsRequest = new FollowStatsAction.StatsRequest(); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + statsRequest = new FollowStatsAction.StatsRequest(); + statsRequest.setIndices(new String[] {"follower1"}); + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + assertAcked(client().execute(PauseFollowAction.INSTANCE, new PauseFollowAction.Request("follower1")).actionGet()); + } + } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsActionTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsActionTests.java new file mode 100644 index 0000000000000..bc8c58f1de7de --- /dev/null +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsActionTests.java @@ -0,0 +1,66 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.ccr.action; + +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.persistent.PersistentTasksCustomMetaData; +import org.elasticsearch.test.ESTestCase; + +import java.util.Collections; +import java.util.Set; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class TransportFollowStatsActionTests extends ESTestCase { + + public void testFindFollowerIndicesFromShardFollowTasks() { + PersistentTasksCustomMetaData.Builder persistentTasks = PersistentTasksCustomMetaData.builder() + .addTask("1", ShardFollowTask.NAME, createShardFollowTask("abc"), null) + .addTask("2", ShardFollowTask.NAME, createShardFollowTask("def"), null); + + ClusterState clusterState = ClusterState.builder(new ClusterName("_cluster")) + .metaData(MetaData.builder().putCustom(PersistentTasksCustomMetaData.TYPE, persistentTasks.build()).build()) + .build(); + Set result = TransportFollowStatsAction.findFollowerIndicesFromShardFollowTasks(clusterState, null); + assertThat(result.size(), equalTo(2)); + assertThat(result.contains("abc"), is(true)); + assertThat(result.contains("def"), is(true)); + + result = TransportFollowStatsAction.findFollowerIndicesFromShardFollowTasks(clusterState, new String[]{"def"}); + assertThat(result.size(), equalTo(1)); + assertThat(result.contains("def"), is(true)); + + result = TransportFollowStatsAction.findFollowerIndicesFromShardFollowTasks(clusterState, new String[]{"ghi"}); + assertThat(result.size(), equalTo(0)); + } + + private static ShardFollowTask createShardFollowTask(String followerIndex) { + return new ShardFollowTask( + null, + new ShardId(followerIndex, "", 0), + new ShardId("leader_index", "", 0), + 1024, + TransportResumeFollowAction.DEFAULT_MAX_READ_REQUEST_SIZE, + 1, + 1024, + TransportResumeFollowAction.DEFAULT_MAX_READ_REQUEST_SIZE, + 1, + 10240, + new ByteSizeValue(512, ByteSizeUnit.MB), + TimeValue.timeValueMillis(10), + TimeValue.timeValueMillis(10), + Collections.emptyMap() + ); + } + +} From aef1f04144f32bc9b187bd933da8d3d234f371c4 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 15 Jan 2019 09:11:21 +0100 Subject: [PATCH 4/6] only resolve based on follower index names in the shard follow tasks --- .../action/TransportFollowStatsAction.java | 32 ++++--------------- .../xpack/ccr/action/FollowStatsIT.java | 31 ++++++++++++++++++ 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java index 674e4356bd9ee..2ea628f73122b 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java @@ -6,14 +6,13 @@ package org.elasticsearch.xpack.ccr.action; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.FailedNodeException; import org.elasticsearch.action.TaskOperationFailure; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.tasks.TransportTasksAction; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; @@ -41,15 +40,13 @@ public class TransportFollowStatsAction extends TransportTasksAction< FollowStatsAction.StatsResponses, FollowStatsAction.StatsResponse> { private final CcrLicenseChecker ccrLicenseChecker; - private final IndexNameExpressionResolver resolver; @Inject public TransportFollowStatsAction( final ClusterService clusterService, final TransportService transportService, final ActionFilters actionFilters, - final CcrLicenseChecker ccrLicenseChecker, - final IndexNameExpressionResolver resolver) { + final CcrLicenseChecker ccrLicenseChecker) { super( FollowStatsAction.NAME, clusterService, @@ -60,7 +57,6 @@ public TransportFollowStatsAction( FollowStatsAction.StatsResponse::new, Ccr.CCR_THREAD_POOL_NAME); this.ccrLicenseChecker = Objects.requireNonNull(ccrLicenseChecker); - this.resolver = resolver; } @Override @@ -74,27 +70,11 @@ protected void doExecute( } final ClusterState state = clusterService.state(); - try { - String[] concreteIndices = resolver.concreteIndexNames(state, IndicesOptions.strictExpand(), request.indices()); - - // Also include matching shard follow task's follower indices: - // A follower index may be removed and therefor concreteIndexNames(...) does not include it. - // Shard follow tasks for these removed follower indices do exist (with a fatal error set). - Set shardFollowTaskFollowerIndices = new HashSet<>(Arrays.asList(concreteIndices)); - shardFollowTaskFollowerIndices.addAll(findFollowerIndicesFromShardFollowTasks(state, request.indices())); - - request.setIndices(shardFollowTaskFollowerIndices.toArray(new String[0])); - } catch (IndexNotFoundException e) { - // It is possible that there are failed shard follow tasks (with fatal error set) for a none existing index - // (in case the follower index has been removed) - // we should also include the stats for these shard follow tasks: - final Set followerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); - if (followerIndices.size() != 0) { - request.setIndices(followerIndices.toArray(new String[0])); - } else { - throw e; - } + Set shardFollowTaskFollowerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); + if (Strings.isAllOrWildcard(request.indices()) == false && shardFollowTaskFollowerIndices.isEmpty()) { + throw new ResourceNotFoundException("No shard follow tasks for index [{}]", request.indices()[0]); } + request.setIndices(shardFollowTaskFollowerIndices.toArray(new String[0])); super.doExecute(task, request, listener); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java index b2f38202607ea..916e87f59d1b9 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ccr.action; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; @@ -118,6 +119,36 @@ public void testFollowStatsApiFollowerIndexFiltering() throws Exception { }); } + public void testFollowStatsApiResourceNotFound() throws Exception { + FollowStatsAction.StatsRequest statsRequest = new FollowStatsAction.StatsRequest(); + FollowStatsAction.StatsResponses response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(0)); + + statsRequest.setIndices(new String[] {"follower1"}); + Exception e = expectThrows(ResourceNotFoundException.class, + () -> client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet()); + assertThat(e.getMessage(), equalTo("No shard follow tasks for index [follower1]")); + + final String leaderIndexSettings = getIndexSettings(1, 0, + singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")); + assertAcked(client().admin().indices().prepareCreate("leader1").setSource(leaderIndexSettings, XContentType.JSON)); + ensureGreen("leader1"); + + PutFollowAction.Request followRequest = getPutFollowRequest("leader1", "follower1"); + client().execute(PutFollowAction.INSTANCE, followRequest).get(); + + response = client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet(); + assertThat(response.getStatsResponses().size(), equalTo(1)); + assertThat(response.getStatsResponses().get(0).status().followerIndex(), equalTo("follower1")); + + statsRequest.setIndices(new String[] {"follower2"}); + e = expectThrows(ResourceNotFoundException.class, + () -> client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet()); + assertThat(e.getMessage(), equalTo("No shard follow tasks for index [follower2]")); + + assertAcked(client().execute(PauseFollowAction.INSTANCE, new PauseFollowAction.Request("follower1")).actionGet()); + } + public void testFollowStatsApiIncludeShardFollowStatsWithRemovedFollowerIndex() throws Exception { final String leaderIndexSettings = getIndexSettings(1, 0, singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")); From 19bd452411a49bd93042d40c1a3463f6959b9abf Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 15 Jan 2019 09:21:41 +0100 Subject: [PATCH 5/6] iter --- .../xpack/ccr/action/TransportFollowStatsAction.java | 5 ++--- .../org/elasticsearch/xpack/ccr/action/FollowStatsIT.java | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java index 2ea628f73122b..7c905ef2d66bc 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java @@ -16,7 +16,6 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.tasks.Task; @@ -72,9 +71,9 @@ protected void doExecute( final ClusterState state = clusterService.state(); Set shardFollowTaskFollowerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); if (Strings.isAllOrWildcard(request.indices()) == false && shardFollowTaskFollowerIndices.isEmpty()) { - throw new ResourceNotFoundException("No shard follow tasks for index [{}]", request.indices()[0]); + String resources = String.join(",", request.indices()); + throw new ResourceNotFoundException("No shard follow tasks for follower indices [{}]", resources); } - request.setIndices(shardFollowTaskFollowerIndices.toArray(new String[0])); super.doExecute(task, request, listener); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java index 916e87f59d1b9..bffe73a80e63b 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowStatsIT.java @@ -127,7 +127,7 @@ public void testFollowStatsApiResourceNotFound() throws Exception { statsRequest.setIndices(new String[] {"follower1"}); Exception e = expectThrows(ResourceNotFoundException.class, () -> client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet()); - assertThat(e.getMessage(), equalTo("No shard follow tasks for index [follower1]")); + assertThat(e.getMessage(), equalTo("No shard follow tasks for follower indices [follower1]")); final String leaderIndexSettings = getIndexSettings(1, 0, singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")); @@ -144,7 +144,7 @@ public void testFollowStatsApiResourceNotFound() throws Exception { statsRequest.setIndices(new String[] {"follower2"}); e = expectThrows(ResourceNotFoundException.class, () -> client().execute(FollowStatsAction.INSTANCE, statsRequest).actionGet()); - assertThat(e.getMessage(), equalTo("No shard follow tasks for index [follower2]")); + assertThat(e.getMessage(), equalTo("No shard follow tasks for follower indices [follower2]")); assertAcked(client().execute(PauseFollowAction.INSTANCE, new PauseFollowAction.Request("follower1")).actionGet()); } From 50350713c9dfaa7b292554c610b4e99a4851c855 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 22 Jan 2019 09:56:57 +0100 Subject: [PATCH 6/6] only execute if concrete indices have been specified --- .../xpack/ccr/action/TransportFollowStatsAction.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java index 7c905ef2d66bc..dc684fbc904ce 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/TransportFollowStatsAction.java @@ -68,11 +68,13 @@ protected void doExecute( return; } - final ClusterState state = clusterService.state(); - Set shardFollowTaskFollowerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); - if (Strings.isAllOrWildcard(request.indices()) == false && shardFollowTaskFollowerIndices.isEmpty()) { - String resources = String.join(",", request.indices()); - throw new ResourceNotFoundException("No shard follow tasks for follower indices [{}]", resources); + if (Strings.isAllOrWildcard(request.indices()) == false) { + final ClusterState state = clusterService.state(); + Set shardFollowTaskFollowerIndices = findFollowerIndicesFromShardFollowTasks(state, request.indices()); + if (shardFollowTaskFollowerIndices.isEmpty()) { + String resources = String.join(",", request.indices()); + throw new ResourceNotFoundException("No shard follow tasks for follower indices [{}]", resources); + } } super.doExecute(task, request, listener); }