Browse Source

Allow excluding a cluster from a CCS search using minus sign in multi-target syntax (#97865)

Elasticsearch 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
Michael Peterson 2 years ago
parent
commit
50b1749bca

+ 7 - 0
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:
 

+ 53 - 5
server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java

@@ -13,10 +13,12 @@ import org.elasticsearch.cluster.node.DiscoveryNode;
 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 @@ import java.util.stream.Stream;
  * 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 @@ public abstract class RemoteClusterAware {
      * 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<String, List<String>> groupClusterIndices(Set<String> remoteClusterNames, String[] requestIndices) {
         Map<String, List<String>> perClusterIndices = new HashMap<>();
+        Set<String> clustersToRemove = new HashSet<>();
         for (String index : requestIndices) {
             int i = index.indexOf(RemoteClusterService.REMOTE_CLUSTER_INDEX_SEPARATOR);
             if (i >= 0) {
@@ -72,16 +82,54 @@ public abstract class RemoteClusterAware {
                     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<String> 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<String> excludeFailed = new ArrayList<>();
+        for (String exclude : clustersToRemove) {
+            List<String> 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().stream().map(s -> s.equals("") ? "(local)" : s).collect(Collectors.toList()),
+                String.join(",", requestIndices)
+            );
+            throw new IllegalArgumentException(warning);
+        }
+        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)
+                    + "]"
+            );
+        }
         return perClusterIndices;
     }
 

+ 139 - 17
server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java

@@ -48,6 +48,7 @@ import static org.elasticsearch.test.NodeRoles.masterOnlyNode;
 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 class RemoteClusterServiceTests extends ESTestCase {
                     assertTrue(service.isRemoteClusterRegistered("cluster_1"));
                     assertTrue(service.isRemoteClusterRegistered("cluster_2"));
                     assertFalse(service.isRemoteClusterRegistered("foo"));
-                    Map<String, List<String>> 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<String> 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<String, List<String>> 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<String> 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,125 @@ public class RemoteClusterServiceTests extends ESTestCase {
                             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<String, List<String>> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices);
+                        assertEquals(2, perClusterIndices.size());
+                        List<String> localIndexes = perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
+                        assertNotNull(localIndexes);
+                        assertEquals(1, localIndexes.size());
+                        assertEquals("foo", localIndexes.get(0));
+
+                        List<String> 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<String, List<String>> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices);
+                        assertEquals(1, perClusterIndices.size());
+
+                        List<String> 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<String, List<String>> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices);
+                        assertEquals(1, perClusterIndices.size());
+                        List<String> 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<String, List<String>> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(), indices);
+                        assertEquals(1, perClusterIndices.size());
+                        List<String> 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(),
+                            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(),
+                            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,
+                            () -> 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.")
+                        );
+                    }
                 }
             }
         }