From 4e9ff51ff0f251aaf20a87a22427ac16b72efe71 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 21 Jul 2023 11:29:45 -0400 Subject: [PATCH 1/5] Allow excluding a cluster from a CCS search using multi-target syntax The multi-target syntax for indices allows excluding an index with the "-" sign. Public docs: https://www.elastic.co/guide/en/elasticsearch/reference/current/api-conventions.html#api-multi-index This commit expands that functionality to index expressions that include a cluster alias. For example: POST logs*,*:logs*,-remote4:*,-remote1*:*/_async_search Would result in search all remote clusters except for remote4 and remote1, remote11, remote12, remote13, etc.. A singleton wildcard is required in the index position of the `cluster:index`, to specify that we are excluding the entire cluster. This is useful when a cluster is down or slow during CCS searches. Excluding a subset of indexes on a remote cluster is not supported in this commit. For example, this will throw an error: POST logs*,*:logs*,-remote4:logs*/_async_search --- .../transport/RemoteClusterAware.java | 52 ++++++- .../transport/RemoteClusterServiceTests.java | 130 +++++++++++++++--- 2 files changed, 160 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index 641e87bbe3bae..5c567aa2b7bce 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -13,10 +13,12 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Strings; import org.elasticsearch.node.Node; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -28,7 +30,6 @@ * Base class for all services and components that need up-to-date information about the registered remote clusters */ public abstract class RemoteClusterAware { - public static final char REMOTE_CLUSTER_INDEX_SEPARATOR = ':'; public static final String LOCAL_CLUSTER_GROUP_KEY = ""; @@ -58,13 +59,22 @@ protected static Set getEnabledRemoteClusters(final Settings settings) { * indices per cluster are collected as a list in the returned map keyed by the cluster alias. Local indices are grouped under * {@link #LOCAL_CLUSTER_GROUP_KEY}. The returned map is mutable. * - * @param remoteClusterNames the remote cluster names + * This method supports excluding clusters by using the {@code -cluster:*} index expression. + * For example, if requestIndices is [blogs, *:blogs, -remote1:*] and *:blogs resolves to "remote1:blogs, remote2:blogs" + * the map returned by the function will not have the remote1 entry. It will have only {"":blogs, remote2:blogs}. + * The index for the excluded cluster must be '*' to clarify that the entire cluster should be removed. + * A wildcard in the "-" excludes notation is also allowed. For example, suppose there are three remote clusters, + * remote1, remote2, remote3, and this index expression is provided: blogs,rem*:blogs,-rem*1:*. That would successfully + * remove remote1 from the list of clusters to be included. + * + * @param remoteClusterNames the remote cluster names. If a clusterAlias is preceded by a minus sign that cluster will be excluded. * @param requestIndices the indices in the search request to filter * * @return a map of grouped remote and local indices */ protected Map> groupClusterIndices(Set remoteClusterNames, String[] requestIndices) { Map> perClusterIndices = new HashMap<>(); + Set clustersToRemove = new HashSet<>(); for (String index : requestIndices) { int i = index.indexOf(RemoteClusterService.REMOTE_CLUSTER_INDEX_SEPARATOR); if (i >= 0) { @@ -72,16 +82,48 @@ protected Map> groupClusterIndices(Set remoteCluste assert remoteClusterNames.isEmpty() : remoteClusterNames; throw new IllegalArgumentException("node [" + nodeName + "] does not have the remote cluster client role enabled"); } - String remoteClusterName = index.substring(0, i); + int startIdx = index.charAt(0) == '-' ? 1 : 0; + String remoteClusterName = index.substring(startIdx, i); List clusters = ClusterNameExpressionResolver.resolveClusterNames(remoteClusterNames, remoteClusterName); String indexName = index.substring(i + 1); - for (String clusterName : clusters) { - perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName); + if (startIdx == 1) { + if (indexName.equals("*") == false) { + throw new IllegalArgumentException( + Strings.format( + "To exclude a cluster you must specify the '*' wildcard for " + "the index expression, but found: [%s]", + indexName + ) + ); + } + clustersToRemove.addAll(clusters); + } else { + for (String clusterName : clusters) { + perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName); + } } } else { perClusterIndices.computeIfAbsent(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index); } } + List excludeFailed = new ArrayList<>(); + for (String exclude : clustersToRemove) { + List removed = perClusterIndices.remove(exclude); + if (removed == null) { + excludeFailed.add(exclude); + } + } + if (excludeFailed.size() > 0) { + String warning = Strings.format( + "Attempt to exclude cluster%s %s failed as %s not included in the list of clusters to be included: %s. Input: [%s]", + excludeFailed.size() == 1 ? "" : "s", + excludeFailed, + excludeFailed.size() == 1 ? "it is" : "they are", + perClusterIndices.keySet(), + String.join(",", requestIndices) + ); + throw new IllegalArgumentException(warning); + } + return perClusterIndices; } diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 0391e885bb0af..f295592948c02 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -48,6 +48,7 @@ import static org.elasticsearch.test.NodeRoles.nonMasterNode; import static org.elasticsearch.test.NodeRoles.onlyRoles; import static org.elasticsearch.test.NodeRoles.removeRoles; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -165,23 +166,25 @@ public void testGroupClusterIndices() throws IOException { assertTrue(service.isRemoteClusterRegistered("cluster_1")); assertTrue(service.isRemoteClusterRegistered("cluster_2")); assertFalse(service.isRemoteClusterRegistered("foo")); - Map> perClusterIndices = service.groupClusterIndices( - service.getRemoteClusterNames(), - new String[] { - "cluster_1:bar", - "cluster_2:foo:bar", - "cluster_1:test", - "cluster_2:foo*", - "foo", - "cluster*:baz", - "*:boo" } - ); - List localIndices = perClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); - assertNotNull(localIndices); - assertEquals("foo", localIndices.get(0)); - assertEquals(2, perClusterIndices.size()); - assertEquals(Arrays.asList("bar", "test", "baz", "boo"), perClusterIndices.get("cluster_1")); - assertEquals(Arrays.asList("foo:bar", "foo*", "baz", "boo"), perClusterIndices.get("cluster_2")); + { + Map> perClusterIndices = service.groupClusterIndices( + service.getRemoteClusterNames(), + new String[] { + "cluster_1:bar", + "cluster_2:foo:bar", + "cluster_1:test", + "cluster_2:foo*", + "foo", + "cluster*:baz", + "*:boo" } + ); + List localIndices = perClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); + assertNotNull(localIndices); + assertEquals("foo", localIndices.get(0)); + assertEquals(2, perClusterIndices.size()); + assertEquals(Arrays.asList("bar", "test", "baz", "boo"), perClusterIndices.get("cluster_1")); + assertEquals(Arrays.asList("foo:bar", "foo*", "baz", "boo"), perClusterIndices.get("cluster_2")); + } expectThrows( NoSuchRemoteClusterException.class, @@ -198,6 +201,99 @@ public void testGroupClusterIndices() throws IOException { new String[] { "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "does_not_exist:*" } ) ); + + // test cluster exclusions + { + String[] indices = shuffledList(List.of("cluster*:foo*", "foo", "-cluster_1:*", "*:boo")).toArray(new String[0]); + + Map> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices); + assertEquals(2, perClusterIndices.size()); + List localIndexes = perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); + assertNotNull(localIndexes); + assertEquals(1, localIndexes.size()); + assertEquals("foo", localIndexes.get(0)); + + List cluster2 = perClusterIndices.get("cluster_2"); + assertNotNull(cluster2); + assertEquals(2, cluster2.size()); + assertEquals(List.of("boo", "foo*"), cluster2.stream().sorted().toList()); + } + { + String[] indices = shuffledList(List.of("*:*", "-clu*_1:*", "*:boo")).toArray(new String[0]); + + Map> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices); + assertEquals(1, perClusterIndices.size()); + + List cluster2 = perClusterIndices.get("cluster_2"); + assertNotNull(cluster2); + assertEquals(2, cluster2.size()); + assertEquals(List.of("*", "boo"), cluster2.stream().sorted().toList()); + } + { + String[] indices = shuffledList(List.of("cluster*:foo*", "cluster_2:*", "foo", "-cluster_1:*", "-c*:*")).toArray( + new String[0] + ); + + Map> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices); + assertEquals(1, perClusterIndices.size()); + List localIndexes = perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); + assertNotNull(localIndexes); + assertEquals(1, localIndexes.size()); + assertEquals("foo", localIndexes.get(0)); + } + { + String[] indices = shuffledList(List.of("cluster*:*", "foo", "-*:*")).toArray(new String[0]); + + Map> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices); + assertEquals(1, perClusterIndices.size()); + List localIndexes = perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY); + assertNotNull(localIndexes); + assertEquals(1, localIndexes.size()); + assertEquals("foo", localIndexes.get(0)); + } + { + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices( + service.getRemoteClusterNames(), + // -cluster_1:foo* is not allowed, only -cluster_1:* + new String[] { "cluster_1:bar", "-cluster_2:foo*", "cluster_1:test", "cluster_2:foo*", "foo" } + ) + ); + assertThat( + e.getMessage(), + equalTo("To exclude a cluster you must specify the '*' wildcard for the index expression, but found: [foo*]") + ); + } + { + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices( + service.getRemoteClusterNames(), + // -cluster_1:* will fail since cluster_1 was never included in order to qualify to be excluded + new String[] { "-cluster_1:*", "cluster_2:foo*", "foo" } + ) + ); + assertThat(e.getMessage(), containsString("Attempt to exclude cluster [cluster_1] failed")); + } + { + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices(service.getRemoteClusterNames(), new String[] { "-cluster_1:*" }) + ); + assertThat(e.getMessage(), containsString("Attempt to exclude cluster [cluster_1] failed")); + } + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices(service.getRemoteClusterNames(), new String[] { "-*:*" }) + ); + assertThat( + e.getMessage(), + equalTo( + "Attempt to exclude clusters [cluster_1, cluster_2] failed as they are not included in the list of " + + "clusters to be included: []. Input: [-*:*]" + ) + ); } } } From ba55ed45dca9f814a2d26fe15d1498d61a5f31a6 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 21 Jul 2023 14:01:20 -0400 Subject: [PATCH 2/5] improved error message of 'included' clusters to have (local) rather than empty string --- .../transport/RemoteClusterAware.java | 2 +- .../transport/RemoteClusterServiceTests.java | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index 5c567aa2b7bce..ef6196f8786fe 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -118,7 +118,7 @@ protected Map> groupClusterIndices(Set remoteCluste excludeFailed.size() == 1 ? "" : "s", excludeFailed, excludeFailed.size() == 1 ? "it is" : "they are", - perClusterIndices.keySet(), + perClusterIndices.keySet().stream().map(s -> s.equals("") ? "(local)" : s).collect(Collectors.toList()), String.join(",", requestIndices) ); throw new IllegalArgumentException(warning); diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index f295592948c02..380554df08303 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -48,7 +48,6 @@ import static org.elasticsearch.test.NodeRoles.nonMasterNode; import static org.elasticsearch.test.NodeRoles.onlyRoles; import static org.elasticsearch.test.NodeRoles.removeRoles; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -274,14 +273,26 @@ public void testGroupClusterIndices() throws IOException { new String[] { "-cluster_1:*", "cluster_2:foo*", "foo" } ) ); - assertThat(e.getMessage(), containsString("Attempt to exclude cluster [cluster_1] failed")); + assertThat( + e.getMessage(), + equalTo( + "Attempt to exclude cluster [cluster_1] failed as it is not included in the list of clusters to " + + "be included: [(local), cluster_2]. Input: [-cluster_1:*,cluster_2:foo*,foo]" + ) + ); } { IllegalArgumentException e = expectThrows( IllegalArgumentException.class, () -> service.groupClusterIndices(service.getRemoteClusterNames(), new String[] { "-cluster_1:*" }) ); - assertThat(e.getMessage(), containsString("Attempt to exclude cluster [cluster_1] failed")); + assertThat( + e.getMessage(), + equalTo( + "Attempt to exclude cluster [cluster_1] failed as it is not included in the list of clusters to " + + "be included: []. Input: [-cluster_1:*]" + ) + ); } IllegalArgumentException e = expectThrows( IllegalArgumentException.class, From c03d07cb02f0c23499184075277075cf7f60ac4a Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 26 Jul 2023 09:07:41 -0400 Subject: [PATCH 3/5] If all remote clusters are excluded by the exclusion list, an error will be thrown --- .../transport/RemoteClusterAware.java | 8 +++- .../transport/RemoteClusterServiceTests.java | 37 +++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index ef6196f8786fe..bb87a49070592 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -123,7 +123,13 @@ protected Map> groupClusterIndices(Set remoteCluste ); throw new IllegalArgumentException(warning); } - + if (requestIndices.length > 0 && perClusterIndices.size() == 0) { + throw new IllegalArgumentException( + "The '-' exclusions in the index expression list excludes all indexes. Nothing to search. Input: [" + + String.join(",", requestIndices) + + "]" + ); + } return perClusterIndices; } diff --git a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java index 380554df08303..57f56f898087f 100644 --- a/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java @@ -48,6 +48,7 @@ import static org.elasticsearch.test.NodeRoles.nonMasterNode; import static org.elasticsearch.test.NodeRoles.onlyRoles; import static org.elasticsearch.test.NodeRoles.removeRoles; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -294,17 +295,31 @@ public void testGroupClusterIndices() throws IOException { ) ); } - IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, - () -> service.groupClusterIndices(service.getRemoteClusterNames(), new String[] { "-*:*" }) - ); - assertThat( - e.getMessage(), - equalTo( - "Attempt to exclude clusters [cluster_1, cluster_2] failed as they are not included in the list of " - + "clusters to be included: []. Input: [-*:*]" - ) - ); + { + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices(service.getRemoteClusterNames(), new String[] { "-*:*" }) + ); + assertThat( + e.getMessage(), + equalTo( + "Attempt to exclude clusters [cluster_1, cluster_2] failed as they are not included in the list of " + + "clusters to be included: []. Input: [-*:*]" + ) + ); + } + { + String[] indices = shuffledList(List.of("cluster*:*", "*:foo", "-*:*")).toArray(new String[0]); + + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> service.groupClusterIndices(service.getRemoteClusterNames(), indices) + ); + assertThat( + e.getMessage(), + containsString("The '-' exclusions in the index expression list excludes all indexes. Nothing to search.") + ); + } } } } From 1f0fa2aaaf89fe85026c96e57b251e654b2b945d Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 26 Jul 2023 09:16:56 -0400 Subject: [PATCH 4/5] Added update to api-conventions.asciidoc end user docs --- docs/reference/api-conventions.asciidoc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/reference/api-conventions.asciidoc b/docs/reference/api-conventions.asciidoc index 0e231056e8daf..64cb499a9cd4e 100644 --- a/docs/reference/api-conventions.asciidoc +++ b/docs/reference/api-conventions.asciidoc @@ -195,6 +195,13 @@ request that targets an excluded alias. For example, if `test3` is an index alias, the pattern `test*,-test3` still targets the indices for `test3`. To avoid this, exclude the concrete indices for the alias instead. +You can also exclude clusters from a list of clusters to search using the `-` character: +`remote*:*,-remote1:*,-remote4:*` will search all clusters with an alias that starts +with "remote" except for "remote1" and "remote4". Note that to exclude a cluster +with this notation you must exclude all of its indexes. Excluding a subset of indexes +on a remote cluster is currently not supported. For example, this will throw an exception: +`remote*:*,-remote1:logs*`. + Multi-target APIs that can target indices support the following query string parameters: From c7a016dfa4896d542856012d61b1dabfd87a1e8c Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 26 Jul 2023 10:52:49 -0400 Subject: [PATCH 5/5] Improved the logic of when to throw error about exclusions list excluded all clusters, so Jbdc tests pass again --- .../java/org/elasticsearch/transport/RemoteClusterAware.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java index bb87a49070592..a8a4925441709 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java @@ -123,7 +123,7 @@ protected Map> groupClusterIndices(Set remoteCluste ); throw new IllegalArgumentException(warning); } - if (requestIndices.length > 0 && perClusterIndices.size() == 0) { + if (clustersToRemove.size() > 0 && perClusterIndices.size() == 0) { throw new IllegalArgumentException( "The '-' exclusions in the index expression list excludes all indexes. Nothing to search. Input: [" + String.join(",", requestIndices)