From a6e2a2500b06b6b58bcd3474b48e4245b088b692 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 27 Nov 2018 12:16:20 +0100 Subject: [PATCH 1/5] [CCR] AutoFollowCoordinator should tolerate that auto follow patterns may be removed AutoFollowCoordinator should take into account that after auto following an index and while updating that a leader index has been followed, that the auto follow pattern may have been removed via delete auto follow patters api. Closes #35480 --- .../src/test/java/org/elasticsearch/client/CCRIT.java | 1 - .../xpack/ccr/action/AutoFollowCoordinator.java | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java index 55ee556476f23..cd817ec7daa06 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java @@ -128,7 +128,6 @@ public void testIndexFollowing() throws Exception { assertThat(unfollowResponse.isAcknowledged(), is(true)); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/35937") public void testAutoFollowing() throws Exception { CcrClient ccrClient = highLevelClient().ccr(); PutAutoFollowPatternRequest putAutoFollowPatternRequest = diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java index 6323fb7f103db..a4021019eab06 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinator.java @@ -394,6 +394,13 @@ static Function recordLeaderIndexAsFollowFunction(St return currentState -> { AutoFollowMetadata currentAutoFollowMetadata = currentState.metaData().custom(AutoFollowMetadata.TYPE); Map> newFollowedIndexUUIDS = new HashMap<>(currentAutoFollowMetadata.getFollowedLeaderIndexUUIDs()); + if (newFollowedIndexUUIDS.containsKey(name) == false) { + // A delete auto follow pattern request can have removed the auto follow pattern while we want to update + // the auto follow metadata with the fact that an index was successfully auto followed. If this + // happens, we can just skip this step. + return currentState; + } + newFollowedIndexUUIDS.compute(name, (key, existingUUIDs) -> { assert existingUUIDs != null; List newUUIDs = new ArrayList<>(existingUUIDs); From 1388f4eccd3ccc7ad8b9b980a90f14f87f4300f9 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 29 Nov 2018 14:57:21 +0100 Subject: [PATCH 2/5] added unit tests --- .../action/AutoFollowCoordinatorTests.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinatorTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinatorTests.java index 4624a3622b992..2b7fee13502af 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinatorTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/AutoFollowCoordinatorTests.java @@ -40,8 +40,10 @@ import java.util.function.Consumer; import java.util.function.Function; +import static org.elasticsearch.xpack.ccr.action.AutoFollowCoordinator.AutoFollower.recordLeaderIndexAsFollowFunction; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Matchers.anyString; @@ -384,6 +386,33 @@ public void testGetLeaderIndicesToFollow_shardsNotStarted() { assertThat(result.get(1).getName(), equalTo("index2")); } + public void testRecordLeaderIndexAsFollowFunction() { + AutoFollowMetadata autoFollowMetadata = new AutoFollowMetadata(Collections.emptyMap(), + Collections.singletonMap("pattern1", Collections.emptyList()), Collections.emptyMap()); + ClusterState clusterState = new ClusterState.Builder(new ClusterName("name")) + .metaData(new MetaData.Builder().putCustom(AutoFollowMetadata.TYPE, autoFollowMetadata)) + .build(); + Function function = recordLeaderIndexAsFollowFunction("pattern1", new Index("index1", "index1")); + + ClusterState result = function.apply(clusterState); + AutoFollowMetadata autoFollowMetadataResult = result.metaData().custom(AutoFollowMetadata.TYPE); + assertThat(autoFollowMetadataResult.getFollowedLeaderIndexUUIDs().get("pattern1"), notNullValue()); + assertThat(autoFollowMetadataResult.getFollowedLeaderIndexUUIDs().get("pattern1").size(), equalTo(1)); + assertThat(autoFollowMetadataResult.getFollowedLeaderIndexUUIDs().get("pattern1").get(0), equalTo("index1")); + } + + public void testRecordLeaderIndexAsFollowFunctionNoEntry() { + AutoFollowMetadata autoFollowMetadata = new AutoFollowMetadata(Collections.emptyMap(), Collections.emptyMap(), + Collections.emptyMap()); + ClusterState clusterState = new ClusterState.Builder(new ClusterName("name")) + .metaData(new MetaData.Builder().putCustom(AutoFollowMetadata.TYPE, autoFollowMetadata)) + .build(); + Function function = recordLeaderIndexAsFollowFunction("pattern1", new Index("index1", "index1")); + + ClusterState result = function.apply(clusterState); + assertThat(result, sameInstance(clusterState)); + } + public void testGetFollowerIndexName() { AutoFollowPattern autoFollowPattern = new AutoFollowPattern("remote", Collections.singletonList("metrics-*"), null, null, null, null, null, null, null, null, null, null, null); From 7962bf3293e558833c5085c0a6256a7ff3c2bf06 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 4 Dec 2018 09:11:45 +0100 Subject: [PATCH 3/5] fixed test --- .../src/test/java/org/elasticsearch/client/CCRIT.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java index 6adadebd8e743..a0d4ebad60c31 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java @@ -39,6 +39,7 @@ import org.elasticsearch.client.ccr.ResumeFollowRequest; import org.elasticsearch.client.ccr.UnfollowRequest; import org.elasticsearch.client.core.AcknowledgedResponse; +import org.elasticsearch.common.xcontent.ObjectPath; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -148,14 +149,18 @@ public void testAutoFollowing() throws Exception { assertBusy(() -> { assertThat(indexExists("copy-logs-20200101"), is(true)); + // TODO: replace with HLRC follow stats when available: + Map rsp = toMap(client().performRequest(new Request("GET", "/copy-logs-20200101/_ccr/stats"))); + String index = ObjectPath.eval("indices.0.index", rsp); + assertThat(index, equalTo("copy-logs-20200101")); }); GetAutoFollowPatternRequest getAutoFollowPatternRequest = randomBoolean() ? new GetAutoFollowPatternRequest("pattern1") : new GetAutoFollowPatternRequest(); GetAutoFollowPatternResponse getAutoFollowPatternResponse = execute(getAutoFollowPatternRequest, ccrClient::getAutoFollowPattern, ccrClient::getAutoFollowPatternAsync); - assertThat(getAutoFollowPatternResponse.getPatterns().size(), equalTo(1L)); - GetAutoFollowPatternResponse.Pattern pattern = getAutoFollowPatternResponse.getPatterns().get("patterns1"); + assertThat(getAutoFollowPatternResponse.getPatterns().size(), equalTo(1)); + GetAutoFollowPatternResponse.Pattern pattern = getAutoFollowPatternResponse.getPatterns().get("pattern1"); assertThat(pattern, notNullValue()); assertThat(pattern.getRemoteCluster(), equalTo(putAutoFollowPatternRequest.getRemoteCluster())); assertThat(pattern.getLeaderIndexPatterns(), equalTo(putAutoFollowPatternRequest.getLeaderIndexPatterns())); From 44fb6881fc98dd0c6028740d8e07ce12cfbd17f1 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 4 Dec 2018 10:57:51 +0100 Subject: [PATCH 4/5] ensure remote connection is ready before starting tests --- .../src/test/java/org/elasticsearch/client/CCRIT.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java index a0d4ebad60c31..a9ad519d1771e 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java @@ -56,7 +56,7 @@ public class CCRIT extends ESRestHighLevelClientTestCase { @Before - public void setupRemoteClusterConfig() throws IOException { + public void setupRemoteClusterConfig() throws Exception { // Configure local cluster as remote cluster: // TODO: replace with nodes info highlevel rest client code when it is available: final Request request = new Request("GET", "/_nodes"); @@ -70,6 +70,14 @@ public void setupRemoteClusterConfig() throws IOException { ClusterUpdateSettingsResponse updateSettingsResponse = highLevelClient().cluster().putSettings(updateSettingsRequest, RequestOptions.DEFAULT); assertThat(updateSettingsResponse.isAcknowledged(), is(true)); + + assertBusy(() -> { + Map localConnection = (Map) toMap(client() + .performRequest(new Request("GET", "/_remote/info"))) + .get("local"); + assertThat(localConnection, notNullValue()); + assertThat(localConnection.get("connected"), is(true)); + }); } public void testIndexFollowing() throws Exception { From 725183027bb6d56d873610ffeda640ace9630427 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 4 Dec 2018 13:25:43 +0100 Subject: [PATCH 5/5] fixed tests --- .../src/test/java/org/elasticsearch/client/CCRIT.java | 5 ++++- .../elasticsearch/xpack/ccr/CcrLicenseChecker.java | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java index a9ad519d1771e..391ee1fcd18b4 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CCRIT.java @@ -159,7 +159,10 @@ public void testAutoFollowing() throws Exception { assertThat(indexExists("copy-logs-20200101"), is(true)); // TODO: replace with HLRC follow stats when available: Map rsp = toMap(client().performRequest(new Request("GET", "/copy-logs-20200101/_ccr/stats"))); - String index = ObjectPath.eval("indices.0.index", rsp); + String index = null; + try { + index = ObjectPath.eval("indices.0.index", rsp); + } catch (Exception e){ } assertThat(index, equalTo("copy-logs-20200101")); }); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrLicenseChecker.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrLicenseChecker.java index 3985b90a71b23..77ac94da4aa53 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrLicenseChecker.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/CcrLicenseChecker.java @@ -160,15 +160,22 @@ public void checkRemoteClusterLicenseAndFetchClusterState( final ClusterStateRequest request, final Consumer onFailure, final Consumer leaderClusterStateConsumer) { - checkRemoteClusterLicenseAndFetchClusterState( + try { + Client remoteClient = systemClient(client.getRemoteClusterClient(clusterAlias)); + checkRemoteClusterLicenseAndFetchClusterState( client, clusterAlias, - systemClient(client.getRemoteClusterClient(clusterAlias)), + remoteClient, request, onFailure, leaderClusterStateConsumer, CcrLicenseChecker::clusterStateNonCompliantRemoteLicense, e -> clusterStateUnknownRemoteLicense(clusterAlias, e)); + } catch (Exception e) { + // client.getRemoteClusterClient(...) can fail with a IllegalArgumentException if remote + // connection is unknown + onFailure.accept(e); + } } /**