From de677365f6f8e64c48cf19ece8ffa1b72e78e14c Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 1 Nov 2024 16:21:02 -0400 Subject: [PATCH 01/17] Intmd commit - going to split into refactoring ticket --- ...CrossClusterQueryUnavailableRemotesIT.java | 204 ++--- .../esql/action/CrossClustersEnrichIT.java | 1 + .../esql/action/CrossClustersQueryIT.java | 699 +++++++++++++----- .../esql/session/EsqlSessionCCSUtils.java | 65 +- 4 files changed, 649 insertions(+), 320 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java index 0f1aa8541fdd9..c3b08420cfa4f 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java @@ -137,108 +137,108 @@ public void testCCSAgainstDisconnectedRemoteWithSkipUnavailableTrue() throws Exc } // scenario where there are no indices to match because - // 1) the local cluster indexExpression and REMOTE_CLUSTER_2 indexExpression match no indices - // 2) the REMOTE_CLUSTER_1 is unavailable - // 3) both remotes are marked as skip_un=true - String query = "FROM nomatch*," + REMOTE_CLUSTER_1 + ":logs-*," + REMOTE_CLUSTER_2 + ":nomatch* | STATS sum (v)"; - try (EsqlQueryResponse resp = runQuery(query, requestIncludeMeta)) { - List> values = getValuesList(resp); - assertThat(values, hasSize(0)); - - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); - - EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); - assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); - assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remote1Cluster.getTotalShards(), equalTo(0)); - assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); - assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); - assertThat(remote1Cluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); - assertThat(remote2Cluster.getIndexExpression(), equalTo("nomatch*")); - assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remote2Cluster.getTotalShards(), equalTo(0)); - assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); - assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); - assertThat(remote2Cluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("nomatch*")); - // local cluster should never be marked as SKIPPED - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(0)); - assertThat(localCluster.getSuccessfulShards(), equalTo(0)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); - - // ensure that the _clusters metadata is present only if requested - assertClusterMetadataInResponse(resp, responseExpectMeta); - } - - // close remote-cluster-2 so that it is also unavailable - cluster(REMOTE_CLUSTER_2).close(); - - try (EsqlQueryResponse resp = runQuery("FROM logs-*,*:logs-* | STATS sum (v)", requestIncludeMeta)) { - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of(45L))); - - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); - - EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); - assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); - assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remote1Cluster.getTotalShards(), equalTo(0)); - assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); - assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); - assertThat(remote1Cluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); - assertThat(remote2Cluster.getIndexExpression(), equalTo("logs-*")); - assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remote2Cluster.getTotalShards(), equalTo(0)); - assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); - assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); - assertThat(remote2Cluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); - assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); - - // ensure that the _clusters metadata is present only if requested - assertClusterMetadataInResponse(resp, responseExpectMeta); - } +// // 1) the local cluster indexExpression and REMOTE_CLUSTER_2 indexExpression match no indices +// // 2) the REMOTE_CLUSTER_1 is unavailable +// // 3) both remotes are marked as skip_un=true +// String query = "FROM nomatch*," + REMOTE_CLUSTER_1 + ":logs-*," + REMOTE_CLUSTER_2 + ":nomatch* | STATS sum (v)"; +// try (EsqlQueryResponse resp = runQuery(query, requestIncludeMeta)) { +// List> values = getValuesList(resp); +// assertThat(values, hasSize(0)); +// +// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); +// assertNotNull(executionInfo); +// assertThat(executionInfo.isCrossClusterSearch(), is(true)); +// long overallTookMillis = executionInfo.overallTook().millis(); +// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); +// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); +// +// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); +// +// EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); +// assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); +// assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); +// assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(remote1Cluster.getTotalShards(), equalTo(0)); +// assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); +// assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); +// assertThat(remote1Cluster.getFailedShards(), equalTo(0)); +// +// EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); +// assertThat(remote2Cluster.getIndexExpression(), equalTo("nomatch*")); +// assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); +// assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(remote2Cluster.getTotalShards(), equalTo(0)); +// assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); +// assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); +// assertThat(remote2Cluster.getFailedShards(), equalTo(0)); +// +// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); +// assertThat(localCluster.getIndexExpression(), equalTo("nomatch*")); +// // local cluster should never be marked as SKIPPED +// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); +// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(localCluster.getTotalShards(), equalTo(0)); +// assertThat(localCluster.getSuccessfulShards(), equalTo(0)); +// assertThat(localCluster.getSkippedShards(), equalTo(0)); +// assertThat(localCluster.getFailedShards(), equalTo(0)); +// +// // ensure that the _clusters metadata is present only if requested +// assertClusterMetadataInResponse(resp, responseExpectMeta); +// } +// +// // close remote-cluster-2 so that it is also unavailable +// cluster(REMOTE_CLUSTER_2).close(); +// +// try (EsqlQueryResponse resp = runQuery("FROM logs-*,*:logs-* | STATS sum (v)", requestIncludeMeta)) { +// List> values = getValuesList(resp); +// assertThat(values, hasSize(1)); +// assertThat(values.get(0), equalTo(List.of(45L))); +// +// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); +// assertNotNull(executionInfo); +// assertThat(executionInfo.isCrossClusterSearch(), is(true)); +// long overallTookMillis = executionInfo.overallTook().millis(); +// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); +// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); +// +// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); +// +// EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); +// assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); +// assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); +// assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(remote1Cluster.getTotalShards(), equalTo(0)); +// assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); +// assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); +// assertThat(remote1Cluster.getFailedShards(), equalTo(0)); +// +// EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); +// assertThat(remote2Cluster.getIndexExpression(), equalTo("logs-*")); +// assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); +// assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(remote2Cluster.getTotalShards(), equalTo(0)); +// assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); +// assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); +// assertThat(remote2Cluster.getFailedShards(), equalTo(0)); +// +// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); +// assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); +// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); +// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); +// assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); +// assertThat(localCluster.getSkippedShards(), equalTo(0)); +// assertThat(localCluster.getFailedShards(), equalTo(0)); +// +// // ensure that the _clusters metadata is present only if requested +// assertClusterMetadataInResponse(resp, responseExpectMeta); +// } } finally { clearSkipUnavailable(numClusters); } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java index e8e9f45694e9c..16fa041b7f203 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java @@ -61,6 +61,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +// MP TODO: what to do about missing enrich policies on skip_unavailable=true clusters? Should that be fatal or non-fatal? Write tests for those cases. public class CrossClustersEnrichIT extends AbstractMultiClustersTestCase { @Override diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index ba44adb5a85e0..179a440c1f763 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.esql.action; import org.elasticsearch.Build; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.client.internal.Client; @@ -27,6 +28,7 @@ import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.plugin.EsqlPlugin; import org.elasticsearch.xpack.esql.plugin.QueryPragmas; @@ -39,26 +41,30 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; public class CrossClustersQueryIT extends AbstractMultiClustersTestCase { - private static final String REMOTE_CLUSTER = "cluster-a"; + private static final String REMOTE_CLUSTER_1 = "cluster-a"; + private static final String REMOTE_CLUSTER_2 = "remote-b"; @Override protected Collection remoteClusterAlias() { - return List.of(REMOTE_CLUSTER); + return List.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2); } @Override protected Map skipUnavailableForRemoteClusters() { - return Map.of(REMOTE_CLUSTER, randomBoolean()); + return Map.of(REMOTE_CLUSTER_1, randomBoolean()); } @Override @@ -90,7 +96,7 @@ public void testSuccessfulPathways() { Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); boolean responseExpectMeta = includeCCSMetadata.v2(); - try (EsqlQueryResponse resp = runQuery("from logs-*,*:logs-* | stats sum (v)", requestIncludeMeta)) { + try (EsqlQueryResponse resp = runQuery("from logs-*,c*:logs-* | stats sum (v)", requestIncludeMeta)) { List> values = getValuesList(resp); assertThat(values, hasSize(1)); assertThat(values.get(0), equalTo(List.of(330L))); @@ -102,9 +108,9 @@ public void testSuccessfulPathways() { assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs-*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -128,7 +134,7 @@ public void testSuccessfulPathways() { assertClusterMetadataInResponse(resp, responseExpectMeta); } - try (EsqlQueryResponse resp = runQuery("from logs-*,*:logs-* | stats count(*) by tag | sort tag | keep tag", requestIncludeMeta)) { + try (EsqlQueryResponse resp = runQuery("from logs-*,c*:logs-* | stats count(*) by tag | sort tag | keep tag", requestIncludeMeta)) { List> values = getValuesList(resp); assertThat(values, hasSize(2)); assertThat(values.get(0), equalTo(List.of("local"))); @@ -141,9 +147,9 @@ public void testSuccessfulPathways() { assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs-*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -168,139 +174,413 @@ public void testSuccessfulPathways() { } } - public void testSearchesWhereMissingIndicesAreSpecified() { - Map testClusterInfo = setupTwoClusters(); + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { + Map testClusterInfo = setupClusters(3); int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); - int remoteNumShards = (Integer) testClusterInfo.get("remote.num_shards"); + int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); + int remote2NumShards = (Integer) testClusterInfo.get("remote2.num_shards"); + + setSkipUnavailable(REMOTE_CLUSTER_1, false); + setSkipUnavailable(REMOTE_CLUSTER_2, true); // MP TODO: what do here? randomBoolean()? Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); boolean responseExpectMeta = includeCCSMetadata.v2(); - // since a valid local index was specified, the invalid index on cluster-a does not throw an exception, - // but instead is simply ignored - ensure this is captured in the EsqlExecutionInfo - try (EsqlQueryResponse resp = runQuery("from logs-*,cluster-a:no_such_index | stats sum (v)", requestIncludeMeta)) { - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of(45L))); + try { + // missing concrete local index is fatal + { + String q = "FROM nomatch,cluster-a:logs*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + } - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + // missing concrete remote index is fatal when skip_unavailable=false + { + String q = "FROM logs*,cluster-a:nomatch,remote*:*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch] on remote cluster [cluster-a]")); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch] on remote cluster [cluster-a]")); + } - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); // 0 since no matching index, thus no shards to search - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); + // non-matching wildcarded local expression is not fatal + { + String q = "FROM nomatch*,*:logs*"; + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards), + new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + )); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), equalTo(0)); + assertThat(resp.columns().size(), greaterThan(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // LIMIT 0 searches always have total shards = 0 + new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + )); + } + } - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); - assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); - } + // missing concrete remote index 'cluster-a:nomatch*' is fatal since cluster-a has skip_unavailable=false + { + // MP TODO: waiting on feedback from Andrei + String q = "FROM logs*,cluster-a:nomatch*,remote*:logs*"; +// EsqlQueryResponse response = runQuery(q, requestIncludeMeta); +// System.err.println(response.getExecutionInfo()); +// VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); +// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*] on remote cluster [cluster-a]")); + } - // since the remote cluster has a valid index expression, the missing local index is ignored - // make this is captured in the EsqlExecutionInfo - try ( - EsqlQueryResponse resp = runQuery( - "from no_such_index,*:logs-* | stats count(*) by tag | sort tag | keep tag", - requestIncludeMeta - ) - ) { - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of("remote"))); + // an error is thrown if there are no matching indices at all - single remote cluster with concrete index expression + { + String q = "FROM cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + } - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + // an error is thrown if there are no matching indices at all - single remote cluster with wildcard index expression + { + String q = "FROM cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("logs-*")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(remoteNumShards)); - assertThat(remoteCluster.getSuccessfulShards(), equalTo(remoteNumShards)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + } - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("no_such_index")); - // TODO: a follow on PR will change this to throw an Exception when the local cluster requests a concrete index that is missing - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(0)); - assertThat(localCluster.getSuccessfulShards(), equalTo(0)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); + // an error is thrown if there are no matching indices at all - local with wildcard, remote with concrete + { + String q = "FROM nomatch*,cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - local with wildcard, remote with wildcard + { + String q = "FROM nomatch*,cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - local with concrete, remote with concrete + { + String q = "FROM nomatch,cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch]")); + } + + // an error is thrown if there are no matching indices at all - local with concrete, remote with wildcard + { + String q = "FROM nomatch,cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + } + + { + String q = "FROM cluster-a:nomatch,remote*:*"; +// VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); +// assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + } + } finally { + clearSkipUnavailable(); } + } - // when multiple invalid indices are specified on the remote cluster, both should be ignored and present - // in the index expression of the EsqlExecutionInfo and with an indication that zero shards were searched - try ( - EsqlQueryResponse resp = runQuery( - "FROM no_such_index*,*:no_such_index1,*:no_such_index2,logs-1 | STATS COUNT(*) by tag | SORT tag | KEEP tag", - requestIncludeMeta - ) - ) { - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of("local"))); + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { + Map testClusterInfo = setupClusters(3); + int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); + int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); + int remote2NumShards = (Integer) testClusterInfo.get("remote2.num_shards"); - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + setSkipUnavailable(REMOTE_CLUSTER_1, true); + setSkipUnavailable(REMOTE_CLUSTER_2, true); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + Tuple includeCCSMetadata = randomIncludeCCSMetadata(); + Boolean requestIncludeMeta = includeCCSMetadata.v1(); + boolean responseExpectMeta = includeCCSMetadata.v2(); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index1,no_such_index2")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); + try { + // missing concrete local index is fatal + { + String q = "FROM nomatch,cluster-a:logs*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + } - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("no_such_index*,logs-1")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); - assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); + // missing concrete remote index is not fatal when skip_unavailable=true + { + String q = "FROM logs*,cluster-a:nomatch,remote*:*"; + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + )); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(resp.columns().size(), greaterThan(0)); + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + )); + } + } + + // non-matching wildcarded local expression is not fatal + { + String q = "FROM nomatch*,*:logs*"; + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards), + new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + )); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), equalTo(0)); + assertThat(resp.columns().size(), greaterThan(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // LIMIT 0 searches always have total shards = 0 + new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + )); + } + } + + // missing wildcarded remote index 'cluster-a:nomatch*' is not fatal since cluster-a has skip_unavailable=true + { + // MP TODO: waiting on feedback from Andrei + String q = "FROM logs*,cluster-a:nomatch*,remote*:*"; + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + )); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(resp.columns().size(), greaterThan(0)); + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + )); + } + } + + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=false + { + // with non-matching concrete index + String q = "FROM cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + } + + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=false (??? + { + // with non-matching wildcard index + String q = "FROM cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - local with wildcard, remote with concrete + { + String q = "FROM nomatch*,cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - local with wildcard, remote with wildcard + { + String q = "FROM nomatch*,cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - local with concrete, remote with concrete + { + String q = "FROM nomatch,cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch,nomatch]")); + } + + // an error is thrown if there are no matching indices at all - local with concrete, remote with wildcard + { + String q = "FROM nomatch,cluster-a:nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + } + + // since cluster-a is skip_unavailable=true and one cluster has a matching indices, no error is thrown + { + String q = "FROM cluster-a:nomatch,remote*:*"; + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClusters(executionInfo, List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + )); + } + } + + } finally { + clearSkipUnavailable(); } + } - // wildcard on remote cluster that matches nothing - should be present in EsqlExecutionInfo marked as SKIPPED, no shards searched - try (EsqlQueryResponse resp = runQuery("from cluster-a:no_such_index*,logs-* | stats sum (v)", requestIncludeMeta)) { + record ExpectedCluster(String clusterAlias, String indexExpression, EsqlExecutionInfo.Cluster.Status status, Integer totalShards) {} + + public void assertExpectedClusters(EsqlExecutionInfo executionInfo, List expected) { + long overallTookMillis = executionInfo.overallTook().millis(); + assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); + + Set expectedClusterAliases = expected.stream().map(c -> c.clusterAlias()).collect(Collectors.toSet()); + assertThat(executionInfo.clusterAliases(), equalTo(expectedClusterAliases)); + + for (ExpectedCluster expectedCluster : expected) { + EsqlExecutionInfo.Cluster cluster = executionInfo.getCluster(expectedCluster.clusterAlias()); + String msg = cluster.getClusterAlias(); + assertThat(msg, cluster.getIndexExpression(), equalTo(expectedCluster.indexExpression())); + assertThat(msg, cluster.getStatus(), equalTo(expectedCluster.status())); + assertThat(msg, cluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(msg, cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(msg, cluster.getTotalShards(), equalTo(expectedCluster.totalShards())); + if (cluster.getStatus() == EsqlExecutionInfo.Cluster.Status.SUCCESSFUL) { + assertThat(msg, cluster.getSuccessfulShards(), equalTo(expectedCluster.totalShards())); + assertThat(msg, cluster.getSkippedShards(), equalTo(0)); + } else if (cluster.getStatus() == EsqlExecutionInfo.Cluster.Status.SKIPPED) { + assertThat(msg, cluster.getSuccessfulShards(), equalTo(0)); + assertThat(msg, cluster.getSkippedShards(), equalTo(expectedCluster.totalShards())); + } + // currently failed shards is always zero - change this once we start allowing partial data for individual shard failures + assertThat(msg, cluster.getFailedShards(), equalTo(0)); + } + } + + public void testSearchesWhereMissingIndicesAreSpecifiedWithSkipUnavailableTrueORIG() { + Map testClusterInfo = setupClusters(3); + int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); + int remoteNumShards = (Integer) testClusterInfo.get("remote.num_shards"); + + setSkipUnavailable(REMOTE_CLUSTER_1, true); + setSkipUnavailable(REMOTE_CLUSTER_2, true); + + Tuple includeCCSMetadata = randomIncludeCCSMetadata(); + Boolean requestIncludeMeta = includeCCSMetadata.v1(); + boolean responseExpectMeta = includeCCSMetadata.v2(); + + // since a valid local index was specified, the invalid index on cluster-a does not throw an exception, + // but instead is simply ignored - ensure this is captured in the EsqlExecutionInfo + try (EsqlQueryResponse resp = runQuery("from logs-*,cluster-a:no_such_index | stats sum (v)", requestIncludeMeta)) { EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); List> values = getValuesList(resp); assertThat(values, hasSize(1)); @@ -312,14 +592,14 @@ public void testSearchesWhereMissingIndicesAreSpecified() { assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index*")); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); + assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); + assertThat(remoteCluster.getTotalShards(), equalTo(0)); // 0 since no matching index, thus no shards to search assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); assertThat(remoteCluster.getSkippedShards(), equalTo(0)); assertThat(remoteCluster.getFailedShards(), equalTo(0)); @@ -376,9 +656,9 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -405,7 +685,10 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { * at query time. */ public void testCCSExecutionOnSearchesWithLimit0() { - setupTwoClusters(); + Map setupMap = setupTwoClusters(); + boolean skipUnavailable = (Boolean) setupMap.get("remote.skip_unavailable"); + + System.err.println(skipUnavailable); Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); @@ -427,9 +710,9 @@ public void testCCSExecutionOnSearchesWithLimit0() { long overallTookMillis = executionInfo.overallTook().millis(); assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -450,65 +733,52 @@ public void testCCSExecutionOnSearchesWithLimit0() { assertThat(remoteCluster.getFailedShards(), equalTo(0)); } - try (EsqlQueryResponse resp = runQuery("FROM logs*,cluster-a:nomatch* | LIMIT 0", requestIncludeMeta)) { - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); - - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("nomatch*")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("logs*")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(0)); - assertThat(localCluster.getSuccessfulShards(), equalTo(0)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); - } - - try (EsqlQueryResponse resp = runQuery("FROM nomatch*,cluster-a:* | LIMIT 0", requestIncludeMeta)) { - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); - - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); - assertThat(remoteCluster.getIndexExpression(), equalTo("*")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("nomatch*")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); - } +// // MP TODO: delete this one? +// if (skipUnavailable == false) { +// VerificationException e = expectThrows(VerificationException.class, +// () -> runQuery("FROM logs*,cluster-a:nomatch* | LIMIT 0", requestIncludeMeta)); +// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*] on remote cluster [cluster-a]")); +// } else { +// try (EsqlQueryResponse resp = runQuery("FROM logs*,cluster-a:nomatch* | LIMIT 0", requestIncludeMeta)) { +// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); +// assertNotNull(executionInfo); +// assertThat(executionInfo.isCrossClusterSearch(), is(true)); +// long overallTookMillis = executionInfo.overallTook().millis(); +// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); +// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); +// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); +// +// EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); +// assertThat(remoteCluster.getIndexExpression(), equalTo("nomatch*")); +// assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); +// assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(remoteCluster.getTotalShards(), equalTo(0)); +// assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); +// assertThat(remoteCluster.getSkippedShards(), equalTo(0)); +// assertThat(remoteCluster.getFailedShards(), equalTo(0)); +// +// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); +// assertThat(localCluster.getIndexExpression(), equalTo("logs*")); +// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); +// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); +// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); +// assertThat(localCluster.getTotalShards(), equalTo(0)); +// assertThat(localCluster.getSuccessfulShards(), equalTo(0)); +// assertThat(localCluster.getSkippedShards(), equalTo(0)); +// assertThat(localCluster.getFailedShards(), equalTo(0)); +// } +// } +// { +// VerificationException e = expectThrows(VerificationException.class, +// () -> runQuery("FROM nomatch*,cluster-a:* | LIMIT 0", requestIncludeMeta)); +// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*]")); +// } +// { +// VerificationException e = expectThrows(VerificationException.class, +// () -> runQuery("FROM nomatch*,foo*,cluster-a:* | LIMIT 0", requestIncludeMeta)); +// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*,foo*]")); +// } } public void testMetadataIndex() { @@ -536,7 +806,7 @@ public void testMetadataIndex() { assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -571,12 +841,12 @@ public void testProfile() { .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).put("index.routing.rebalance.enable", "none")) .get(); waitForNoInitializingShards(client(LOCAL_CLUSTER), TimeValue.timeValueSeconds(30), "logs-1"); - client(REMOTE_CLUSTER).admin() + client(REMOTE_CLUSTER_1).admin() .indices() .prepareUpdateSettings("logs-2") .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).put("index.routing.rebalance.enable", "none")) .get(); - waitForNoInitializingShards(client(REMOTE_CLUSTER), TimeValue.timeValueSeconds(30), "logs-2"); + waitForNoInitializingShards(client(REMOTE_CLUSTER_1), TimeValue.timeValueSeconds(30), "logs-2"); final int localOnlyProfiles; { EsqlQueryRequest request = EsqlQueryRequest.syncEsqlQueryRequest(); @@ -593,7 +863,7 @@ public void testProfile() { EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertNotNull(executionInfo); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertNull(remoteCluster); assertThat(executionInfo.isCrossClusterSearch(), is(false)); assertThat(executionInfo.includeCCSMetadata(), is(false)); @@ -621,7 +891,7 @@ public void testProfile() { assertThat(executionInfo.includeCCSMetadata(), is(false)); assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -654,7 +924,7 @@ public void testProfile() { assertThat(executionInfo.includeCCSMetadata(), is(false)); assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -704,7 +974,7 @@ public void testWarnings() throws Exception { assertThat(executionInfo.includeCCSMetadata(), is(false)); assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER); + EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); assertThat(remoteCluster.getIndexExpression(), equalTo("logs*")); assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); @@ -792,13 +1062,18 @@ void waitForNoInitializingShards(Client client, TimeValue timeout, String... ind } Map setupTwoClusters() { + return setupClusters(2); + } + + Map setupClusters(int numClusters) { + assert numClusters == 2 || numClusters == 3 : "2 or 3 clusters supported not: " + numClusters; String localIndex = "logs-1"; int numShardsLocal = randomIntBetween(1, 5); populateLocalIndices(localIndex, numShardsLocal); String remoteIndex = "logs-2"; int numShardsRemote = randomIntBetween(1, 5); - populateRemoteIndices(remoteIndex, numShardsRemote); + populateRemoteIndices(REMOTE_CLUSTER_1, remoteIndex, numShardsRemote); Map clusterInfo = new HashMap<>(); clusterInfo.put("local.num_shards", numShardsLocal); @@ -806,8 +1081,15 @@ Map setupTwoClusters() { clusterInfo.put("remote.num_shards", numShardsRemote); clusterInfo.put("remote.index", remoteIndex); - String skipUnavailableKey = Strings.format("cluster.remote.%s.skip_unavailable", REMOTE_CLUSTER); - Setting skipUnavailableSetting = cluster(REMOTE_CLUSTER).clusterService().getClusterSettings().get(skipUnavailableKey); + if (numClusters == 3) { + int numShardsRemote2 = randomIntBetween(1, 5); + populateRemoteIndices(REMOTE_CLUSTER_2, remoteIndex, numShardsRemote2); + clusterInfo.put("remote2.index", remoteIndex); + clusterInfo.put("remote2.num_shards", numShardsRemote2); + } + + String skipUnavailableKey = Strings.format("cluster.remote.%s.skip_unavailable", REMOTE_CLUSTER_1); + Setting skipUnavailableSetting = cluster(REMOTE_CLUSTER_1).clusterService().getClusterSettings().get(skipUnavailableKey); boolean skipUnavailable = (boolean) cluster(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY).clusterService() .getClusterSettings() .get(skipUnavailableSetting); @@ -816,6 +1098,7 @@ Map setupTwoClusters() { return clusterInfo; } + void populateLocalIndices(String indexName, int numShards) { Client localClient = client(LOCAL_CLUSTER); assertAcked( @@ -831,8 +1114,8 @@ void populateLocalIndices(String indexName, int numShards) { localClient.admin().indices().prepareRefresh(indexName).get(); } - void populateRemoteIndices(String indexName, int numShards) { - Client remoteClient = client(REMOTE_CLUSTER); + void populateRemoteIndices(String clusterAlias, String indexName, int numShards) { + Client remoteClient = client(clusterAlias); assertAcked( remoteClient.admin() .indices() @@ -845,4 +1128,22 @@ void populateRemoteIndices(String indexName, int numShards) { } remoteClient.admin().indices().prepareRefresh(indexName).get(); } + + private void setSkipUnavailable(String clusterAlias, boolean skip) { + client(LOCAL_CLUSTER).admin() + .cluster() + .prepareUpdateSettings(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT) + .setPersistentSettings(Settings.builder().put("cluster.remote." + clusterAlias + ".skip_unavailable", skip).build()) + .get(); + } + + private void clearSkipUnavailable() { + Settings.Builder settingsBuilder = Settings.builder().putNull("cluster.remote." + REMOTE_CLUSTER_1 + ".skip_unavailable") + .putNull("cluster.remote." + REMOTE_CLUSTER_2 + ".skip_unavailable"); + client(LOCAL_CLUSTER).admin() + .cluster() + .prepareUpdateSettings(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT) + .setPersistentSettings(settingsBuilder.build()) + .get(); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 80709d8f6c4f7..329f8f2b56c79 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -17,6 +17,7 @@ import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.transport.RemoteTransportException; +import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlExecutionInfo; import org.elasticsearch.xpack.esql.analysis.Analyzer; import org.elasticsearch.xpack.esql.index.IndexResolution; @@ -33,7 +34,6 @@ class EsqlSessionCCSUtils { private EsqlSessionCCSUtils() {} - // visible for testing static Map determineUnavailableRemoteClusters(List failures) { Map unavailableRemotes = new HashMap<>(); for (FieldCapabilitiesFailure failure : failures) { @@ -132,7 +132,6 @@ static void updateExecutionInfoToReturnEmptyResult(EsqlExecutionInfo executionIn } } - // visible for testing static String createIndexExpressionFromAvailableClusters(EsqlExecutionInfo executionInfo) { StringBuilder sb = new StringBuilder(); for (String clusterAlias : executionInfo.clusterAliases()) { @@ -181,7 +180,6 @@ static void updateExecutionInfoWithUnavailableClusters(EsqlExecutionInfo execInf } } - // visible for testing static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionInfo executionInfo, IndexResolution indexResolution) { Set clustersWithResolvedIndices = new HashSet<>(); // determine missing clusters @@ -191,29 +189,58 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionIn Set clustersRequested = executionInfo.clusterAliases(); Set clustersWithNoMatchingIndices = Sets.difference(clustersRequested, clustersWithResolvedIndices); clustersWithNoMatchingIndices.removeAll(indexResolution.getUnavailableClusters().keySet()); + + /** + * The rules to determine whether missing indices is an error or not are: + * 1. no matching indices on any cluster: error MP TODO: I think this is handled elsewhere? Where? Check this + * ✓ 2. skip_unavailable=false remotes with no matching indices: error + * 3. missing concrete index expression: + * (a) error for local and skip_unavailable=false remotes + * (b) not an error for skip_unavailable=true remotes as long as there is at least one other matching index expression in the query (for any cluster) + * 4. wildcard index expressions, if at least one matches: not an error + */ + + String fatalErrorMessage = null; /* + * MP TODO: update this code comment * These are clusters in the original request that are not present in the field-caps response. They were * specified with an index or indices that do not exist, so the search on that cluster is done. * Mark it as SKIPPED with 0 shards searched and took=0. */ for (String c : clustersWithNoMatchingIndices) { - // TODO: in a follow-on PR, throw a Verification(400 status code) for local and remotes with skip_unavailable=false if - // they were requested with one or more concrete indices - // for now we never mark the local cluster as SKIPPED - final var status = RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY.equals(c) - ? EsqlExecutionInfo.Cluster.Status.SUCCESSFUL - : EsqlExecutionInfo.Cluster.Status.SKIPPED; - executionInfo.swapCluster( - c, - (k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setStatus(status) - .setTook(new TimeValue(0)) - .setTotalShards(0) - .setSuccessfulShards(0) - .setSkippedShards(0) - .setFailedShards(0) - .build() - ); + if (executionInfo.isSkipUnavailable(c) == false)) { + String error = "No matching indices for [" + executionInfo.getCluster(c).getIndexExpression() + "]"; + if (c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY) == false) { + error += " on remote cluster [" + c + "]"; + } + if (fatalErrorMessage == null) { + fatalErrorMessage = error; + } else { + fatalErrorMessage += "; " + error; + } + } else { + EsqlExecutionInfo.Cluster.Status status = c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY) ? + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL : EsqlExecutionInfo.Cluster.Status.SKIPPED; + executionInfo.swapCluster( + c, + (k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setStatus(status) + .setTook(new TimeValue(0)) + .setTotalShards(0) + .setSuccessfulShards(0) + .setSkippedShards(0) + .setFailedShards(0) + .build() + ); + } } + if (fatalErrorMessage != null) { + throw new VerificationException(fatalErrorMessage); + } + } + + // MP TODO: is there a better method for doing this, say in RemoteClusterService or RemoteClusterAware? + private static boolean concreteIndexRequested(String indexExpression) { + return indexExpression.indexOf('*') < 0; } // visible for testing From 0669837149aba43affe217e076c217abb13f33da Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 6 Nov 2024 13:14:32 -0500 Subject: [PATCH 02/17] updateExecutionInfoWithClustersWithNoMatchingIndices updated to follow new rules; tests updated --- ...CrossClusterQueryUnavailableRemotesIT.java | 204 +++--- .../esql/action/CrossClustersQueryIT.java | 602 +++++++++++------- .../xpack/esql/plugin/ComputeService.java | 24 +- .../esql/session/EsqlSessionCCSUtils.java | 76 ++- .../session/EsqlSessionCCSUtilsTests.java | 51 +- 5 files changed, 584 insertions(+), 373 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java index c3b08420cfa4f..0f1aa8541fdd9 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryUnavailableRemotesIT.java @@ -137,108 +137,108 @@ public void testCCSAgainstDisconnectedRemoteWithSkipUnavailableTrue() throws Exc } // scenario where there are no indices to match because -// // 1) the local cluster indexExpression and REMOTE_CLUSTER_2 indexExpression match no indices -// // 2) the REMOTE_CLUSTER_1 is unavailable -// // 3) both remotes are marked as skip_un=true -// String query = "FROM nomatch*," + REMOTE_CLUSTER_1 + ":logs-*," + REMOTE_CLUSTER_2 + ":nomatch* | STATS sum (v)"; -// try (EsqlQueryResponse resp = runQuery(query, requestIncludeMeta)) { -// List> values = getValuesList(resp); -// assertThat(values, hasSize(0)); -// -// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); -// assertNotNull(executionInfo); -// assertThat(executionInfo.isCrossClusterSearch(), is(true)); -// long overallTookMillis = executionInfo.overallTook().millis(); -// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); -// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); -// -// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); -// -// EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); -// assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); -// assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); -// assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(remote1Cluster.getTotalShards(), equalTo(0)); -// assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); -// assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); -// assertThat(remote1Cluster.getFailedShards(), equalTo(0)); -// -// EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); -// assertThat(remote2Cluster.getIndexExpression(), equalTo("nomatch*")); -// assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); -// assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(remote2Cluster.getTotalShards(), equalTo(0)); -// assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); -// assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); -// assertThat(remote2Cluster.getFailedShards(), equalTo(0)); -// -// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); -// assertThat(localCluster.getIndexExpression(), equalTo("nomatch*")); -// // local cluster should never be marked as SKIPPED -// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); -// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(localCluster.getTotalShards(), equalTo(0)); -// assertThat(localCluster.getSuccessfulShards(), equalTo(0)); -// assertThat(localCluster.getSkippedShards(), equalTo(0)); -// assertThat(localCluster.getFailedShards(), equalTo(0)); -// -// // ensure that the _clusters metadata is present only if requested -// assertClusterMetadataInResponse(resp, responseExpectMeta); -// } -// -// // close remote-cluster-2 so that it is also unavailable -// cluster(REMOTE_CLUSTER_2).close(); -// -// try (EsqlQueryResponse resp = runQuery("FROM logs-*,*:logs-* | STATS sum (v)", requestIncludeMeta)) { -// List> values = getValuesList(resp); -// assertThat(values, hasSize(1)); -// assertThat(values.get(0), equalTo(List.of(45L))); -// -// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); -// assertNotNull(executionInfo); -// assertThat(executionInfo.isCrossClusterSearch(), is(true)); -// long overallTookMillis = executionInfo.overallTook().millis(); -// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); -// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); -// -// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); -// -// EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); -// assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); -// assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); -// assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(remote1Cluster.getTotalShards(), equalTo(0)); -// assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); -// assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); -// assertThat(remote1Cluster.getFailedShards(), equalTo(0)); -// -// EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); -// assertThat(remote2Cluster.getIndexExpression(), equalTo("logs-*")); -// assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); -// assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(remote2Cluster.getTotalShards(), equalTo(0)); -// assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); -// assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); -// assertThat(remote2Cluster.getFailedShards(), equalTo(0)); -// -// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); -// assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); -// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); -// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); -// assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); -// assertThat(localCluster.getSkippedShards(), equalTo(0)); -// assertThat(localCluster.getFailedShards(), equalTo(0)); -// -// // ensure that the _clusters metadata is present only if requested -// assertClusterMetadataInResponse(resp, responseExpectMeta); -// } + // 1) the local cluster indexExpression and REMOTE_CLUSTER_2 indexExpression match no indices + // 2) the REMOTE_CLUSTER_1 is unavailable + // 3) both remotes are marked as skip_un=true + String query = "FROM nomatch*," + REMOTE_CLUSTER_1 + ":logs-*," + REMOTE_CLUSTER_2 + ":nomatch* | STATS sum (v)"; + try (EsqlQueryResponse resp = runQuery(query, requestIncludeMeta)) { + List> values = getValuesList(resp); + assertThat(values, hasSize(0)); + + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertNotNull(executionInfo); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + long overallTookMillis = executionInfo.overallTook().millis(); + assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); + + EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); + assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); + assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); + assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(remote1Cluster.getTotalShards(), equalTo(0)); + assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); + assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); + assertThat(remote1Cluster.getFailedShards(), equalTo(0)); + + EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); + assertThat(remote2Cluster.getIndexExpression(), equalTo("nomatch*")); + assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); + assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(remote2Cluster.getTotalShards(), equalTo(0)); + assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); + assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); + assertThat(remote2Cluster.getFailedShards(), equalTo(0)); + + EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); + assertThat(localCluster.getIndexExpression(), equalTo("nomatch*")); + // local cluster should never be marked as SKIPPED + assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); + assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(localCluster.getTotalShards(), equalTo(0)); + assertThat(localCluster.getSuccessfulShards(), equalTo(0)); + assertThat(localCluster.getSkippedShards(), equalTo(0)); + assertThat(localCluster.getFailedShards(), equalTo(0)); + + // ensure that the _clusters metadata is present only if requested + assertClusterMetadataInResponse(resp, responseExpectMeta); + } + + // close remote-cluster-2 so that it is also unavailable + cluster(REMOTE_CLUSTER_2).close(); + + try (EsqlQueryResponse resp = runQuery("FROM logs-*,*:logs-* | STATS sum (v)", requestIncludeMeta)) { + List> values = getValuesList(resp); + assertThat(values, hasSize(1)); + assertThat(values.get(0), equalTo(List.of(45L))); + + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertNotNull(executionInfo); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + long overallTookMillis = executionInfo.overallTook().millis(); + assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + + assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, REMOTE_CLUSTER_2, LOCAL_CLUSTER))); + + EsqlExecutionInfo.Cluster remote1Cluster = executionInfo.getCluster(REMOTE_CLUSTER_1); + assertThat(remote1Cluster.getIndexExpression(), equalTo("logs-*")); + assertThat(remote1Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); + assertThat(remote1Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(remote1Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(remote1Cluster.getTotalShards(), equalTo(0)); + assertThat(remote1Cluster.getSuccessfulShards(), equalTo(0)); + assertThat(remote1Cluster.getSkippedShards(), equalTo(0)); + assertThat(remote1Cluster.getFailedShards(), equalTo(0)); + + EsqlExecutionInfo.Cluster remote2Cluster = executionInfo.getCluster(REMOTE_CLUSTER_2); + assertThat(remote2Cluster.getIndexExpression(), equalTo("logs-*")); + assertThat(remote2Cluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); + assertThat(remote2Cluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(remote2Cluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(remote2Cluster.getTotalShards(), equalTo(0)); + assertThat(remote2Cluster.getSuccessfulShards(), equalTo(0)); + assertThat(remote2Cluster.getSkippedShards(), equalTo(0)); + assertThat(remote2Cluster.getFailedShards(), equalTo(0)); + + EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); + assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); + assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); + assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); + assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); + assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); + assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); + assertThat(localCluster.getSkippedShards(), equalTo(0)); + assertThat(localCluster.getFailedShards(), equalTo(0)); + + // ensure that the _clusters metadata is present only if requested + assertClusterMetadataInResponse(resp, responseExpectMeta); + } } finally { clearSkipUnavailable(numClusters); } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index 179a440c1f763..fa1d423398697 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.esql.action; import org.elasticsearch.Build; -import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.client.internal.Client; @@ -22,6 +21,7 @@ import org.elasticsearch.compute.operator.exchange.ExchangeService; import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; +import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.AbstractMultiClustersTestCase; import org.elasticsearch.test.InternalTestCluster; @@ -50,6 +50,7 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; @@ -174,14 +175,72 @@ public void testSuccessfulPathways() { } } - public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { + public void testSearchesAgainstNonMatchingIndicesWithLocalOnly() { + Map testClusterInfo = setupClusters(2); + String localIndex = (String) testClusterInfo.get("local.index"); + + { + String q = "FROM nomatch," + localIndex; + IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> runQuery(q, false)); + assertThat(e.getDetailedMessage(), containsString("no such index [nomatch]")); + + // MP TODO: am I able to fix this from the field-caps call? Yes, if we detect concrete vs. wildcard expressions in user query + // TODO bug - this does not throw; uncomment this test once https://github.com/elastic/elasticsearch/issues/114495 is fixed + // String limit0 = q + " | LIMIT 0"; + // VerificationException ve = expectThrows(VerificationException.class, () -> runQuery(limit0, false)); + // assertThat(ve.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + } + + { + // no failure since concrete index matches, so wildcard matching is lenient + String q = "FROM nomatch*," + localIndex; + try (EsqlQueryResponse resp = runQuery(q, false)) { + // we are only testing that this does not throw an Exception, so the asserts below are minimal + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(false)); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, false)) { + // we are only testing that this does not throw an Exception, so the asserts below are minimal + assertThat(resp.columns().size(), greaterThanOrEqualTo(1)); + assertThat(getValuesList(resp).size(), equalTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(false)); + } + } + { + String q = "FROM nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, false)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, false)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); + } + { + String q = "FROM nomatch*"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, false)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, false)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch*]")); + } + } + + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { Map testClusterInfo = setupClusters(3); int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); int remote2NumShards = (Integer) testClusterInfo.get("remote2.num_shards"); + String localIndex = (String) testClusterInfo.get("local.index"); + String remote1Index = (String) testClusterInfo.get("remote.index"); + String remote2Index = (String) testClusterInfo.get("remote2.index"); - setSkipUnavailable(REMOTE_CLUSTER_1, false); - setSkipUnavailable(REMOTE_CLUSTER_2, true); // MP TODO: what do here? randomBoolean()? + setSkipUnavailable(REMOTE_CLUSTER_1, true); + setSkipUnavailable(REMOTE_CLUSTER_2, true); Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); @@ -190,40 +249,71 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { try { // missing concrete local index is fatal { - String q = "FROM nomatch,cluster-a:logs*"; + String q = "FROM nomatch,cluster-a:" + remote1Index; VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); String limit0 = q + " | LIMIT 0"; e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); } - // missing concrete remote index is fatal when skip_unavailable=false + // missing concrete remote index is not fatal when skip_unavailable=true (as long as an index matches on another cluster) { - String q = "FROM logs*,cluster-a:nomatch,remote*:*"; - VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch] on remote cluster [cluster-a]")); + String q = String.format("FROM %s,cluster-a:nomatch", localIndex); + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + ) + ); + } String limit0 = q + " | LIMIT 0"; - e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch] on remote cluster [cluster-a]")); + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(resp.columns().size(), greaterThan(0)); + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + System.err.println(executionInfo); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + ) + ); + } } - // non-matching wildcarded local expression is not fatal + // since there is at least one matching index in the query, the missing wildcarded local index is not an error { - String q = "FROM nomatch*,*:logs*"; + String q = "FROM nomatch*,cluster-a:" + remote1Index; try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched - new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards), - new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster( + REMOTE_CLUSTER_1, + remote1Index, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote1NumShards + ) + ) + ); } String limit0 = q + " | LIMIT 0"; @@ -233,28 +323,55 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched - new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - // LIMIT 0 searches always have total shards = 0 - new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // LIMIT 0 searches always have total shards = 0 + new ExpectedCluster(REMOTE_CLUSTER_1, remote1Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); } } - // missing concrete remote index 'cluster-a:nomatch*' is fatal since cluster-a has skip_unavailable=false + // since at least one index of the query matches on some cluster, a wildcarded index on skip_un=true is not an error { - // MP TODO: waiting on feedback from Andrei - String q = "FROM logs*,cluster-a:nomatch*,remote*:logs*"; -// EsqlQueryResponse response = runQuery(q, requestIncludeMeta); -// System.err.println(response.getExecutionInfo()); -// VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); -// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*] on remote cluster [cluster-a]")); + String q = String.format("FROM %s,cluster-a:nomatch*", localIndex); + try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + ) + ); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(resp.columns().size(), greaterThan(0)); + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + ) + ); + } } - // an error is thrown if there are no matching indices at all - single remote cluster with concrete index expression + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=true { + // with non-matching concrete index String q = "FROM cluster-a:nomatch"; VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); @@ -264,8 +381,10 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); } - // an error is thrown if there are no matching indices at all - single remote cluster with wildcard index expression + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=true and the + // index was wildcarded { + // with non-matching wildcard index String q = "FROM cluster-a:nomatch*"; VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -319,138 +438,225 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); } + // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown { - String q = "FROM cluster-a:nomatch,remote*:*"; -// VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); -// assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + // TODO solve in follow-on PR which does skip_unavailable handling at execution time + // String q = String.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote1Index); + // try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { + // assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); + // EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + // assertThat(executionInfo.isCrossClusterSearch(), is(true)); + // assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + // assertExpectedClustersForMissingIndicesTests(executionInfo, List.of( + // // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + // new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + // new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + // )); + // } + + // TODO: handle LIMIT 0 for this case in follow-on PR + // String limit0 = q + " | LIMIT 0"; + // try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + // assertThat(resp.columns().size(), greaterThanOrEqualTo(1)); + // assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); + // EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + // assertThat(executionInfo.isCrossClusterSearch(), is(true)); + // assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + // assertExpectedClustersForMissingIndicesTests(executionInfo, List.of( + // // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + // new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch," + remote1Index + "*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + // )); + // } } - } finally { - clearSkipUnavailable(); - } - } - public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { - Map testClusterInfo = setupClusters(3); - int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); - int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); - int remote2NumShards = (Integer) testClusterInfo.get("remote2.num_shards"); + // tests with three clusters --- - setSkipUnavailable(REMOTE_CLUSTER_1, true); - setSkipUnavailable(REMOTE_CLUSTER_2, true); - - Tuple includeCCSMetadata = randomIncludeCCSMetadata(); - Boolean requestIncludeMeta = includeCCSMetadata.v1(); - boolean responseExpectMeta = includeCCSMetadata.v2(); - - try { - // missing concrete local index is fatal + // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown + // cluster-a should be marked as SKIPPED with VerificationException { - String q = "FROM nomatch,cluster-a:logs*"; - VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); - - String limit0 = q + " | LIMIT 0"; - e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); - assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch]")); - } - - // missing concrete remote index is not fatal when skip_unavailable=true - { - String q = "FROM logs*,cluster-a:nomatch,remote*:*"; + String q = String.format("FROM nomatch*,cluster-a:nomatch,%s:%s", REMOTE_CLUSTER_2, remote2Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), - new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster( + REMOTE_CLUSTER_2, + remote2Index, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote2NumShards + ) + ) + ); } String limit0 = q + " | LIMIT 0"; try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { - assertThat(resp.columns().size(), greaterThan(0)); + assertThat(resp.columns().size(), greaterThanOrEqualTo(1)); assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, remote2Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); } } - // non-matching wildcarded local expression is not fatal + // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown + // cluster-a should be marked as SKIPPED with a "NoMatchingIndicesException" since a wildcard index was requested { - String q = "FROM nomatch*,*:logs*"; + String q = String.format("FROM nomatch*,cluster-a:nomatch*,%s:%s", REMOTE_CLUSTER_2, remote2Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched - new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards), - new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster( + REMOTE_CLUSTER_2, + remote2Index, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote2NumShards + ) + ) + ); } String limit0 = q + " | LIMIT 0"; try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { - assertThat(getValuesList(resp).size(), equalTo(0)); - assertThat(resp.columns().size(), greaterThan(0)); + assertThat(resp.columns().size(), greaterThanOrEqualTo(1)); + assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched - new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - // LIMIT 0 searches always have total shards = 0 - new ExpectedCluster(REMOTE_CLUSTER_1, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, remote2Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); } } - // missing wildcarded remote index 'cluster-a:nomatch*' is not fatal since cluster-a has skip_unavailable=true + } finally { + clearSkipUnavailable(); + } + } + + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { + Map testClusterInfo = setupClusters(3); + int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); + String localIndex = (String) testClusterInfo.get("local.index"); + String remote1Index = (String) testClusterInfo.get("remote.index"); + String remote2Index = (String) testClusterInfo.get("remote2.index"); + + setSkipUnavailable(REMOTE_CLUSTER_1, false); + setSkipUnavailable(REMOTE_CLUSTER_2, false); + + Tuple includeCCSMetadata = randomIncludeCCSMetadata(); + Boolean requestIncludeMeta = includeCCSMetadata.v1(); + boolean responseExpectMeta = includeCCSMetadata.v2(); + + try { + // missing concrete local index is an error + { + String q = "FROM nomatch,cluster-a:" + remote1Index; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); + } + + // missing concrete remote index is fatal when skip_unavailable=false { - // MP TODO: waiting on feedback from Andrei - String q = "FROM logs*,cluster-a:nomatch*,remote*:*"; + String q = "FROM logs*,cluster-a:nomatch"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + } + + // No error since local non-matching has wildcard and the remote cluster matches + { + String q = String.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_1, remote1Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), - new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster( + REMOTE_CLUSTER_1, + remote1Index, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote1NumShards + ) + ) + ); } String limit0 = q + " | LIMIT 0"; try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(getValuesList(resp).size(), equalTo(0)); assertThat(resp.columns().size(), greaterThan(0)); - assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(0)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, "logs*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), - new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) - )); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched + new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // LIMIT 0 searches always have total shards = 0 + new ExpectedCluster(REMOTE_CLUSTER_1, remote1Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); } } - // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=false + // query is fatal since cluster-a has skip_unavailable=false and has no matching indices + { + String q = String.format("FROM %s,cluster-a:nomatch*", localIndex); + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + } + + // an error is thrown if there are no matching indices at all - single remote cluster with concrete index expression { - // with non-matching concrete index String q = "FROM cluster-a:nomatch"; VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); @@ -460,9 +666,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); } - // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=false (??? + // an error is thrown if there are no matching indices at all - single remote cluster with wildcard index expression { - // with non-matching wildcard index String q = "FROM cluster-a:nomatch*"; VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -516,22 +721,43 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); } - // since cluster-a is skip_unavailable=true and one cluster has a matching indices, no error is thrown + // Missing concrete index on skip_unavailable=false cluster is a fatal error, even when another index expression + // against that cluster matches { - String q = "FROM cluster-a:nomatch,remote*:*"; - try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { - assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - assertExpectedClusters(executionInfo, List.of( - // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched - new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) - )); - } + String q = String.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote2Index); + IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("no such index [nomatch]")); + + // TODO: in follow on PR, add support for throwing a VerificationException from this scenario + // String limit0 = q + " | LIMIT 0"; + // VerificationException e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + // assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,nomatch]")); + } + + // --- test against 3 clusters + + // skip_unavailable=false cluster having no matching indices is a fatal error. This error + // is fatal at plan time, so it throws VerificationException, not IndexNotFoundException (thrown at execution time) + { + String q = String.format("FROM %s*,cluster-a:nomatch,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); } + // skip_unavailable=false cluster having no matching indices is a fatal error (even if wildcarded) + { + String q = String.format("FROM %s*,cluster-a:nomatch*,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + } } finally { clearSkipUnavailable(); } @@ -539,7 +765,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { record ExpectedCluster(String clusterAlias, String indexExpression, EsqlExecutionInfo.Cluster.Status status, Integer totalShards) {} - public void assertExpectedClusters(EsqlExecutionInfo executionInfo, List expected) { + public void assertExpectedClustersForMissingIndicesTests(EsqlExecutionInfo executionInfo, List expected) { long overallTookMillis = executionInfo.overallTook().millis(); assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); @@ -560,62 +786,16 @@ public void assertExpectedClusters(EsqlExecutionInfo executionInfo, List testClusterInfo = setupClusters(3); - int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); - int remoteNumShards = (Integer) testClusterInfo.get("remote.num_shards"); - - setSkipUnavailable(REMOTE_CLUSTER_1, true); - setSkipUnavailable(REMOTE_CLUSTER_2, true); - - Tuple includeCCSMetadata = randomIncludeCCSMetadata(); - Boolean requestIncludeMeta = includeCCSMetadata.v1(); - boolean responseExpectMeta = includeCCSMetadata.v2(); - - // since a valid local index was specified, the invalid index on cluster-a does not throw an exception, - // but instead is simply ignored - ensure this is captured in the EsqlExecutionInfo - try (EsqlQueryResponse resp = runQuery("from logs-*,cluster-a:no_such_index | stats sum (v)", requestIncludeMeta)) { - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of(45L))); - - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); - - EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); - assertThat(remoteCluster.getIndexExpression(), equalTo("no_such_index")); - assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); - assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(remoteCluster.getTotalShards(), equalTo(0)); // 0 since no matching index, thus no shards to search - assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); - - EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); - assertThat(localCluster.getIndexExpression(), equalTo("logs-*")); - assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); - assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); - assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); - assertThat(localCluster.getTotalShards(), equalTo(localNumShards)); - assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); - assertThat(localCluster.getSkippedShards(), equalTo(0)); - assertThat(localCluster.getFailedShards(), equalTo(0)); - } - } - public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { Map testClusterInfo = setupTwoClusters(); int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); @@ -639,6 +819,10 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L)); } + // skip_un must be true for the next test or it will fail on "cluster-a:no_such_index*" with a + // VerificationException because there are no matching indices for that skip_un=false cluster. + setSkipUnavailable(REMOTE_CLUSTER_1, true); + // cluster-foo* matches nothing and so should not be present in the EsqlExecutionInfo try ( EsqlQueryResponse resp = runQuery( @@ -675,6 +859,8 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); assertThat(localCluster.getSkippedShards(), equalTo(0)); assertThat(localCluster.getFailedShards(), equalTo(0)); + } finally { + clearSkipUnavailable(); } } @@ -683,6 +869,9 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { * (which involves cross-cluster field-caps calls), it is a coordinator only operation at query time * which uses a different pathway compared to queries that require data node (and remote data node) operations * at query time. + * + * Note: the tests covering "nonmatching indices" also do LIMIT 0 tests. + * This one is mostly focuses on took time values. */ public void testCCSExecutionOnSearchesWithLimit0() { Map setupMap = setupTwoClusters(); @@ -732,53 +921,6 @@ public void testCCSExecutionOnSearchesWithLimit0() { assertThat(remoteCluster.getSkippedShards(), equalTo(0)); assertThat(remoteCluster.getFailedShards(), equalTo(0)); } - -// // MP TODO: delete this one? -// if (skipUnavailable == false) { -// VerificationException e = expectThrows(VerificationException.class, -// () -> runQuery("FROM logs*,cluster-a:nomatch* | LIMIT 0", requestIncludeMeta)); -// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*] on remote cluster [cluster-a]")); -// } else { -// try (EsqlQueryResponse resp = runQuery("FROM logs*,cluster-a:nomatch* | LIMIT 0", requestIncludeMeta)) { -// EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); -// assertNotNull(executionInfo); -// assertThat(executionInfo.isCrossClusterSearch(), is(true)); -// long overallTookMillis = executionInfo.overallTook().millis(); -// assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); -// assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); -// assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER_1, LOCAL_CLUSTER))); -// -// EsqlExecutionInfo.Cluster remoteCluster = executionInfo.getCluster(REMOTE_CLUSTER_1); -// assertThat(remoteCluster.getIndexExpression(), equalTo("nomatch*")); -// assertThat(remoteCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SKIPPED)); -// assertThat(remoteCluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(remoteCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(remoteCluster.getTotalShards(), equalTo(0)); -// assertThat(remoteCluster.getSuccessfulShards(), equalTo(0)); -// assertThat(remoteCluster.getSkippedShards(), equalTo(0)); -// assertThat(remoteCluster.getFailedShards(), equalTo(0)); -// -// EsqlExecutionInfo.Cluster localCluster = executionInfo.getCluster(LOCAL_CLUSTER); -// assertThat(localCluster.getIndexExpression(), equalTo("logs*")); -// assertThat(localCluster.getStatus(), equalTo(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL)); -// assertThat(localCluster.getTook().millis(), greaterThanOrEqualTo(0L)); -// assertThat(localCluster.getTook().millis(), lessThanOrEqualTo(overallTookMillis)); -// assertThat(localCluster.getTotalShards(), equalTo(0)); -// assertThat(localCluster.getSuccessfulShards(), equalTo(0)); -// assertThat(localCluster.getSkippedShards(), equalTo(0)); -// assertThat(localCluster.getFailedShards(), equalTo(0)); -// } -// } -// { -// VerificationException e = expectThrows(VerificationException.class, -// () -> runQuery("FROM nomatch*,cluster-a:* | LIMIT 0", requestIncludeMeta)); -// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*]")); -// } -// { -// VerificationException e = expectThrows(VerificationException.class, -// () -> runQuery("FROM nomatch*,foo*,cluster-a:* | LIMIT 0", requestIncludeMeta)); -// assertThat(e.getDetailedMessage(), containsString("No matching indices for [nomatch*,foo*]")); -// } } public void testMetadataIndex() { @@ -1089,7 +1231,7 @@ Map setupClusters(int numClusters) { } String skipUnavailableKey = Strings.format("cluster.remote.%s.skip_unavailable", REMOTE_CLUSTER_1); - Setting skipUnavailableSetting = cluster(REMOTE_CLUSTER_1).clusterService().getClusterSettings().get(skipUnavailableKey); + Setting skipUnavailableSetting = cluster(REMOTE_CLUSTER_1).clusterService().getClusterSettings().get(skipUnavailableKey); boolean skipUnavailable = (boolean) cluster(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY).clusterService() .getClusterSettings() .get(skipUnavailableSetting); @@ -1098,7 +1240,6 @@ Map setupClusters(int numClusters) { return clusterInfo; } - void populateLocalIndices(String indexName, int numShards) { Client localClient = client(LOCAL_CLUSTER); assertAcked( @@ -1138,7 +1279,8 @@ private void setSkipUnavailable(String clusterAlias, boolean skip) { } private void clearSkipUnavailable() { - Settings.Builder settingsBuilder = Settings.builder().putNull("cluster.remote." + REMOTE_CLUSTER_1 + ".skip_unavailable") + Settings.Builder settingsBuilder = Settings.builder() + .putNull("cluster.remote." + REMOTE_CLUSTER_1 + ".skip_unavailable") .putNull("cluster.remote." + REMOTE_CLUSTER_2 + ".skip_unavailable"); client(LOCAL_CLUSTER).admin() .cluster() diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java index 108e70d7d3a50..e9192c92263e8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java @@ -251,19 +251,17 @@ private static void updateExecutionInfoAfterCoordinatorOnlyQuery(EsqlExecutionIn if (execInfo.isCrossClusterSearch()) { assert execInfo.planningTookTime() != null : "Planning took time should be set on EsqlExecutionInfo but is null"; for (String clusterAlias : execInfo.clusterAliases()) { - // took time and shard counts for SKIPPED clusters were added at end of planning, so only update other cases here - if (execInfo.getCluster(clusterAlias).getStatus() != EsqlExecutionInfo.Cluster.Status.SKIPPED) { - execInfo.swapCluster( - clusterAlias, - (k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setTook(execInfo.overallTook()) - .setStatus(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL) - .setTotalShards(0) - .setSuccessfulShards(0) - .setSkippedShards(0) - .setFailedShards(0) - .build() - ); - } + execInfo.swapCluster(clusterAlias, (k, v) -> { + var builder = new EsqlExecutionInfo.Cluster.Builder(v).setTook(execInfo.overallTook()) + .setTotalShards(0) + .setSuccessfulShards(0) + .setSkippedShards(0) + .setFailedShards(0); + if (v.getStatus() == EsqlExecutionInfo.Cluster.Status.RUNNING) { + builder.setStatus(EsqlExecutionInfo.Cluster.Status.SUCCESSFUL); + } + return builder.build(); + }); } } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 329f8f2b56c79..0318573bb685d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -75,10 +75,10 @@ public void onFailure(Exception e) { /** * Whether to return an empty result (HTTP status 200) for a CCS rather than a top level 4xx/5xx error. - * + *

* For cases where field-caps had no indices to search and the remotes were unavailable, we * return an empty successful response (200) if all remotes are marked with skip_unavailable=true. - * + *

* Note: a follow-on PR will expand this logic to handle cases where no indices could be found to match * on any of the requested clusters. */ @@ -191,46 +191,52 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionIn clustersWithNoMatchingIndices.removeAll(indexResolution.getUnavailableClusters().keySet()); /** - * The rules to determine whether missing indices is an error or not are: - * 1. no matching indices on any cluster: error MP TODO: I think this is handled elsewhere? Where? Check this - * ✓ 2. skip_unavailable=false remotes with no matching indices: error - * 3. missing concrete index expression: - * (a) error for local and skip_unavailable=false remotes - * (b) not an error for skip_unavailable=true remotes as long as there is at least one other matching index expression in the query (for any cluster) - * 4. wildcard index expressions, if at least one matches: not an error + * Rules enforced at planning time around non-matching indices + * P1. fail query if no matching indices on any cluster (VerificationException) - that is handled elsewhere (TODO: document where) + * P2. fail query if a skip_unavailable:false cluster has no matching indices (the local cluster already has this rule enforced at planning time) + * P3. fail query if the local cluster has no matching indices and a concrete index was specified */ - String fatalErrorMessage = null; /* - * MP TODO: update this code comment * These are clusters in the original request that are not present in the field-caps response. They were - * specified with an index or indices that do not exist, so the search on that cluster is done. + * specified with an index expression matched no indices, so the search on that cluster is done. * Mark it as SKIPPED with 0 shards searched and took=0. */ for (String c : clustersWithNoMatchingIndices) { - if (executionInfo.isSkipUnavailable(c) == false)) { - String error = "No matching indices for [" + executionInfo.getCluster(c).getIndexExpression() + "]"; - if (c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY) == false) { - error += " on remote cluster [" + c + "]"; - } + final String indexExpression = executionInfo.getCluster(c).getIndexExpression(); + if (missingIndicesIsFatal(c, executionInfo)) { + String error = Strings.format( + "Unknown index [%s]", + (c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY) ? indexExpression : c + ":" + indexExpression) + ); if (fatalErrorMessage == null) { fatalErrorMessage = error; } else { fatalErrorMessage += "; " + error; } } else { - EsqlExecutionInfo.Cluster.Status status = c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY) ? - EsqlExecutionInfo.Cluster.Status.SUCCESSFUL : EsqlExecutionInfo.Cluster.Status.SKIPPED; - executionInfo.swapCluster( - c, - (k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setStatus(status) + // handles local cluster (when no concrete indices requested) and skip_unavailable=true clusters + EsqlExecutionInfo.Cluster.Status status; + ShardSearchFailure failure; + if (c.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY)) { + status = EsqlExecutionInfo.Cluster.Status.SUCCESSFUL; + failure = null; + } else { + status = EsqlExecutionInfo.Cluster.Status.SKIPPED; + failure = new ShardSearchFailure(new VerificationException("Unknown index [" + indexExpression + "]")); + } + executionInfo.swapCluster(c, (k, v) -> { + var builder = new EsqlExecutionInfo.Cluster.Builder(v).setStatus(status) .setTook(new TimeValue(0)) .setTotalShards(0) .setSuccessfulShards(0) .setSkippedShards(0) - .setFailedShards(0) - .build() - ); + .setFailedShards(0); + if (failure != null) { + builder.setFailures(List.of(failure)); + } + return builder.build(); + }); } } if (fatalErrorMessage != null) { @@ -238,9 +244,27 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionIn } } + // visible for testing + static boolean missingIndicesIsFatal(String clusterAlias, EsqlExecutionInfo executionInfo) { + if (executionInfo.getCluster(clusterAlias).isSkipUnavailable()) { + return false; + } + // missing indices on local cluster is fatal only if a concrete index requested + if (clusterAlias.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY)) { + return concreteIndexRequested(executionInfo.getCluster(clusterAlias).getIndexExpression()); + } + return true; + } + // MP TODO: is there a better method for doing this, say in RemoteClusterService or RemoteClusterAware? private static boolean concreteIndexRequested(String indexExpression) { - return indexExpression.indexOf('*') < 0; + for (String expr : indexExpression.split(",")) { + // TODO need to also detect date math before this check? + if (expr.indexOf('*') < 0) { + return true; + } + } + return false; } // visible for testing diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java index e60024ecd5db4..00ce7f08a6d4b 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.transport.NoSuchRemoteClusterException; import org.elasticsearch.transport.RemoteClusterAware; import org.elasticsearch.transport.RemoteTransportException; +import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlExecutionInfo; import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.index.EsIndex; @@ -293,7 +294,7 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { EsqlExecutionInfo executionInfo = new EsqlExecutionInfo(true); executionInfo.swapCluster(localClusterAlias, (k, v) -> new EsqlExecutionInfo.Cluster(localClusterAlias, "logs*", false)); executionInfo.swapCluster(remote1Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote1Alias, "*", true)); - executionInfo.swapCluster(remote2Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote2Alias, "mylogs1,mylogs2,logs*", false)); + executionInfo.swapCluster(remote2Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote2Alias, "mylogs1,mylogs2,logs*", true)); EsIndex esIndex = new EsIndex( "logs*,remote2:mylogs1,remote2:mylogs2,remote2:logs*", @@ -342,7 +343,11 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { var failure = new FieldCapabilitiesFailure(new String[] { "logs-a" }, new NoSeedNodeLeftException("unable to connect")); IndexResolution indexResolution = IndexResolution.valid(esIndex, Map.of(remote1Alias, failure)); - EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution); + VerificationException ve = expectThrows( + VerificationException.class, + () -> EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution) + ); + assertThat(ve.getDetailedMessage(), containsString("Unknown index [remote2:mylogs1,mylogs2,logs*]")); } } @@ -579,4 +584,46 @@ public void testUpdateExecutionInfoToReturnEmptyResult() { assertThat(remoteFailures.get(0).reason(), containsString("unable to connect to remote cluster")); } } + + public void testMissingIndicesIsFatal() { + String localClusterAlias = RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY; + String remote1Alias = "remote1"; + String remote2Alias = "remote2"; + String remote3Alias = "remote3"; + + // scenario 1: cluster is skip_unavailable=true - not fatal + { + EsqlExecutionInfo executionInfo = new EsqlExecutionInfo(true); + executionInfo.swapCluster(localClusterAlias, (k, v) -> new EsqlExecutionInfo.Cluster(localClusterAlias, "logs*", false)); + executionInfo.swapCluster(remote1Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote1Alias, "mylogs1,mylogs2,logs*", true)); + assertThat(EsqlSessionCCSUtils.missingIndicesIsFatal(remote1Alias, executionInfo), equalTo(false)); + } + + // scenario 2: cluster is local cluster and had no concrete indices - not fatal + { + EsqlExecutionInfo executionInfo = new EsqlExecutionInfo(true); + executionInfo.swapCluster(localClusterAlias, (k, v) -> new EsqlExecutionInfo.Cluster(localClusterAlias, "logs*", false)); + executionInfo.swapCluster(remote1Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote1Alias, "mylogs1,mylogs2,logs*", true)); + assertThat(EsqlSessionCCSUtils.missingIndicesIsFatal(localClusterAlias, executionInfo), equalTo(false)); + } + + // scenario 3: cluster is local cluster and user specified a concrete index - fatal + { + EsqlExecutionInfo executionInfo = new EsqlExecutionInfo(true); + String localIndexExpr = randomFrom("foo*,logs", "logs", "logs,metrics", "bar*,x*,logs", "logs-1,*x*"); + executionInfo.swapCluster(localClusterAlias, (k, v) -> new EsqlExecutionInfo.Cluster(localClusterAlias, localIndexExpr, false)); + executionInfo.swapCluster(remote1Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote1Alias, "mylogs1,mylogs2,logs*", true)); + assertThat(EsqlSessionCCSUtils.missingIndicesIsFatal(localClusterAlias, executionInfo), equalTo(true)); + } + + // scenario 4: cluster is skip_unavailable=false - always fatal + { + EsqlExecutionInfo executionInfo = new EsqlExecutionInfo(true); + executionInfo.swapCluster(localClusterAlias, (k, v) -> new EsqlExecutionInfo.Cluster(localClusterAlias, "*", false)); + String indexExpr = randomFrom("foo*,logs", "logs", "bar*,x*,logs", "logs-1,*x*", "*"); + executionInfo.swapCluster(remote1Alias, (k, v) -> new EsqlExecutionInfo.Cluster(remote1Alias, indexExpr, false)); + assertThat(EsqlSessionCCSUtils.missingIndicesIsFatal(remote1Alias, executionInfo), equalTo(true)); + } + + } } From d89845ba369d5bf65fb8490d9a978079ba4e4130 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 6 Nov 2024 13:19:08 -0500 Subject: [PATCH 03/17] Update docs/changelog/116348.yaml --- docs/changelog/116348.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/116348.yaml diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml new file mode 100644 index 0000000000000..7fa3083b0a6e4 --- /dev/null +++ b/docs/changelog/116348.yaml @@ -0,0 +1,6 @@ +pr: 116348 +summary: "DRAFT: ESQL honors `skip_unavailable` setting for nonmatching indices errors\ + \ at planning time" +area: ES|QL +type: enhancement +issues: [] From 1b0dc2ee6ac7045d752577013d9dac2e5e00e859 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 6 Nov 2024 13:43:55 -0500 Subject: [PATCH 04/17] Fixed checkstyle issues --- .../elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java | 3 ++- .../elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java index 16fa041b7f203..c6f74cbc72d43 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java @@ -61,7 +61,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -// MP TODO: what to do about missing enrich policies on skip_unavailable=true clusters? Should that be fatal or non-fatal? Write tests for those cases. +// MP TODO: what to do about missing enrich policies on skip_unavailable=true clusters? Should that be fatal or non-fatal? +// Write tests for those cases. public class CrossClustersEnrichIT extends AbstractMultiClustersTestCase { @Override diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 0318573bb685d..b65b8bcd30db0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -193,7 +193,8 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionIn /** * Rules enforced at planning time around non-matching indices * P1. fail query if no matching indices on any cluster (VerificationException) - that is handled elsewhere (TODO: document where) - * P2. fail query if a skip_unavailable:false cluster has no matching indices (the local cluster already has this rule enforced at planning time) + * P2. fail query if a skip_unavailable:false cluster has no matching indices (the local cluster already has this rule + * enforced at planning time) * P3. fail query if the local cluster has no matching indices and a concrete index was specified */ String fatalErrorMessage = null; From 3e824b1efc6e7dbae0d3bf2b2888d647f9211480 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Thu, 7 Nov 2024 10:47:07 -0500 Subject: [PATCH 05/17] It turns out we cannot rely on Map indexNameWithModes in EsIndex to always have the list of indices resolved by field-caps because when there are no mappings found in that index we pass in an empty map (which seems wrong). I tried to stop doing that but soon hit layers of ESQL I don't understand, so tried the simplier approach of adding a new (somewhat redundant) new field to IndexResolver: Set resolvedIndices. This solves the problem of allowing the CCS handler code in EsqlSession.preAnalyze (which calls EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices) to determine whether or not a cluster had no matching indices from the field-caps call, allowing either an error to be thrown or for the CCS metadata to be updated. --- .../xpack/esql/index/IndexResolution.java | 40 ++++++++++++++----- .../xpack/esql/session/EsqlSession.java | 3 +- .../esql/session/EsqlSessionCCSUtils.java | 2 +- .../xpack/esql/session/IndexResolver.java | 20 ++++++---- .../elasticsearch/xpack/esql/CsvTests.java | 3 +- .../esql/analysis/AnalyzerTestUtils.java | 3 +- .../xpack/esql/analysis/AnalyzerTests.java | 9 +++-- .../xpack/esql/analysis/ParsingTests.java | 3 +- .../xpack/esql/analysis/VerifierTests.java | 3 +- .../LocalLogicalPlanOptimizerTests.java | 5 ++- .../LocalPhysicalPlanOptimizerTests.java | 2 +- .../optimizer/LogicalPlanOptimizerTests.java | 13 +++--- .../optimizer/PhysicalPlanOptimizerTests.java | 2 +- .../xpack/esql/planner/FilterTests.java | 3 +- .../esql/planner/QueryTranslatorTests.java | 3 +- .../esql/plugin/DataNodeRequestTests.java | 3 +- .../session/EsqlSessionCCSUtilsTests.java | 11 +++-- 17 files changed, 84 insertions(+), 44 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java index b2eaefcf09d65..ca64e452bc829 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java @@ -12,22 +12,28 @@ import java.util.Collections; import java.util.Map; import java.util.Objects; +import java.util.Set; public final class IndexResolution { - public static IndexResolution valid(EsIndex index, Map unavailableClusters) { + public static IndexResolution valid( + EsIndex index, + Set resolvedIndices, + Map unavailableClusters + ) { Objects.requireNonNull(index, "index must not be null if it was found"); + Objects.requireNonNull(resolvedIndices, "resolvedIndices must not be null"); Objects.requireNonNull(unavailableClusters, "unavailableClusters must not be null"); - return new IndexResolution(index, null, unavailableClusters); + return new IndexResolution(index, null, resolvedIndices, unavailableClusters); } - public static IndexResolution valid(EsIndex index) { - return valid(index, Collections.emptyMap()); + public static IndexResolution valid(EsIndex index, Set resolvedIndices) { + return valid(index, resolvedIndices, Collections.emptyMap()); } public static IndexResolution invalid(String invalid) { Objects.requireNonNull(invalid, "invalid must not be null to signal that the index is invalid"); - return new IndexResolution(null, invalid, Collections.emptyMap()); + return new IndexResolution(null, invalid, Collections.emptySet(), Collections.emptyMap()); } public static IndexResolution notFound(String name) { @@ -39,12 +45,20 @@ public static IndexResolution notFound(String name) { @Nullable private final String invalid; + // all indices found by field-caps + private final Set resolvedIndices; // remote clusters included in the user's index expression that could not be connected to private final Map unavailableClusters; - private IndexResolution(EsIndex index, @Nullable String invalid, Map unavailableClusters) { + private IndexResolution( + EsIndex index, + @Nullable String invalid, + Set resolvedIndices, + Map unavailableClusters + ) { this.index = index; this.invalid = invalid; + this.resolvedIndices = resolvedIndices; this.unavailableClusters = unavailableClusters; } @@ -64,8 +78,8 @@ public EsIndex get() { } /** - * Is the index valid for use with ql? Returns {@code false} if the - * index wasn't found. + * Is the index valid for use with ql? + * @return {@code false} if the index wasn't found. */ public boolean isValid() { return invalid == null; @@ -79,6 +93,13 @@ public Map getUnavailableClusters() { return unavailableClusters; } + /** + * @return all indices found by field-caps (regardless of whether they had any mappings) + */ + public Set getResolvedIndices() { + return resolvedIndices; + } + @Override public boolean equals(Object obj) { if (obj == null || obj.getClass() != getClass()) { @@ -87,12 +108,13 @@ public boolean equals(Object obj) { IndexResolution other = (IndexResolution) obj; return Objects.equals(index, other.index) && Objects.equals(invalid, other.invalid) + && Objects.equals(resolvedIndices, other.resolvedIndices) && Objects.equals(unavailableClusters, other.unavailableClusters); } @Override public int hashCode() { - return Objects.hash(index, invalid, unavailableClusters); + return Objects.hash(index, invalid, resolvedIndices, unavailableClusters); } @Override diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 504689fdac39b..244fb10127208 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -72,6 +72,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -386,7 +387,7 @@ private void preAnalyzeIndices( String indexExpressionToResolve = EsqlSessionCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo); if (indexExpressionToResolve.isEmpty()) { // if this was a pure remote CCS request (no local indices) and all remotes are offline, return an empty IndexResolution - listener.onResponse(IndexResolution.valid(new EsIndex(table.index(), Map.of(), Map.of()))); + listener.onResponse(IndexResolution.valid(new EsIndex(table.index(), Map.of(), Map.of()), Collections.emptySet())); } else { // call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types indexResolver.resolveAsMergedMapping(indexExpressionToResolve, fieldNames, listener); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index b65b8bcd30db0..8f099b2739838 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -183,7 +183,7 @@ static void updateExecutionInfoWithUnavailableClusters(EsqlExecutionInfo execInf static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionInfo executionInfo, IndexResolution indexResolution) { Set clustersWithResolvedIndices = new HashSet<>(); // determine missing clusters - for (String indexName : indexResolution.get().indexNameWithModes().keySet()) { + for (String indexName : indexResolution.getResolvedIndices()) { clustersWithResolvedIndices.add(RemoteClusterAware.parseClusterAlias(indexName)); } Set clustersRequested = executionInfo.clusterAliases(); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java index 210f991306bac..0be8cf820d345 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.session; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.fieldcaps.FieldCapabilitiesFailure; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesIndexResponse; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse; @@ -143,21 +144,24 @@ public IndexResolution mergedMappings(String indexPattern, FieldCapabilitiesResp fields.put(name, field); } + Map unavailableRemotes = EsqlSessionCCSUtils.determineUnavailableRemoteClusters( + fieldCapsResponse.getFailures() + ); + + Map concreteIndices = Maps.newMapWithExpectedSize(fieldCapsResponse.getIndexResponses().size()); + for (FieldCapabilitiesIndexResponse ir : fieldCapsResponse.getIndexResponses()) { + concreteIndices.put(ir.getIndexName(), ir.getIndexMode()); + } + boolean allEmpty = true; for (FieldCapabilitiesIndexResponse ir : fieldCapsResponse.getIndexResponses()) { allEmpty &= ir.get().isEmpty(); } if (allEmpty) { // If all the mappings are empty we return an empty set of resolved indices to line up with QL - return IndexResolution.valid(new EsIndex(indexPattern, rootFields, Map.of())); - } - - Map concreteIndices = Maps.newMapWithExpectedSize(fieldCapsResponse.getIndexResponses().size()); - for (FieldCapabilitiesIndexResponse ir : fieldCapsResponse.getIndexResponses()) { - concreteIndices.put(ir.getIndexName(), ir.getIndexMode()); + return IndexResolution.valid(new EsIndex(indexPattern, rootFields, Map.of()), concreteIndices.keySet(), unavailableRemotes); } - EsIndex esIndex = new EsIndex(indexPattern, rootFields, concreteIndices); - return IndexResolution.valid(esIndex, EsqlSessionCCSUtils.determineUnavailableRemoteClusters(fieldCapsResponse.getFailures())); + return IndexResolution.valid(new EsIndex(indexPattern, rootFields, concreteIndices), concreteIndices.keySet(), unavailableRemotes); } private boolean allNested(List caps) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 348ca4acd100e..32baabc687041 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -98,6 +98,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -328,7 +329,7 @@ private static IndexResolution loadIndexResolution(String mappingName, String in } } } - return IndexResolution.valid(new EsIndex(indexName, mapping, Map.of(indexName, IndexMode.STANDARD))); + return IndexResolution.valid(new EsIndex(indexName, mapping, Map.of(indexName, IndexMode.STANDARD)), Set.of(indexName)); } private static EnrichResolution loadEnrichPolicies() { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java index a63ee53cdd498..65ee3d205f830 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.GEO_MATCH_TYPE; import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.MATCH_TYPE; @@ -91,7 +92,7 @@ public static LogicalPlan analyze(String query, String mapping, QueryParams para public static IndexResolution loadMapping(String resource, String indexName) { EsIndex test = new EsIndex(indexName, EsqlTestUtils.loadMapping(resource)); - return IndexResolution.valid(test); + return IndexResolution.valid(test, Set.of(indexName)); } public static IndexResolution analyzerDefaultMapping() { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index c1b2adddfc838..c2e09d3cd405e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -103,7 +103,7 @@ public class AnalyzerTests extends ESTestCase { public void testIndexResolution() { EsIndex idx = new EsIndex("idx", Map.of()); - Analyzer analyzer = analyzer(IndexResolution.valid(idx)); + Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); var plan = analyzer.analyze(UNRESOLVED_RELATION); var limit = as(plan, Limit.class); @@ -120,7 +120,8 @@ public void testFailOnUnresolvedIndex() { public void testIndexWithClusterResolution() { EsIndex idx = new EsIndex("cluster:idx", Map.of()); - Analyzer analyzer = analyzer(IndexResolution.valid(idx)); + // MP TODO: is this right? should it be "idx" or "cluster:idx"? + Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of(idx.name()))); var plan = analyzer.analyze(UNRESOLVED_RELATION); var limit = as(plan, Limit.class); @@ -130,7 +131,7 @@ public void testIndexWithClusterResolution() { public void testAttributeResolution() { EsIndex idx = new EsIndex("idx", LoadMapping.loadMapping("mapping-one-field.json")); - Analyzer analyzer = analyzer(IndexResolution.valid(idx)); + Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); var plan = analyzer.analyze( new Eval(EMPTY, UNRESOLVED_RELATION, List.of(new Alias(EMPTY, "e", new UnresolvedAttribute(EMPTY, "emp_no")))) @@ -183,7 +184,7 @@ public void testAttributeResolutionOfChainedReferences() { public void testRowAttributeResolution() { EsIndex idx = new EsIndex("idx", Map.of()); - Analyzer analyzer = analyzer(IndexResolution.valid(idx)); + Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); var plan = analyzer.analyze( new Eval( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java index 3cafd42b731f6..7fa36ebbcfd94 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_CFG; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_VERIFIER; @@ -120,6 +121,6 @@ private String error(String query) { } private static IndexResolution loadIndexResolution(String name) { - return IndexResolution.valid(new EsIndex(INDEX_NAME, LoadMapping.loadMapping(name))); + return IndexResolution.valid(new EsIndex(INDEX_NAME, LoadMapping.loadMapping(name)), Set.of(INDEX_NAME)); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index d6cda4a3a9ff7..9f5be28d27d7c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -86,7 +86,8 @@ public void testUnsupportedAndMultiTypedFields() { new EsIndex( "test*", Map.of(unsupported, unsupportedField, multiTyped, multiTypedField, "int", unsupportedField, "double", multiTypedField) - ) + ), + ipIndices ); Analyzer analyzer = AnalyzerTestUtils.analyzer(indexWithUnsupportedAndMultiTypedField); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index baef20081a4f2..831a7f4c780a3 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -56,6 +56,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static org.elasticsearch.xpack.esql.EsqlTestUtils.L; @@ -92,7 +93,7 @@ public static void init() { mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); analyzer = new Analyzer( @@ -400,7 +401,7 @@ public void testSparseDocument() throws Exception { SearchStats searchStats = statsForExistingField("field000", "field001", "field002", "field003", "field004"); EsIndex index = new EsIndex("large", large, Map.of("large", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(index); + IndexResolution getIndexResult = IndexResolution.valid(index, Set.of("large")); var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); var analyzer = new Analyzer( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index 905ca190ebe79..ba1490c3e3064 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -140,7 +140,7 @@ public void init() { private Analyzer makeAnalyzer(String mappingFileName, EnrichResolution enrichResolution) { var mapping = loadMapping(mappingFileName); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, test.concreteIndices()); return new Analyzer( new AnalyzerContext(config, new EsqlFunctionRegistry(), getIndexResult, enrichResolution), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index fdc4935d457e9..6dfd75a2cf9fb 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -119,6 +119,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; @@ -210,7 +211,7 @@ public static void init() { // Most tests used data from the test index, so we load it here, and use it in the plan() function. mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, enrichResolution), TEST_VERIFIER @@ -219,7 +220,7 @@ public static void init() { // Some tests use data from the airports index, so we load it here, and use it in the plan_airports() function. mappingAirports = loadMapping("mapping-airports.json"); EsIndex airports = new EsIndex("airports", mappingAirports, Map.of("airports", IndexMode.STANDARD)); - IndexResolution getIndexResultAirports = IndexResolution.valid(airports); + IndexResolution getIndexResultAirports = IndexResolution.valid(airports, Set.of("airports")); analyzerAirports = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution), TEST_VERIFIER @@ -228,7 +229,7 @@ public static void init() { // Some tests need additional types, so we load that index here and use it in the plan_types() function. mappingTypes = loadMapping("mapping-all-types.json"); EsIndex types = new EsIndex("types", mappingTypes, Map.of("types", IndexMode.STANDARD)); - IndexResolution getIndexResultTypes = IndexResolution.valid(types); + IndexResolution getIndexResultTypes = IndexResolution.valid(types, Set.of("types")); analyzerTypes = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultTypes, enrichResolution), TEST_VERIFIER @@ -237,14 +238,14 @@ public static void init() { // Some tests use mappings from mapping-extra.json to be able to test more types so we load it here mappingExtra = loadMapping("mapping-extra.json"); EsIndex extra = new EsIndex("extra", mappingExtra, Map.of("extra", IndexMode.STANDARD)); - IndexResolution getIndexResultExtra = IndexResolution.valid(extra); + IndexResolution getIndexResultExtra = IndexResolution.valid(extra, Set.of("extra")); analyzerExtra = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultExtra, enrichResolution), TEST_VERIFIER ); metricMapping = loadMapping("k8s-mappings.json"); - var metricsIndex = IndexResolution.valid(new EsIndex("k8s", metricMapping, Map.of("k8s", IndexMode.TIME_SERIES))); + var metricsIndex = IndexResolution.valid(new EsIndex("k8s", metricMapping, Map.of("k8s", IndexMode.TIME_SERIES)), Set.of("k8s")); metricsAnalyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), metricsIndex, enrichResolution), TEST_VERIFIER @@ -4228,7 +4229,7 @@ private static boolean oneLeaveIsNull(Expression e) { public void testEmptyMappingIndex() { EsIndex empty = new EsIndex("empty_test", emptyMap(), Map.of()); - IndexResolution getIndexResultAirports = IndexResolution.valid(empty); + IndexResolution getIndexResultAirports = IndexResolution.valid(empty, Set.of("empty_test")); var analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution), TEST_VERIFIER diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java index 9f5d6440e4a06..0d645b0deffa2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java @@ -262,7 +262,7 @@ TestDataSource makeTestDataSource( ) { Map mapping = loadMapping(mappingFileName); EsIndex index = new EsIndex(indexName, mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(index); + IndexResolution getIndexResult = IndexResolution.valid(index, Set.of("test")); Analyzer analyzer = new Analyzer(new AnalyzerContext(config, functionRegistry, getIndexResult, enrichResolution), TEST_VERIFIER); return new TestDataSource(mapping, index, analyzer); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java index bb937700ef771..51c63c718cdc2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java @@ -46,6 +46,7 @@ import java.io.UncheckedIOException; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Arrays.asList; import static org.elasticsearch.index.query.QueryBuilders.rangeQuery; @@ -77,7 +78,7 @@ public static void init() { Map mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(EsqlTestUtils.TEST_CFG)); mapper = new Mapper(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java index cf90cf96fe683..2d5fd7bff5438 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; +import java.util.Set; import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; @@ -42,7 +43,7 @@ public class QueryTranslatorTests extends ESTestCase { private static Analyzer makeAnalyzer(String mappingFileName) { var mapping = loadMapping(mappingFileName); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); return new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, new EnrichResolution()), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java index 4553551c40cd3..188f6273b53e8 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomConfiguration; import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomTables; @@ -263,7 +264,7 @@ protected DataNodeRequest mutateInstance(DataNodeRequest in) throws IOException static LogicalPlan parse(String query) { Map mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test); + IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(TEST_CFG)); var analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, emptyPolicyResolution()), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java index 00ce7f08a6d4b..0faeb68d8dc71 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java @@ -229,7 +229,8 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { IndexMode.STANDARD ) ); - IndexResolution indexResolution = IndexResolution.valid(esIndex, Map.of()); + + IndexResolution indexResolution = IndexResolution.valid(esIndex, esIndex.concreteIndices(), Map.of()); EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution); @@ -267,7 +268,7 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { IndexMode.STANDARD ) ); - IndexResolution indexResolution = IndexResolution.valid(esIndex, Map.of()); + IndexResolution indexResolution = IndexResolution.valid(esIndex, esIndex.concreteIndices(), Map.of()); EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution); @@ -303,7 +304,7 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { ); // remote1 is unavailable var failure = new FieldCapabilitiesFailure(new String[] { "logs-a" }, new NoSeedNodeLeftException("unable to connect")); - IndexResolution indexResolution = IndexResolution.valid(esIndex, Map.of(remote1Alias, failure)); + IndexResolution indexResolution = IndexResolution.valid(esIndex, esIndex.concreteIndices(), Map.of(remote1Alias, failure)); EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution); @@ -342,7 +343,7 @@ public void testUpdateExecutionInfoWithClustersWithNoMatchingIndices() { ); var failure = new FieldCapabilitiesFailure(new String[] { "logs-a" }, new NoSeedNodeLeftException("unable to connect")); - IndexResolution indexResolution = IndexResolution.valid(esIndex, Map.of(remote1Alias, failure)); + IndexResolution indexResolution = IndexResolution.valid(esIndex, esIndex.concreteIndices(), Map.of(remote1Alias, failure)); VerificationException ve = expectThrows( VerificationException.class, () -> EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution) @@ -407,6 +408,8 @@ public void testDetermineUnavailableRemoteClusters() { } } + // MP TODO: need to add a test for the "bypass IndexMode" thingy in mergedMappings that the rest test caught + public void testUpdateExecutionInfoAtEndOfPlanning() { String localClusterAlias = RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY; String remote1Alias = "remote1"; From 33cd1378005707ddcdcabfd7293c5841660e9015 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Thu, 7 Nov 2024 11:20:07 -0500 Subject: [PATCH 06/17] Fixed forbiddenAPI issues in CrossClustersQueryIT --- .../esql/action/CrossClustersQueryIT.java | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index fa1d423398697..79536b2112aec 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -260,7 +260,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // missing concrete remote index is not fatal when skip_unavailable=true (as long as an index matches on another cluster) { - String q = String.format("FROM %s,cluster-a:nomatch", localIndex); + String q = Strings.format("FROM %s,cluster-a:nomatch", localIndex); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -282,7 +282,6 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); assertThat(executionInfo.isCrossClusterSearch(), is(true)); assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); - System.err.println(executionInfo); assertExpectedClustersForMissingIndicesTests( executionInfo, List.of( @@ -337,7 +336,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since at least one index of the query matches on some cluster, a wildcarded index on skip_un=true is not an error { - String q = String.format("FROM %s,cluster-a:nomatch*", localIndex); + String q = Strings.format("FROM %s,cluster-a:nomatch*", localIndex); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -441,7 +440,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown { // TODO solve in follow-on PR which does skip_unavailable handling at execution time - // String q = String.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote1Index); + // String q = Strings.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote1Index); // try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { // assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); // EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -475,7 +474,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown // cluster-a should be marked as SKIPPED with VerificationException { - String q = String.format("FROM nomatch*,cluster-a:nomatch,%s:%s", REMOTE_CLUSTER_2, remote2Index); + String q = Strings.format("FROM nomatch*,cluster-a:nomatch,%s:%s", REMOTE_CLUSTER_2, remote2Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -519,7 +518,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown // cluster-a should be marked as SKIPPED with a "NoMatchingIndicesException" since a wildcard index was requested { - String q = String.format("FROM nomatch*,cluster-a:nomatch*,%s:%s", REMOTE_CLUSTER_2, remote2Index); + String q = Strings.format("FROM nomatch*,cluster-a:nomatch*,%s:%s", REMOTE_CLUSTER_2, remote2Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -604,7 +603,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // No error since local non-matching has wildcard and the remote cluster matches { - String q = String.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_1, remote1Index); + String q = Strings.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_1, remote1Index); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -646,7 +645,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // query is fatal since cluster-a has skip_unavailable=false and has no matching indices { - String q = String.format("FROM %s,cluster-a:nomatch*", localIndex); + String q = Strings.format("FROM %s,cluster-a:nomatch*", localIndex); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -724,7 +723,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // Missing concrete index on skip_unavailable=false cluster is a fatal error, even when another index expression // against that cluster matches { - String q = String.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote2Index); + String q = Strings.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote2Index); IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("no such index [nomatch]")); @@ -739,7 +738,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // skip_unavailable=false cluster having no matching indices is a fatal error. This error // is fatal at plan time, so it throws VerificationException, not IndexNotFoundException (thrown at execution time) { - String q = String.format("FROM %s*,cluster-a:nomatch,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + String q = Strings.format("FROM %s*,cluster-a:nomatch,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); @@ -750,7 +749,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // skip_unavailable=false cluster having no matching indices is a fatal error (even if wildcarded) { - String q = String.format("FROM %s*,cluster-a:nomatch*,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + String q = Strings.format("FROM %s*,cluster-a:nomatch*,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -874,11 +873,7 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { * This one is mostly focuses on took time values. */ public void testCCSExecutionOnSearchesWithLimit0() { - Map setupMap = setupTwoClusters(); - boolean skipUnavailable = (Boolean) setupMap.get("remote.skip_unavailable"); - - System.err.println(skipUnavailable); - + setupTwoClusters(); Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); boolean responseExpectMeta = includeCCSMetadata.v2(); From c5bf23e0fa3bb843ad9636011e6a59dd6a999ca0 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 8 Nov 2024 10:45:42 -0500 Subject: [PATCH 07/17] Added CrossClusterEsqlRCS1MissingIndicesIT --- .../esql/action/CrossClustersQueryIT.java | 2 +- .../CrossClusterEsqlRCS1MissingIndicesIT.java | 574 ++++++++++++++++++ 2 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index 79536b2112aec..0732334fff712 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -449,7 +449,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // assertExpectedClustersForMissingIndicesTests(executionInfo, List.of( // // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched // new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - // new ExpectedCluster(REMOTE_CLUSTER_2, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) + // new ExpectedCluster(REMOTE_CLUSTER_1, "*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards) // )); // } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java new file mode 100644 index 0000000000000..0f39104511be0 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1MissingIndicesIT.java @@ -0,0 +1,574 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.remotecluster; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.json.JsonXContent; +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; + +/** + * Tests cross-cluster ES|QL queries under RCS1.0 security model for cases where index expressions do not match + * to ensure handling of those matches the expected rules defined in EsqlSessionCrossClusterUtils. + */ +public class CrossClusterEsqlRCS1MissingIndicesIT extends AbstractRemoteClusterSecurityTestCase { + + private static final AtomicBoolean SSL_ENABLED_REF = new AtomicBoolean(); + + static { + // remote cluster + fulfillingCluster = ElasticsearchCluster.local() + .name("fulfilling-cluster") + .nodes(1) + .module("x-pack-esql") + .module("x-pack-enrich") + .apply(commonClusterConfig) + .setting("remote_cluster.port", "0") + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + .node(0, spec -> spec.setting("remote_cluster_server.enabled", "true")) + .build(); + + // "local" cluster + queryCluster = ElasticsearchCluster.local() + .name("query-cluster") + .module("x-pack-esql") + .module("x-pack-enrich") + .apply(commonClusterConfig) + .setting("xpack.ml.enabled", "false") + .setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .build(); + } + + @ClassRule + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + private static final String INDEX1 = "points"; // on local cluster only + private static final String INDEX2 = "squares"; // on local and remote clusters + + record ExpectedCluster(String clusterAlias, String indexExpression, String status, Integer totalShards) {} + + @SuppressWarnings("unchecked") + public void assertExpectedClustersForMissingIndicesTests(Map responseMap, List expected) { + Map clusters = (Map) responseMap.get("_clusters"); + assertThat((int) responseMap.get("took"), greaterThan(0)); + + Map detailsMap = (Map) clusters.get("details"); + assertThat(detailsMap.size(), is(expected.size())); + + assertThat((int) clusters.get("total"), is(expected.size())); + assertThat((int) clusters.get("successful"), is((int) expected.stream().filter(ec -> ec.status().equals("successful")).count())); + assertThat((int) clusters.get("skipped"), is((int) expected.stream().filter(ec -> ec.status().equals("skipped")).count())); + assertThat((int) clusters.get("failed"), is((int) expected.stream().filter(ec -> ec.status().equals("failed")).count())); + + for (ExpectedCluster expectedCluster : expected) { + Map clusterDetails = (Map) detailsMap.get(expectedCluster.clusterAlias()); + String msg = expectedCluster.clusterAlias(); + + assertThat(msg, (int) clusterDetails.get("took"), greaterThan(0)); + assertThat(msg, clusterDetails.get("status"), is(expectedCluster.status())); + Map shards = (Map) clusterDetails.get("_shards"); + if (expectedCluster.totalShards() == null) { + assertThat(msg, (int) shards.get("total"), greaterThan(0)); + } else { + assertThat(msg, (int) shards.get("total"), is(expectedCluster.totalShards())); + } + + if (expectedCluster.status().equals("successful")) { + assertThat((int) shards.get("successful"), is((int) shards.get("total"))); + assertThat((int) shards.get("skipped"), is(0)); + + } else if (expectedCluster.status().equals("skipped")) { + assertThat((int) shards.get("successful"), is(0)); + assertThat((int) shards.get("skipped"), is((int) shards.get("total"))); + ArrayList failures = (ArrayList) clusterDetails.get("failures"); + assertThat(failures.size(), is(1)); + Map failure1 = (Map) failures.get(0); + Map innerReason = (Map) failure1.get("reason"); + String expectedMsg = "Unknown index [" + expectedCluster.indexExpression() + "]"; + assertThat(innerReason.get("reason").toString(), containsString(expectedMsg)); + assertThat(innerReason.get("type").toString(), containsString("verification_exception")); + + } else { + fail(msg + "; Unexpected status: " + expectedCluster.status()); + } + // currently failed shards is always zero - change this once we start allowing partial data for individual shard failures + assertThat((int) shards.get("failed"), is(0)); + } + } + + @SuppressWarnings("unchecked") + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() throws Exception { + setupRolesAndPrivileges(); + setupIndex(); + + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), true); + + // missing concrete local index is an error + { + String q = Strings.format("FROM nomatch,%s:%s | STATS count(*)", REMOTE_CLUSTER_ALIAS, INDEX2); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString("Unknown index [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), Matchers.containsString("Unknown index [nomatch]")); + } + + // missing concrete remote index is not fatal when skip_unavailable=true (as long as an index matches on another cluster) + { + String q = Strings.format("FROM %s,%s:nomatch | STATS count(*)", INDEX1, REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + Response response = client().performRequest(esqlRequest(limit1)); + assertOK(response); + + Map map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), greaterThanOrEqualTo(1)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + new ExpectedCluster("(local)", INDEX1, "successful", null), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch", "skipped", 0) + ) + ); + + String limit0 = q + " | LIMIT 0"; + response = client().performRequest(esqlRequest(limit0)); + assertOK(response); + + map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), is(0)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + new ExpectedCluster("(local)", INDEX1, "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch", "skipped", 0) + ) + ); + } + + // since there is at least one matching index in the query, the missing wildcarded local index is not an error + { + String q = Strings.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_ALIAS, INDEX2); + + String limit1 = q + " | LIMIT 1"; + Response response = client().performRequest(esqlRequest(limit1)); + assertOK(response); + + Map map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), greaterThanOrEqualTo(1)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster("(local)", "nomatch*", "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, INDEX2, "successful", null) + ) + ); + + String limit0 = q + " | LIMIT 0"; + response = client().performRequest(esqlRequest(limit0)); + assertOK(response); + + map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), is(0)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster("(local)", "nomatch*", "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, INDEX2, "successful", 0) + ) + ); + } + + // since at least one index of the query matches on some cluster, a wildcarded index on skip_un=true is not an error + { + String q = Strings.format("FROM %s,%s:nomatch*", INDEX1, REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + Response response = client().performRequest(esqlRequest(limit1)); + assertOK(response); + + Map map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), greaterThanOrEqualTo(1)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + new ExpectedCluster("(local)", INDEX1, "successful", null), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch*", "skipped", 0) + ) + ); + + String limit0 = q + " | LIMIT 0"; + response = client().performRequest(esqlRequest(limit0)); + assertOK(response); + + map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), is(0)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + new ExpectedCluster("(local)", INDEX1, "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch*", "skipped", 0) + ) + ); + } + + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=true + { + // with non-matching concrete index + String q = Strings.format("FROM %s:nomatch", REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + } + + // an error is thrown if there are no matching indices at all, even when the cluster is skip_unavailable=true and the + // index was wildcarded + { + String q = Strings.format("FROM %s:nomatch*", REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch*]", REMOTE_CLUSTER_ALIAS))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch*]", REMOTE_CLUSTER_ALIAS))); + } + + // an error is thrown if there are no matching indices at all + { + String localExpr = randomFrom("nomatch", "nomatch*"); + String remoteExpr = randomFrom("nomatch", "nomatch*"); + String q = Strings.format("FROM %s,%s:%s", localExpr, REMOTE_CLUSTER_ALIAS, remoteExpr); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString("Unknown index")); + assertThat(e.getMessage(), containsString(Strings.format("%s:%s", REMOTE_CLUSTER_ALIAS, remoteExpr))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), containsString("Unknown index")); + assertThat(e.getMessage(), containsString(Strings.format("%s:%s", REMOTE_CLUSTER_ALIAS, remoteExpr))); + } + + // TODO uncomment and test in follow-on PR which does skip_unavailable handling at execution time + // { + // String q = Strings.format("FROM %s,%s:nomatch,%s:%s*", INDEX1, REMOTE_CLUSTER_ALIAS, REMOTE_CLUSTER_ALIAS, INDEX2); + // + // String limit1 = q + " | LIMIT 1"; + // Response response = client().performRequest(esqlRequest(limit1)); + // assertOK(response); + // + // Map map = responseAsMap(response); + // assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + // assertThat(((ArrayList) map.get("values")).size(), greaterThanOrEqualTo(1)); + // + // assertExpectedClustersForMissingIndicesTests(map, + // List.of( + // new ExpectedCluster("(local)", INDEX1, "successful", null), + // new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch," + INDEX2 + "*", "skipped", 0) + // ) + // ); + // + // String limit0 = q + " | LIMIT 0"; + // response = client().performRequest(esqlRequest(limit0)); + // assertOK(response); + // + // map = responseAsMap(response); + // assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + // assertThat(((ArrayList) map.get("values")).size(), is(0)); + // + // assertExpectedClustersForMissingIndicesTests(map, + // List.of( + // new ExpectedCluster("(local)", INDEX1, "successful", 0), + // new ExpectedCluster(REMOTE_CLUSTER_ALIAS, "nomatch," + INDEX2 + "*", "skipped", 0) + // ) + // ); + // } + } + + @SuppressWarnings("unchecked") + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() throws Exception { + // Remote cluster is closed and skip_unavailable is set to false. + // Although the other cluster is open, we expect an Exception. + + setupRolesAndPrivileges(); + setupIndex(); + + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), false); + + // missing concrete local index is an error + { + String q = Strings.format("FROM nomatch,%s:%s | STATS count(*)", REMOTE_CLUSTER_ALIAS, INDEX2); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString("Unknown index [nomatch]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), Matchers.containsString("Unknown index [nomatch]")); + } + + // missing concrete remote index is not fatal when skip_unavailable=true (as long as an index matches on another cluster) + { + String q = Strings.format("FROM %s,%s:nomatch | STATS count(*)", INDEX1, REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), Matchers.containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + } + + // since there is at least one matching index in the query, the missing wildcarded local index is not an error + { + String q = Strings.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_ALIAS, INDEX2); + + String limit1 = q + " | LIMIT 1"; + Response response = client().performRequest(esqlRequest(limit1)); + assertOK(response); + + Map map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), greaterThanOrEqualTo(1)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster("(local)", "nomatch*", "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, INDEX2, "successful", null) + ) + ); + + String limit0 = q + " | LIMIT 0"; + response = client().performRequest(esqlRequest(limit0)); + assertOK(response); + + map = responseAsMap(response); + assertThat(((ArrayList) map.get("columns")).size(), greaterThanOrEqualTo(1)); + assertThat(((ArrayList) map.get("values")).size(), is(0)); + + assertExpectedClustersForMissingIndicesTests( + map, + List.of( + // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched + new ExpectedCluster("(local)", "nomatch*", "successful", 0), + new ExpectedCluster(REMOTE_CLUSTER_ALIAS, INDEX2, "successful", 0) + ) + ); + } + + // query is fatal since the remote cluster has skip_unavailable=false and has no matching indices + { + String q = Strings.format("FROM %s,%s:nomatch*", INDEX1, REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch*]", REMOTE_CLUSTER_ALIAS))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), Matchers.containsString(Strings.format("Unknown index [%s:nomatch*]", REMOTE_CLUSTER_ALIAS))); + } + + // an error is thrown if there are no matching indices at all + { + // with non-matching concrete index + String q = Strings.format("FROM %s:nomatch", REMOTE_CLUSTER_ALIAS); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + } + + // an error is thrown if there are no matching indices at all + { + String localExpr = randomFrom("nomatch", "nomatch*"); + String remoteExpr = randomFrom("nomatch", "nomatch*"); + String q = Strings.format("FROM %s,%s:%s", localExpr, REMOTE_CLUSTER_ALIAS, remoteExpr); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString("Unknown index")); + assertThat(e.getMessage(), containsString(Strings.format("%s:%s", REMOTE_CLUSTER_ALIAS, remoteExpr))); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + assertThat(e.getMessage(), containsString("Unknown index")); + assertThat(e.getMessage(), containsString(Strings.format("%s:%s", REMOTE_CLUSTER_ALIAS, remoteExpr))); + } + + // error since the remote cluster with skip_unavailable=false specified a concrete index that is not found + { + String q = Strings.format("FROM %s,%s:nomatch,%s:%s*", INDEX1, REMOTE_CLUSTER_ALIAS, REMOTE_CLUSTER_ALIAS, INDEX2); + + String limit1 = q + " | LIMIT 1"; + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit1))); + assertThat(e.getMessage(), containsString(Strings.format("no such index [nomatch]", REMOTE_CLUSTER_ALIAS))); + assertThat(e.getMessage(), containsString(Strings.format("index_not_found_exception", REMOTE_CLUSTER_ALIAS))); + + // TODO: in follow on PR, add support for throwing a VerificationException from this scenario + // String limit0 = q + " | LIMIT 0"; + // e = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(limit0))); + // assertThat(e.getMessage(), containsString(Strings.format("Unknown index [%s:nomatch]", REMOTE_CLUSTER_ALIAS))); + } + } + + private void setupRolesAndPrivileges() throws IOException { + var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + + var putRoleOnRemoteClusterRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleOnRemoteClusterRequest.setJsonEntity(""" + { + "indices": [ + { + "names": ["points", "squares"], + "privileges": ["read", "read_cross_cluster", "create_index", "monitor"] + } + ], + "remote_indices": [ + { + "names": ["points", "squares"], + "privileges": ["read", "read_cross_cluster", "create_index", "monitor"], + "clusters": ["my_remote_cluster"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleOnRemoteClusterRequest)); + } + + private void setupIndex() throws IOException { + Request createIndex = new Request("PUT", INDEX1); + createIndex.setJsonEntity(""" + { + "mappings": { + "properties": { + "id": { "type": "integer" }, + "score": { "type": "integer" } + } + } + } + """); + assertOK(client().performRequest(createIndex)); + + Request bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(""" + { "index": { "_index": "points" } } + { "id": 1, "score": 75} + { "index": { "_index": "points" } } + { "id": 2, "score": 125} + { "index": { "_index": "points" } } + { "id": 3, "score": 100} + { "index": { "_index": "points" } } + { "id": 4, "score": 50} + { "index": { "_index": "points" } } + { "id": 5, "score": 150} + """); + assertOK(client().performRequest(bulkRequest)); + + createIndex = new Request("PUT", INDEX2); + createIndex.setJsonEntity(""" + { + "mappings": { + "properties": { + "num": { "type": "integer" }, + "square": { "type": "integer" } + } + } + } + """); + assertOK(client().performRequest(createIndex)); + + bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(""" + { "index": {"_index": "squares"}} + { "num": 1, "square": 1 } + { "index": {"_index": "squares"}} + { "num": 4, "square": 4 } + { "index": {"_index": "squares"}} + { "num": 3, "square": 9 } + { "index": {"_index": "squares"}} + { "num": 4, "square": 16 } + """); + assertOK(performRequestAgainstFulfillingCluster(bulkRequest)); + } + + private Request esqlRequest(String query) throws IOException { + XContentBuilder body = JsonXContent.contentBuilder(); + + body.startObject(); + body.field("query", query); + body.field("include_ccs_metadata", true); + body.endObject(); + + Request request = new Request("POST", "_query"); + request.setJsonEntity(org.elasticsearch.common.Strings.toString(body)); + + return request; + } +} From d72f22ad6a135b606e9b7a7a7197cd449857ea56 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 8 Nov 2024 12:29:55 -0500 Subject: [PATCH 08/17] Added index aliases and filtered alias to CrossClustersQueryIT for nonmatching indices tests --- .../esql/action/CrossClustersQueryIT.java | 146 ++++++++++++++---- 1 file changed, 112 insertions(+), 34 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index 0732334fff712..2cdcc0ee7d798 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.Build; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Priority; @@ -22,6 +23,8 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.AbstractMultiClustersTestCase; import org.elasticsearch.test.InternalTestCluster; @@ -231,7 +234,8 @@ public void testSearchesAgainstNonMatchingIndicesWithLocalOnly() { } public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { - Map testClusterInfo = setupClusters(3); + int numClusters = 3; + Map testClusterInfo = setupClusters(numClusters); int localNumShards = (Integer) testClusterInfo.get("local.num_shards"); int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); int remote2NumShards = (Integer) testClusterInfo.get("remote2.num_shards"); @@ -239,6 +243,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { String remote1Index = (String) testClusterInfo.get("remote.index"); String remote2Index = (String) testClusterInfo.get("remote2.index"); + createIndexAliases(numClusters); setSkipUnavailable(REMOTE_CLUSTER_1, true); setSkipUnavailable(REMOTE_CLUSTER_2, true); @@ -249,7 +254,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { try { // missing concrete local index is fatal { - String q = "FROM nomatch,cluster-a:" + remote1Index; + String q = "FROM nomatch,cluster-a:" + randomFrom(remote1Index, IDX_ALIAS, FILTERED_IDX_ALIAS); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [nomatch]")); @@ -260,7 +265,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // missing concrete remote index is not fatal when skip_unavailable=true (as long as an index matches on another cluster) { - String q = Strings.format("FROM %s,cluster-a:nomatch", localIndex); + String localIndexName = randomFrom(localIndex, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM %s,cluster-a:nomatch", localIndexName); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -269,7 +275,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertExpectedClustersForMissingIndicesTests( executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) ) ); @@ -285,7 +291,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertExpectedClustersForMissingIndicesTests( executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) ) ); @@ -294,7 +300,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since there is at least one matching index in the query, the missing wildcarded local index is not an error { - String q = "FROM nomatch*,cluster-a:" + remote1Index; + String remoteIndexName = randomFrom(remote1Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = "FROM nomatch*,cluster-a:" + remoteIndexName; try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -307,7 +314,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster( REMOTE_CLUSTER_1, - remote1Index, + remoteIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards ) @@ -328,7 +335,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), // LIMIT 0 searches always have total shards = 0 - new ExpectedCluster(REMOTE_CLUSTER_1, remote1Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + new ExpectedCluster(REMOTE_CLUSTER_1, remoteIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) ) ); } @@ -336,7 +343,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since at least one index of the query matches on some cluster, a wildcarded index on skip_un=true is not an error { - String q = Strings.format("FROM %s,cluster-a:nomatch*", localIndex); + String localIndexName = randomFrom(localIndex, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM %s,cluster-a:nomatch*", localIndexName); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -345,7 +353,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertExpectedClustersForMissingIndicesTests( executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, localNumShards), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) ) ); @@ -361,7 +369,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { assertExpectedClustersForMissingIndicesTests( executionInfo, List.of( - new ExpectedCluster(LOCAL_CLUSTER, localIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) ) ); @@ -474,7 +482,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown // cluster-a should be marked as SKIPPED with VerificationException { - String q = Strings.format("FROM nomatch*,cluster-a:nomatch,%s:%s", REMOTE_CLUSTER_2, remote2Index); + String remote2IndexName = randomFrom(remote2Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM nomatch*,cluster-a:nomatch,%s:%s", REMOTE_CLUSTER_2, remote2IndexName); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -488,7 +497,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), new ExpectedCluster( REMOTE_CLUSTER_2, - remote2Index, + remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards ) @@ -509,7 +518,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, remote2Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + new ExpectedCluster(REMOTE_CLUSTER_2, remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) ) ); } @@ -518,7 +527,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // since cluster-a is skip_unavailable=true and at least one cluster has a matching indices, no error is thrown // cluster-a should be marked as SKIPPED with a "NoMatchingIndicesException" since a wildcard index was requested { - String q = Strings.format("FROM nomatch*,cluster-a:nomatch*,%s:%s", REMOTE_CLUSTER_2, remote2Index); + String remote2IndexName = randomFrom(remote2Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM nomatch*,cluster-a:nomatch*,%s:%s", REMOTE_CLUSTER_2, remote2IndexName); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -532,7 +542,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), new ExpectedCluster( REMOTE_CLUSTER_2, - remote2Index, + remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote2NumShards ) @@ -553,24 +563,25 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { // local cluster is never marked as SKIPPED even when no matching indices - just marked as 0 shards searched new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), - new ExpectedCluster(REMOTE_CLUSTER_2, remote2Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + new ExpectedCluster(REMOTE_CLUSTER_2, remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) ) ); } } - } finally { clearSkipUnavailable(); } } public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { - Map testClusterInfo = setupClusters(3); + int numClusters = 3; + Map testClusterInfo = setupClusters(numClusters); int remote1NumShards = (Integer) testClusterInfo.get("remote.num_shards"); String localIndex = (String) testClusterInfo.get("local.index"); String remote1Index = (String) testClusterInfo.get("remote.index"); String remote2Index = (String) testClusterInfo.get("remote2.index"); + createIndexAliases(numClusters); setSkipUnavailable(REMOTE_CLUSTER_1, false); setSkipUnavailable(REMOTE_CLUSTER_2, false); @@ -603,7 +614,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // No error since local non-matching has wildcard and the remote cluster matches { - String q = Strings.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_1, remote1Index); + String remote1IndexName = randomFrom(remote1Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM nomatch*,%s:%s", REMOTE_CLUSTER_1, remote1IndexName); try (EsqlQueryResponse resp = runQuery(q, requestIncludeMeta)) { assertThat(getValuesList(resp).size(), greaterThanOrEqualTo(1)); EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); @@ -616,7 +628,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), new ExpectedCluster( REMOTE_CLUSTER_1, - remote1Index, + remote1IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, remote1NumShards ) @@ -637,7 +649,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // local cluster is never marked as SKIPPED even when no matcing indices - just marked as 0 shards searched new ExpectedCluster(LOCAL_CLUSTER, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), // LIMIT 0 searches always have total shards = 0 - new ExpectedCluster(REMOTE_CLUSTER_1, remote1Index, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + new ExpectedCluster(REMOTE_CLUSTER_1, remote1IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) ) ); } @@ -645,7 +657,7 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // query is fatal since cluster-a has skip_unavailable=false and has no matching indices { - String q = Strings.format("FROM %s,cluster-a:nomatch*", localIndex); + String q = Strings.format("FROM %s,cluster-a:nomatch*", randomFrom(localIndex, IDX_ALIAS, FILTERED_IDX_ALIAS)); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -723,7 +735,8 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // Missing concrete index on skip_unavailable=false cluster is a fatal error, even when another index expression // against that cluster matches { - String q = Strings.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote2Index); + String remote2IndexName = randomFrom(remote2Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM %s,cluster-a:nomatch,cluster-a:%s*", localIndex, remote2IndexName); IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("no such index [nomatch]")); @@ -738,7 +751,9 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // skip_unavailable=false cluster having no matching indices is a fatal error. This error // is fatal at plan time, so it throws VerificationException, not IndexNotFoundException (thrown at execution time) { - String q = Strings.format("FROM %s*,cluster-a:nomatch,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + String localIndexName = randomFrom(localIndex, IDX_ALIAS, FILTERED_IDX_ALIAS); + String remote2IndexName = randomFrom(remote2Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM %s*,cluster-a:nomatch,%s:%s*", localIndexName, REMOTE_CLUSTER_2, remote2IndexName); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch]")); @@ -749,7 +764,9 @@ public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { // skip_unavailable=false cluster having no matching indices is a fatal error (even if wildcarded) { - String q = Strings.format("FROM %s*,cluster-a:nomatch*,%s:%s*", localIndex, REMOTE_CLUSTER_2, remote2Index); + String localIndexName = randomFrom(localIndex, IDX_ALIAS, FILTERED_IDX_ALIAS); + String remote2IndexName = randomFrom(remote2Index, IDX_ALIAS, FILTERED_IDX_ALIAS); + String q = Strings.format("FROM %s*,cluster-a:nomatch*,%s:%s*", localIndexName, REMOTE_CLUSTER_2, remote2IndexName); VerificationException e = expectThrows(VerificationException.class, () -> runQuery(q, requestIncludeMeta)); assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); @@ -1202,26 +1219,29 @@ Map setupTwoClusters() { return setupClusters(2); } + private static String LOCAL_INDEX = "logs-1"; + private static String IDX_ALIAS = "alias1"; + private static String FILTERED_IDX_ALIAS = "alias-filtered-1"; + private static String REMOTE_INDEX = "logs-2"; + Map setupClusters(int numClusters) { assert numClusters == 2 || numClusters == 3 : "2 or 3 clusters supported not: " + numClusters; - String localIndex = "logs-1"; int numShardsLocal = randomIntBetween(1, 5); - populateLocalIndices(localIndex, numShardsLocal); + populateLocalIndices(LOCAL_INDEX, numShardsLocal); - String remoteIndex = "logs-2"; int numShardsRemote = randomIntBetween(1, 5); - populateRemoteIndices(REMOTE_CLUSTER_1, remoteIndex, numShardsRemote); + populateRemoteIndices(REMOTE_CLUSTER_1, REMOTE_INDEX, numShardsRemote); Map clusterInfo = new HashMap<>(); clusterInfo.put("local.num_shards", numShardsLocal); - clusterInfo.put("local.index", localIndex); + clusterInfo.put("local.index", LOCAL_INDEX); clusterInfo.put("remote.num_shards", numShardsRemote); - clusterInfo.put("remote.index", remoteIndex); + clusterInfo.put("remote.index", REMOTE_INDEX); if (numClusters == 3) { int numShardsRemote2 = randomIntBetween(1, 5); - populateRemoteIndices(REMOTE_CLUSTER_2, remoteIndex, numShardsRemote2); - clusterInfo.put("remote2.index", remoteIndex); + populateRemoteIndices(REMOTE_CLUSTER_2, REMOTE_INDEX, numShardsRemote2); + clusterInfo.put("remote2.index", REMOTE_INDEX); clusterInfo.put("remote2.num_shards", numShardsRemote2); } @@ -1235,6 +1255,64 @@ Map setupClusters(int numClusters) { return clusterInfo; } + /** + * For the local cluster and REMOTE_CLUSTER_1 it creates a standard alias to the index created in populateLocalIndices + * and populateRemoteIndices. It also creates a filtered alias against those indices that looks like: + * PUT /_aliases + * { + * "actions": [ + * { + * "add": { + * "index": "my_index", + * "alias": "my_alias", + * "filter": { + * "terms": { + * "v": [1, 2, 4] + * } + * } + * } + * } + * ] + * } + */ + void createIndexAliases(int numClusters) { + assert numClusters == 2 || numClusters == 3 : "Only 2 or 3 clusters allowed in createIndexAliases"; + + int[] allowed = new int[] { 1, 2, 4 }; + QueryBuilder filterBuilder = new TermsQueryBuilder("v", allowed); + + { + Client localClient = client(LOCAL_CLUSTER); + IndicesAliasesResponse indicesAliasesResponse = localClient.admin() + .indices() + .prepareAliases() + .addAlias(LOCAL_INDEX, IDX_ALIAS) + .addAlias(LOCAL_INDEX, FILTERED_IDX_ALIAS, filterBuilder) + .get(); + assertFalse(indicesAliasesResponse.hasErrors()); + } + { + Client remoteClient = client(REMOTE_CLUSTER_1); + IndicesAliasesResponse indicesAliasesResponse = remoteClient.admin() + .indices() + .prepareAliases() + .addAlias(REMOTE_INDEX, IDX_ALIAS) + .addAlias(REMOTE_INDEX, FILTERED_IDX_ALIAS, filterBuilder) + .get(); + assertFalse(indicesAliasesResponse.hasErrors()); + } + if (numClusters == 3) { + Client remoteClient = client(REMOTE_CLUSTER_2); + IndicesAliasesResponse indicesAliasesResponse = remoteClient.admin() + .indices() + .prepareAliases() + .addAlias(REMOTE_INDEX, IDX_ALIAS) + .addAlias(REMOTE_INDEX, FILTERED_IDX_ALIAS, filterBuilder) + .get(); + assertFalse(indicesAliasesResponse.hasErrors()); + } + } + void populateLocalIndices(String indexName, int numShards) { Client localClient = client(LOCAL_CLUSTER); assertAcked( From a6196981ec3b2cec1808a322e11fac4f48447292 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 8 Nov 2024 16:36:56 -0500 Subject: [PATCH 09/17] Added tests against index with no mappings to ensure correct error message is returned Adjusted RemoteClusterSecurityEsqlIT so it passes for skip_unavailable=true, although the error message returned for skip_unavailable=false is still wrong. --- .../esql/action/CrossClustersQueryIT.java | 93 +++++++++++++++++++ .../session/EsqlSessionCCSUtilsTests.java | 2 - .../RemoteClusterSecurityEsqlIT.java | 55 ++++++++--- 3 files changed, 137 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java index 2cdcc0ee7d798..6801e1f4eb404 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersQueryIT.java @@ -40,6 +40,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -233,6 +234,64 @@ public void testSearchesAgainstNonMatchingIndicesWithLocalOnly() { } } + public void testSearchesAgainstIndicesWithNoMappingsSkipUnavailableTrue() { + int numClusters = 2; + setupClusters(numClusters); + Map clusterToEmptyIndexMap = createEmptyIndicesWithNoMappings(numClusters); + setSkipUnavailable(REMOTE_CLUSTER_1, randomBoolean()); + + Tuple includeCCSMetadata = randomIncludeCCSMetadata(); + Boolean requestIncludeMeta = includeCCSMetadata.v1(); + boolean responseExpectMeta = includeCCSMetadata.v2(); + + try { + String emptyIndex = clusterToEmptyIndexMap.get(REMOTE_CLUSTER_1); + String q = Strings.format("FROM cluster-a:%s", emptyIndex); + // query without referring to fields should work + { + String limit1 = q + " | LIMIT 1"; + try (EsqlQueryResponse resp = runQuery(limit1, requestIncludeMeta)) { + assertThat(resp.columns().size(), equalTo(1)); + assertThat(resp.columns().get(0).name(), equalTo("")); + assertThat(getValuesList(resp).size(), equalTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of(new ExpectedCluster(REMOTE_CLUSTER_1, emptyIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0)) + ); + } + + String limit0 = q + " | LIMIT 0"; + try (EsqlQueryResponse resp = runQuery(limit0, requestIncludeMeta)) { + assertThat(resp.columns().size(), equalTo(1)); + assertThat(resp.columns().get(0).name(), equalTo("")); + assertThat(getValuesList(resp).size(), equalTo(0)); + EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of(new ExpectedCluster(REMOTE_CLUSTER_1, emptyIndex, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0)) + ); + } + } + + // query that refers to missing fields should throw: + // "type": "verification_exception", + // "reason": "Found 1 problem\nline 2:7: Unknown column [foo]", + { + String keepQuery = q + " | KEEP foo | LIMIT 100"; + VerificationException e = expectThrows(VerificationException.class, () -> runQuery(keepQuery, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown column [foo]")); + } + + } finally { + clearSkipUnavailable(); + } + } + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableTrue() { int numClusters = 3; Map testClusterInfo = setupClusters(numClusters); @@ -1313,6 +1372,40 @@ void createIndexAliases(int numClusters) { } } + Map createEmptyIndicesWithNoMappings(int numClusters) { + assert numClusters == 2 || numClusters == 3 : "Only 2 or 3 clusters supported in createEmptyIndicesWithNoMappings"; + + Map clusterToEmptyIndexMap = new HashMap<>(); + + String localIndexName = randomAlphaOfLength(14).toLowerCase(Locale.ROOT) + "1"; + clusterToEmptyIndexMap.put(LOCAL_CLUSTER, localIndexName); + Client localClient = client(LOCAL_CLUSTER); + assertAcked( + localClient.admin().indices().prepareCreate(localIndexName).setSettings(Settings.builder().put("index.number_of_shards", 1)) + ); + + String remote1IndexName = randomAlphaOfLength(14).toLowerCase(Locale.ROOT) + "2"; + clusterToEmptyIndexMap.put(REMOTE_CLUSTER_1, remote1IndexName); + Client remote1Client = client(REMOTE_CLUSTER_1); + assertAcked( + remote1Client.admin().indices().prepareCreate(remote1IndexName).setSettings(Settings.builder().put("index.number_of_shards", 1)) + ); + + if (numClusters == 3) { + String remote2IndexName = randomAlphaOfLength(14).toLowerCase(Locale.ROOT) + "3"; + clusterToEmptyIndexMap.put(REMOTE_CLUSTER_2, remote2IndexName); + Client remote2Client = client(REMOTE_CLUSTER_2); + assertAcked( + remote2Client.admin() + .indices() + .prepareCreate(remote2IndexName) + .setSettings(Settings.builder().put("index.number_of_shards", 1)) + ); + } + + return clusterToEmptyIndexMap; + } + void populateLocalIndices(String indexName, int numShards) { Client localClient = client(LOCAL_CLUSTER); assertAcked( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java index 0faeb68d8dc71..60b632c443f8e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java @@ -408,8 +408,6 @@ public void testDetermineUnavailableRemoteClusters() { } } - // MP TODO: need to add a test for the "bypass IndexMode" thingy in mergedMappings that the rest test caught - public void testUpdateExecutionInfoAtEndOfPlanning() { String localClusterAlias = RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY; String remote1Alias = "remote1"; diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java index d5b3141b539eb..74ef6f0dafe63 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java @@ -495,7 +495,7 @@ public void testCrossClusterQueryWithRemoteDLSAndFLS() throws Exception { } /** - * Note: invalid_remote is "invalid" because it has a bogus API key and the cluster does not exist (cannot be connected to) + * Note: invalid_remote is "invalid" because it has a bogus API key */ @SuppressWarnings("unchecked") public void testCrossClusterQueryAgainstInvalidRemote() throws Exception { @@ -521,13 +521,19 @@ public void testCrossClusterQueryAgainstInvalidRemote() throws Exception { // invalid remote with local index should return local results { var q = "FROM invalid_remote:employees,employees | SORT emp_id DESC | LIMIT 10"; - Response response = performRequestWithRemoteSearchUser(esqlRequest(q)); - // TODO: when skip_unavailable=false for invalid_remote, a fatal exception should be thrown - // this does not yet happen because field-caps returns nothing for this cluster, rather - // than an error, so the current code cannot detect that error. Follow on PR will handle this. - assertLocalOnlyResults(response); + if (skipUnavailable) { + Response response = performRequestWithRemoteSearchUser(esqlRequest(q)); + // this does not yet happen because field-caps returns nothing for this cluster, rather + // than an error, so the current code cannot detect that error. Follow on PR will handle this. + assertLocalOnlyResultsAndSkippedRemote(response); + } else { + // errors from invalid remote should throw an exception if the cluster is marked with skip_unavailable=false + ResponseException error = expectThrows(ResponseException.class, () -> performRequestWithRemoteSearchUser(esqlRequest(q))); + assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(400)); + // TODO: in follow on PR, figure out why this is returning the wrong error - should be "cannot connect to invalid_remote" + assertThat(error.getMessage(), containsString("Unknown index [invalid_remote:employees]")); + } } - { var q = "FROM invalid_remote:employees | SORT emp_id DESC | LIMIT 10"; // errors from invalid remote should be ignored if the cluster is marked with skip_unavailable=true @@ -560,10 +566,9 @@ public void testCrossClusterQueryAgainstInvalidRemote() throws Exception { } else { // errors from invalid remote should throw an exception if the cluster is marked with skip_unavailable=false - ResponseException error = expectThrows(ResponseException.class, () -> { - final Response response1 = performRequestWithRemoteSearchUser(esqlRequest(q)); - }); + ResponseException error = expectThrows(ResponseException.class, () -> performRequestWithRemoteSearchUser(esqlRequest(q))); assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(401)); + // TODO: in follow on PR, figure out why this is returning the wrong error - should be "cannot connect to invalid_remote" assertThat(error.getMessage(), containsString("unable to find apikey")); } } @@ -1049,7 +1054,7 @@ private void assertRemoteOnlyAgainst2IndexResults(Response response) throws IOEx } @SuppressWarnings("unchecked") - private void assertLocalOnlyResults(Response response) throws IOException { + private void assertLocalOnlyResultsAndSkippedRemote(Response response) throws IOException { assertOK(response); Map responseAsMap = entityAsMap(response); List columns = (List) responseAsMap.get("columns"); @@ -1061,6 +1066,34 @@ private void assertLocalOnlyResults(Response response) throws IOException { .collect(Collectors.toList()); // local results assertThat(flatList, containsInAnyOrder("2", "4", "6", "8", "support", "management", "engineering", "marketing")); + Map clusters = (Map) responseAsMap.get("_clusters"); + + /* + clusters map: + {running=0, total=2, details={ + invalid_remote={_shards={total=0, failed=0, successful=0, skipped=0}, took=176, indices=employees, + failures=[{reason={reason=Unable to connect to [invalid_remote], type=connect_transport_exception}, + index=null, shard=-1}], status=skipped}, + (local)={_shards={total=1, failed=0, successful=1, skipped=0}, took=298, indices=employees, status=successful}}, + failed=0, partial=0, successful=1, skipped=1} + */ + + assertThat((int) clusters.get("total"), equalTo(2)); + assertThat((int) clusters.get("successful"), equalTo(1)); + assertThat((int) clusters.get("skipped"), equalTo(1)); + + Map details = (Map) clusters.get("details"); + Map invalidRemoteMap = (Map) details.get("invalid_remote"); + assertThat(invalidRemoteMap.get("status").toString(), equalTo("skipped")); + List failures = (List) invalidRemoteMap.get("failures"); + assertThat(failures.size(), equalTo(1)); + Map failureMap = (Map) failures.get(0); + Map reasonMap = (Map) failureMap.get("reason"); + assertThat(reasonMap.get("reason").toString(), containsString("Unable to connect to [invalid_remote]")); + assertThat(reasonMap.get("type").toString(), containsString("connect_transport_exception")); + + Map localCluster = (Map) details.get("(local)"); + assertThat(localCluster.get("status").toString(), equalTo("successful")); } @SuppressWarnings("unchecked") From 3016c28f9e1f27d5ba2f30ed455623037d0bd205 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Fri, 8 Nov 2024 17:06:45 -0500 Subject: [PATCH 10/17] I have decided that this ticket will NOT try to handle missing enrich policies for skip_un=true clusters; requires modifying the Analyzer --- .../elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java index c6f74cbc72d43..e8e9f45694e9c 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClustersEnrichIT.java @@ -61,8 +61,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -// MP TODO: what to do about missing enrich policies on skip_unavailable=true clusters? Should that be fatal or non-fatal? -// Write tests for those cases. public class CrossClustersEnrichIT extends AbstractMultiClustersTestCase { @Override From 906fb862cba578b630b281db5c14f992818aed90 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Tue, 12 Nov 2024 13:59:21 -0500 Subject: [PATCH 11/17] PR feedback - simplified EsqlSessionCCSUtils.missingIndicesIsFatal --- .../xpack/esql/session/EsqlSessionCCSUtils.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 8f099b2739838..715cce515cd82 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -247,17 +247,13 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionIn // visible for testing static boolean missingIndicesIsFatal(String clusterAlias, EsqlExecutionInfo executionInfo) { - if (executionInfo.getCluster(clusterAlias).isSkipUnavailable()) { - return false; - } // missing indices on local cluster is fatal only if a concrete index requested if (clusterAlias.equals(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY)) { return concreteIndexRequested(executionInfo.getCluster(clusterAlias).getIndexExpression()); } - return true; + return executionInfo.getCluster(clusterAlias).isSkipUnavailable() == false; } - // MP TODO: is there a better method for doing this, say in RemoteClusterService or RemoteClusterAware? private static boolean concreteIndexRequested(String indexExpression) { for (String expr : indexExpression.split(",")) { // TODO need to also detect date math before this check? From 211c7d0bfc6cfb1517f4e09868daeaa325528912 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Tue, 12 Nov 2024 14:07:49 -0500 Subject: [PATCH 12/17] Now handling date math in EsqlSessionCCSUtils.concreteIndexRequested --- .../xpack/esql/session/EsqlSessionCCSUtils.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 715cce515cd82..599692d40ad23 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -256,7 +256,10 @@ static boolean missingIndicesIsFatal(String clusterAlias, EsqlExecutionInfo exec private static boolean concreteIndexRequested(String indexExpression) { for (String expr : indexExpression.split(",")) { - // TODO need to also detect date math before this check? + if (expr.charAt(0) == '<' || expr.startsWith("-<")) { + // skip date math expressions + continue; + } if (expr.indexOf('*') < 0) { return true; } From 3fef0ba4db96fab680e3ab3674b96fcd224585ec Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Tue, 12 Nov 2024 14:15:14 -0500 Subject: [PATCH 13/17] Updatec yaml changelog with proper title and issue ref --- docs/changelog/116348.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml index 7fa3083b0a6e4..5a46265ab8ae7 100644 --- a/docs/changelog/116348.yaml +++ b/docs/changelog/116348.yaml @@ -1,6 +1,6 @@ pr: 116348 -summary: "DRAFT: ESQL honors `skip_unavailable` setting for nonmatching indices errors\ +summary: "ESQL honors `skip_unavailable` setting for nonmatching indices errors\ \ at planning time" area: ES|QL type: enhancement -issues: [] +issues: [ 114531 ] From 690670aa45ffca462d9dc4290650499890a11860 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 13 Nov 2024 10:11:55 -0500 Subject: [PATCH 14/17] PR feedback: Removed IndexResolution.valid(EsIndex index,Set resolvedIndices) to avoid PR noise, since it can be inferred from EsIndex contents --- .../xpack/esql/index/IndexResolution.java | 29 +++++++++++++++---- .../xpack/esql/session/EsqlSession.java | 5 ++-- .../esql/session/EsqlSessionCCSUtils.java | 4 +-- .../elasticsearch/xpack/esql/CsvTests.java | 3 +- .../esql/analysis/AnalyzerTestUtils.java | 3 +- .../xpack/esql/analysis/AnalyzerTests.java | 9 +++--- .../xpack/esql/analysis/ParsingTests.java | 3 +- .../xpack/esql/analysis/VerifierTests.java | 3 +- .../LocalLogicalPlanOptimizerTests.java | 5 ++-- .../LocalPhysicalPlanOptimizerTests.java | 2 +- .../optimizer/LogicalPlanOptimizerTests.java | 13 ++++----- .../optimizer/PhysicalPlanOptimizerTests.java | 2 +- .../xpack/esql/planner/FilterTests.java | 3 +- .../esql/planner/QueryTranslatorTests.java | 3 +- .../esql/plugin/DataNodeRequestTests.java | 3 +- 15 files changed, 49 insertions(+), 41 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java index ca64e452bc829..63abbd5e480d7 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java @@ -16,6 +16,12 @@ public final class IndexResolution { + /** + * @param index EsIndex encapsulating requested index expression, resolved mappings and index modes from field-caps. + * @param resolvedIndices Set of concrete indices resolved by field-caps. (This information is not always present in the EsIndex). + * @param unavailableClusters Remote clusters that could not be contacted during planning + * @return valid IndexResolution + */ public static IndexResolution valid( EsIndex index, Set resolvedIndices, @@ -27,8 +33,11 @@ public static IndexResolution valid( return new IndexResolution(index, null, resolvedIndices, unavailableClusters); } - public static IndexResolution valid(EsIndex index, Set resolvedIndices) { - return valid(index, resolvedIndices, Collections.emptyMap()); + /** + * Use this method only if the set of concrete resolved indices is the same as EsIndex#concreteIndices(). + */ + public static IndexResolution valid(EsIndex index) { + return valid(index, index.concreteIndices(), Collections.emptyMap()); } public static IndexResolution invalid(String invalid) { @@ -89,14 +98,14 @@ public boolean isValid() { * @return Map of unavailable clusters (could not be connected to during field-caps query). Key of map is cluster alias, * value is the {@link FieldCapabilitiesFailure} describing the issue. */ - public Map getUnavailableClusters() { + public Map unavailableClusters() { return unavailableClusters; } /** * @return all indices found by field-caps (regardless of whether they had any mappings) */ - public Set getResolvedIndices() { + public Set resolvedIndices() { return resolvedIndices; } @@ -119,6 +128,16 @@ public int hashCode() { @Override public String toString() { - return invalid != null ? invalid : index.name(); + return "IndexResolution{" + + "index=" + + index + + ", invalid='" + + invalid + + '\'' + + ", resolvedIndices=" + + resolvedIndices + + ", unavailableClusters=" + + unavailableClusters + + '}'; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 244fb10127208..c576d15f92608 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -72,7 +72,6 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -310,7 +309,7 @@ private void preAnalyze( // resolution to updateExecutionInfo if (indexResolution.isValid()) { EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution); - EsqlSessionCCSUtils.updateExecutionInfoWithUnavailableClusters(executionInfo, indexResolution.getUnavailableClusters()); + EsqlSessionCCSUtils.updateExecutionInfoWithUnavailableClusters(executionInfo, indexResolution.unavailableClusters()); if (executionInfo.isCrossClusterSearch() && executionInfo.getClusterStateCount(EsqlExecutionInfo.Cluster.Status.RUNNING) == 0) { // for a CCS, if all clusters have been marked as SKIPPED, nothing to search so send a sentinel @@ -387,7 +386,7 @@ private void preAnalyzeIndices( String indexExpressionToResolve = EsqlSessionCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo); if (indexExpressionToResolve.isEmpty()) { // if this was a pure remote CCS request (no local indices) and all remotes are offline, return an empty IndexResolution - listener.onResponse(IndexResolution.valid(new EsIndex(table.index(), Map.of(), Map.of()), Collections.emptySet())); + listener.onResponse(IndexResolution.valid(new EsIndex(table.index(), Map.of(), Map.of()))); } else { // call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types indexResolver.resolveAsMergedMapping(indexExpressionToResolve, fieldNames, listener); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 599692d40ad23..4fe2fef7e3f45 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -183,12 +183,12 @@ static void updateExecutionInfoWithUnavailableClusters(EsqlExecutionInfo execInf static void updateExecutionInfoWithClustersWithNoMatchingIndices(EsqlExecutionInfo executionInfo, IndexResolution indexResolution) { Set clustersWithResolvedIndices = new HashSet<>(); // determine missing clusters - for (String indexName : indexResolution.getResolvedIndices()) { + for (String indexName : indexResolution.resolvedIndices()) { clustersWithResolvedIndices.add(RemoteClusterAware.parseClusterAlias(indexName)); } Set clustersRequested = executionInfo.clusterAliases(); Set clustersWithNoMatchingIndices = Sets.difference(clustersRequested, clustersWithResolvedIndices); - clustersWithNoMatchingIndices.removeAll(indexResolution.getUnavailableClusters().keySet()); + clustersWithNoMatchingIndices.removeAll(indexResolution.unavailableClusters().keySet()); /** * Rules enforced at planning time around non-matching indices diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 32baabc687041..348ca4acd100e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -98,7 +98,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -329,7 +328,7 @@ private static IndexResolution loadIndexResolution(String mappingName, String in } } } - return IndexResolution.valid(new EsIndex(indexName, mapping, Map.of(indexName, IndexMode.STANDARD)), Set.of(indexName)); + return IndexResolution.valid(new EsIndex(indexName, mapping, Map.of(indexName, IndexMode.STANDARD))); } private static EnrichResolution loadEnrichPolicies() { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java index 65ee3d205f830..a63ee53cdd498 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTestUtils.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Set; import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.GEO_MATCH_TYPE; import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.MATCH_TYPE; @@ -92,7 +91,7 @@ public static LogicalPlan analyze(String query, String mapping, QueryParams para public static IndexResolution loadMapping(String resource, String indexName) { EsIndex test = new EsIndex(indexName, EsqlTestUtils.loadMapping(resource)); - return IndexResolution.valid(test, Set.of(indexName)); + return IndexResolution.valid(test); } public static IndexResolution analyzerDefaultMapping() { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index c2e09d3cd405e..c1b2adddfc838 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -103,7 +103,7 @@ public class AnalyzerTests extends ESTestCase { public void testIndexResolution() { EsIndex idx = new EsIndex("idx", Map.of()); - Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); + Analyzer analyzer = analyzer(IndexResolution.valid(idx)); var plan = analyzer.analyze(UNRESOLVED_RELATION); var limit = as(plan, Limit.class); @@ -120,8 +120,7 @@ public void testFailOnUnresolvedIndex() { public void testIndexWithClusterResolution() { EsIndex idx = new EsIndex("cluster:idx", Map.of()); - // MP TODO: is this right? should it be "idx" or "cluster:idx"? - Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of(idx.name()))); + Analyzer analyzer = analyzer(IndexResolution.valid(idx)); var plan = analyzer.analyze(UNRESOLVED_RELATION); var limit = as(plan, Limit.class); @@ -131,7 +130,7 @@ public void testIndexWithClusterResolution() { public void testAttributeResolution() { EsIndex idx = new EsIndex("idx", LoadMapping.loadMapping("mapping-one-field.json")); - Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); + Analyzer analyzer = analyzer(IndexResolution.valid(idx)); var plan = analyzer.analyze( new Eval(EMPTY, UNRESOLVED_RELATION, List.of(new Alias(EMPTY, "e", new UnresolvedAttribute(EMPTY, "emp_no")))) @@ -184,7 +183,7 @@ public void testAttributeResolutionOfChainedReferences() { public void testRowAttributeResolution() { EsIndex idx = new EsIndex("idx", Map.of()); - Analyzer analyzer = analyzer(IndexResolution.valid(idx, Set.of("idx"))); + Analyzer analyzer = analyzer(IndexResolution.valid(idx)); var plan = analyzer.analyze( new Eval( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java index 7fa36ebbcfd94..3cafd42b731f6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java @@ -30,7 +30,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Set; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_CFG; import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_VERIFIER; @@ -121,6 +120,6 @@ private String error(String query) { } private static IndexResolution loadIndexResolution(String name) { - return IndexResolution.valid(new EsIndex(INDEX_NAME, LoadMapping.loadMapping(name)), Set.of(INDEX_NAME)); + return IndexResolution.valid(new EsIndex(INDEX_NAME, LoadMapping.loadMapping(name))); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index 59930fce39ff6..0a34d6cd848bb 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -86,8 +86,7 @@ public void testUnsupportedAndMultiTypedFields() { new EsIndex( "test*", Map.of(unsupported, unsupportedField, multiTyped, multiTypedField, "int", unsupportedField, "double", multiTypedField) - ), - ipIndices + ) ); Analyzer analyzer = AnalyzerTestUtils.analyzer(indexWithUnsupportedAndMultiTypedField); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java index 831a7f4c780a3..baef20081a4f2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java @@ -56,7 +56,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; import static java.util.Collections.emptyMap; import static org.elasticsearch.xpack.esql.EsqlTestUtils.L; @@ -93,7 +92,7 @@ public static void init() { mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(test); logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); analyzer = new Analyzer( @@ -401,7 +400,7 @@ public void testSparseDocument() throws Exception { SearchStats searchStats = statsForExistingField("field000", "field001", "field002", "field003", "field004"); EsIndex index = new EsIndex("large", large, Map.of("large", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(index, Set.of("large")); + IndexResolution getIndexResult = IndexResolution.valid(index); var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); var analyzer = new Analyzer( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index 07839e898ecbd..073a51ee69114 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -141,7 +141,7 @@ public void init() { private Analyzer makeAnalyzer(String mappingFileName, EnrichResolution enrichResolution) { var mapping = loadMapping(mappingFileName); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, test.concreteIndices()); + IndexResolution getIndexResult = IndexResolution.valid(test); return new Analyzer( new AnalyzerContext(config, new EsqlFunctionRegistry(), getIndexResult, enrichResolution), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 2da2d507397d5..d9a0f9ad57fb1 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -121,7 +121,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; @@ -215,7 +214,7 @@ public static void init() { // Most tests used data from the test index, so we load it here, and use it in the plan() function. mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(test); analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, enrichResolution), TEST_VERIFIER @@ -224,7 +223,7 @@ public static void init() { // Some tests use data from the airports index, so we load it here, and use it in the plan_airports() function. mappingAirports = loadMapping("mapping-airports.json"); EsIndex airports = new EsIndex("airports", mappingAirports, Map.of("airports", IndexMode.STANDARD)); - IndexResolution getIndexResultAirports = IndexResolution.valid(airports, Set.of("airports")); + IndexResolution getIndexResultAirports = IndexResolution.valid(airports); analyzerAirports = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution), TEST_VERIFIER @@ -233,7 +232,7 @@ public static void init() { // Some tests need additional types, so we load that index here and use it in the plan_types() function. mappingTypes = loadMapping("mapping-all-types.json"); EsIndex types = new EsIndex("types", mappingTypes, Map.of("types", IndexMode.STANDARD)); - IndexResolution getIndexResultTypes = IndexResolution.valid(types, Set.of("types")); + IndexResolution getIndexResultTypes = IndexResolution.valid(types); analyzerTypes = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultTypes, enrichResolution), TEST_VERIFIER @@ -242,14 +241,14 @@ public static void init() { // Some tests use mappings from mapping-extra.json to be able to test more types so we load it here mappingExtra = loadMapping("mapping-extra.json"); EsIndex extra = new EsIndex("extra", mappingExtra, Map.of("extra", IndexMode.STANDARD)); - IndexResolution getIndexResultExtra = IndexResolution.valid(extra, Set.of("extra")); + IndexResolution getIndexResultExtra = IndexResolution.valid(extra); analyzerExtra = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultExtra, enrichResolution), TEST_VERIFIER ); metricMapping = loadMapping("k8s-mappings.json"); - var metricsIndex = IndexResolution.valid(new EsIndex("k8s", metricMapping, Map.of("k8s", IndexMode.TIME_SERIES)), Set.of("k8s")); + var metricsIndex = IndexResolution.valid(new EsIndex("k8s", metricMapping, Map.of("k8s", IndexMode.TIME_SERIES))); metricsAnalyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), metricsIndex, enrichResolution), TEST_VERIFIER @@ -4505,7 +4504,7 @@ private static boolean oneLeaveIsNull(Expression e) { public void testEmptyMappingIndex() { EsIndex empty = new EsIndex("empty_test", emptyMap(), Map.of()); - IndexResolution getIndexResultAirports = IndexResolution.valid(empty, Set.of("empty_test")); + IndexResolution getIndexResultAirports = IndexResolution.valid(empty); var analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution), TEST_VERIFIER diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java index e7374b8f65027..eb115ed7b2948 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java @@ -283,7 +283,7 @@ TestDataSource makeTestDataSource( ) { Map mapping = loadMapping(mappingFileName); EsIndex index = new EsIndex(indexName, mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(index, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(index); Analyzer analyzer = new Analyzer(new AnalyzerContext(config, functionRegistry, getIndexResult, enrichResolution), TEST_VERIFIER); return new TestDataSource(mapping, index, analyzer, stats); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java index 322580872a136..8d819f9dbcd6c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/FilterTests.java @@ -46,7 +46,6 @@ import java.io.UncheckedIOException; import java.util.List; import java.util.Map; -import java.util.Set; import static java.util.Arrays.asList; import static org.elasticsearch.index.query.QueryBuilders.rangeQuery; @@ -78,7 +77,7 @@ public static void init() { Map mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(test); logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG)); physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(EsqlTestUtils.TEST_CFG)); mapper = new Mapper(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java index 2d5fd7bff5438..cf90cf96fe683 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; -import java.util.Set; import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; @@ -43,7 +42,7 @@ public class QueryTranslatorTests extends ESTestCase { private static Analyzer makeAnalyzer(String mappingFileName) { var mapping = loadMapping(mappingFileName); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(test); return new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, new EnrichResolution()), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java index 188f6273b53e8..4553551c40cd3 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java @@ -38,7 +38,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Set; import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomConfiguration; import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomTables; @@ -264,7 +263,7 @@ protected DataNodeRequest mutateInstance(DataNodeRequest in) throws IOException static LogicalPlan parse(String query) { Map mapping = loadMapping("mapping-basic.json"); EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD)); - IndexResolution getIndexResult = IndexResolution.valid(test, Set.of("test")); + IndexResolution getIndexResult = IndexResolution.valid(test); var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(TEST_CFG)); var analyzer = new Analyzer( new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, emptyPolicyResolution()), From 8d4d89b81264d43e974ffbf93c1c702cb00227b6 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 13 Nov 2024 12:59:25 -0500 Subject: [PATCH 15/17] Update docs/changelog/116348.yaml --- docs/changelog/116348.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml index 5a46265ab8ae7..fa4210a5b8ce2 100644 --- a/docs/changelog/116348.yaml +++ b/docs/changelog/116348.yaml @@ -1,6 +1,6 @@ pr: 116348 -summary: "ESQL honors `skip_unavailable` setting for nonmatching indices errors\ - \ at planning time" +summary: ESQL honors `skip_unavailable` setting for nonmatching indices errors at + planning time area: ES|QL type: enhancement -issues: [ 114531 ] +issues: [] From 13c5c964adc438b533c36a221e8ca1fc1a067a96 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 13 Nov 2024 13:53:41 -0500 Subject: [PATCH 16/17] Changing IndexResolution toString for invalid cases causes tests to break, so reverting original model for invalid case --- docs/changelog/116348.yaml | 5 ++-- .../xpack/esql/index/IndexResolution.java | 24 ++++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml index fa4210a5b8ce2..fb04afeaf2cfc 100644 --- a/docs/changelog/116348.yaml +++ b/docs/changelog/116348.yaml @@ -1,6 +1,5 @@ pr: 116348 -summary: ESQL honors `skip_unavailable` setting for nonmatching indices errors at - planning time +summary: ESQL: Honor skip_unavailable setting for nonmatching indices errors at planning time area: ES|QL type: enhancement -issues: [] +issues: [ 114531 ] diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java index 63abbd5e480d7..88366bbf9a7c3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java @@ -128,16 +128,18 @@ public int hashCode() { @Override public String toString() { - return "IndexResolution{" - + "index=" - + index - + ", invalid='" - + invalid - + '\'' - + ", resolvedIndices=" - + resolvedIndices - + ", unavailableClusters=" - + unavailableClusters - + '}'; + return invalid != null + ? invalid + : "IndexResolution{" + + "index=" + + index + + ", invalid='" + + invalid + + '\'' + + ", resolvedIndices=" + + resolvedIndices + + ", unavailableClusters=" + + unavailableClusters + + '}'; } } From 6e9965658b38168d4ff5824a29045b7a1ff80338 Mon Sep 17 00:00:00 2001 From: Michael Peterson Date: Wed, 13 Nov 2024 14:06:29 -0500 Subject: [PATCH 17/17] Fix changelong error --- docs/changelog/116348.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml index fb04afeaf2cfc..927ffc5a6121d 100644 --- a/docs/changelog/116348.yaml +++ b/docs/changelog/116348.yaml @@ -1,5 +1,5 @@ pr: 116348 -summary: ESQL: Honor skip_unavailable setting for nonmatching indices errors at planning time +summary: "ESQL: Honor skip_unavailable setting for nonmatching indices errors at planning time" area: ES|QL type: enhancement issues: [ 114531 ]