diff --git a/docs/changelog/116348.yaml b/docs/changelog/116348.yaml new file mode 100644 index 0000000000000..927ffc5a6121d --- /dev/null +++ b/docs/changelog/116348.yaml @@ -0,0 +1,5 @@ +pr: 116348 +summary: "ESQL: Honor skip_unavailable setting for nonmatching indices errors at planning time" +area: ES|QL +type: enhancement +issues: [ 114531 ] diff --git a/docs/reference/esql/functions/kibana/definition/bit_length.json b/docs/reference/esql/functions/kibana/definition/bit_length.json index 156a063984e4d..0c75b76cdbbfb 100644 --- a/docs/reference/esql/functions/kibana/definition/bit_length.json +++ b/docs/reference/esql/functions/kibana/definition/bit_length.json @@ -31,7 +31,7 @@ } ], "examples" : [ - "FROM airports\n| WHERE country == \"India\"\n| KEEP city\n| EVAL fn_length=LENGTH(city), fn_bit_length = BIT_LENGTH(city)" + "FROM airports\n| WHERE country == \"India\"\n| KEEP city\n| EVAL fn_length = LENGTH(city), fn_bit_length = BIT_LENGTH(city)" ], "preview" : false, "snapshot_only" : false diff --git a/docs/reference/esql/functions/kibana/definition/byte_length.json b/docs/reference/esql/functions/kibana/definition/byte_length.json index c8280a572fc62..60f439b9d8133 100644 --- a/docs/reference/esql/functions/kibana/definition/byte_length.json +++ b/docs/reference/esql/functions/kibana/definition/byte_length.json @@ -31,7 +31,7 @@ } ], "examples" : [ - "FROM airports\n| WHERE country == \"India\"\n| KEEP city\n| EVAL fn_length=LENGTH(city), fn_byte_length = BYTE_LENGTH(city)" + "FROM airports\n| WHERE country == \"India\"\n| KEEP city\n| EVAL fn_length = LENGTH(city), fn_byte_length = BYTE_LENGTH(city)" ], "preview" : false, "snapshot_only" : false diff --git a/docs/reference/esql/functions/kibana/definition/length.json b/docs/reference/esql/functions/kibana/definition/length.json index 9ea340ebf7420..bc26acde744f5 100644 --- a/docs/reference/esql/functions/kibana/definition/length.json +++ b/docs/reference/esql/functions/kibana/definition/length.json @@ -31,7 +31,7 @@ } ], "examples" : [ - "FROM airports\n| KEEP city\n| EVAL fn_length = LENGTH(first_name)" + "FROM airports\n| WHERE country == \"India\"\n| KEEP city\n| EVAL fn_length = LENGTH(city)" ], "preview" : false, "snapshot_only" : false diff --git a/docs/reference/esql/functions/kibana/docs/bit_length.md b/docs/reference/esql/functions/kibana/docs/bit_length.md index 253b2cdb6a7c6..b1d8e24c4de76 100644 --- a/docs/reference/esql/functions/kibana/docs/bit_length.md +++ b/docs/reference/esql/functions/kibana/docs/bit_length.md @@ -9,6 +9,6 @@ Returns the bit length of a string. FROM airports | WHERE country == "India" | KEEP city -| EVAL fn_length=LENGTH(city), fn_bit_length = BIT_LENGTH(city) +| EVAL fn_length = LENGTH(city), fn_bit_length = BIT_LENGTH(city) ``` Note: All strings are in UTF-8, so a single character can use multiple bytes. diff --git a/docs/reference/esql/functions/kibana/docs/byte_length.md b/docs/reference/esql/functions/kibana/docs/byte_length.md index 20d96ce38400d..9cd4f87c9883b 100644 --- a/docs/reference/esql/functions/kibana/docs/byte_length.md +++ b/docs/reference/esql/functions/kibana/docs/byte_length.md @@ -9,6 +9,6 @@ Returns the byte length of a string. FROM airports | WHERE country == "India" | KEEP city -| EVAL fn_length=LENGTH(city), fn_byte_length = BYTE_LENGTH(city) +| EVAL fn_length = LENGTH(city), fn_byte_length = BYTE_LENGTH(city) ``` Note: All strings are in UTF-8, so a single character can use multiple bytes. diff --git a/docs/reference/esql/functions/kibana/docs/length.md b/docs/reference/esql/functions/kibana/docs/length.md index ce7726d092bae..aed76ee14cedb 100644 --- a/docs/reference/esql/functions/kibana/docs/length.md +++ b/docs/reference/esql/functions/kibana/docs/length.md @@ -7,7 +7,8 @@ Returns the character length of a string. ``` FROM airports +| WHERE country == "India" | KEEP city -| EVAL fn_length = LENGTH(first_name) +| EVAL fn_length = LENGTH(city) ``` Note: All strings are in UTF-8, so a single character can use multiple bytes. diff --git a/docs/reference/settings/notification-settings.asciidoc b/docs/reference/settings/notification-settings.asciidoc index 145112ef4d27c..c375ddf076a66 100644 --- a/docs/reference/settings/notification-settings.asciidoc +++ b/docs/reference/settings/notification-settings.asciidoc @@ -118,6 +118,17 @@ If you configure multiple email accounts, you must either configure this setting or specify the email account to use in the <> action. See <>. +`xpack.notification.email.recipient_allowlist`:: +(<>) +Specifies addresses to which emails are allowed to be sent. +Emails with recipients (`To:`, `Cc:`, or `Bcc:`) outside of these patterns will be rejected and an +error thrown. This setting defaults to `["*"]` which means all recipients are allowed. +Simple globbing is supported, such as `list-*@company.com` in the list of allowed recipients. + +NOTE: This setting can't be used at the same time as `xpack.notification.email.account.domain_allowlist` +and an error will be thrown if both are set at the same time. This setting can be used to specify domains +to allow by using a wildcard pattern such as `*@company.com`. + `xpack.notification.email.account`:: Specifies account information for sending notifications via email. You can specify the following email account attributes: @@ -129,6 +140,10 @@ Specifies domains to which emails are allowed to be sent. Emails with recipients `Bcc:`) outside of these domains will be rejected and an error thrown. This setting defaults to `["*"]` which means all domains are allowed. Simple globbing is supported, such as `*.company.com` in the list of allowed domains. + +NOTE: This setting can't be used at the same time as `xpack.notification.email.recipient_allowlist` +and an error will be thrown if both are set at the same time. + -- [[email-account-attributes]] diff --git a/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryMetricsTests.java b/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryMetricsTests.java index 61940be247861..e049d4cd372e6 100644 --- a/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryMetricsTests.java +++ b/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryMetricsTests.java @@ -112,7 +112,7 @@ public void testThrottleResponsesAreCountedInMetrics() throws IOException { blobContainer.blobExists(purpose, blobName); // Correct metrics are recorded - metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).expectMetrics() + metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES).expectMetrics() .withRequests(numThrottles + 1) .withThrottles(numThrottles) .withExceptions(numThrottles) @@ -137,7 +137,7 @@ public void testRangeNotSatisfiedAreCountedInMetrics() throws IOException { assertThrows(RequestedRangeNotSatisfiedException.class, () -> blobContainer.readBlob(purpose, blobName)); // Correct metrics are recorded - metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB, repository).expectMetrics() + metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB).expectMetrics() .withRequests(1) .withThrottles(0) .withExceptions(1) @@ -170,7 +170,7 @@ public void testErrorResponsesAreCountedInMetrics() throws IOException { blobContainer.blobExists(purpose, blobName); // Correct metrics are recorded - metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).expectMetrics() + metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES).expectMetrics() .withRequests(numErrors + 1) .withThrottles(throttles.get()) .withExceptions(numErrors) @@ -191,7 +191,7 @@ public void testRequestFailuresAreCountedInMetrics() { assertThrows(IOException.class, () -> blobContainer.listBlobs(purpose)); // Correct metrics are recorded - metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS, repository).expectMetrics() + metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS).expectMetrics() .withRequests(4) .withThrottles(0) .withExceptions(4) @@ -322,20 +322,14 @@ private void clearMetrics(String discoveryNode) { .forEach(TestTelemetryPlugin::resetMeter); } - private MetricsAsserter metricsAsserter( - String dataNodeName, - OperationPurpose operationPurpose, - AzureBlobStore.Operation operation, - String repository - ) { - return new MetricsAsserter(dataNodeName, operationPurpose, operation, repository); + private MetricsAsserter metricsAsserter(String dataNodeName, OperationPurpose operationPurpose, AzureBlobStore.Operation operation) { + return new MetricsAsserter(dataNodeName, operationPurpose, operation); } private class MetricsAsserter { private final String dataNodeName; private final OperationPurpose purpose; private final AzureBlobStore.Operation operation; - private final String repository; enum Result { Success, @@ -361,11 +355,10 @@ List getMeasurements(TestTelemetryPlugin testTelemetryPlugin, Strin abstract List getMeasurements(TestTelemetryPlugin testTelemetryPlugin, String name); } - private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation, String repository) { + private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation) { this.dataNodeName = dataNodeName; this.purpose = purpose; this.operation = operation; - this.repository = repository; } private class Expectations { @@ -458,7 +451,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa .filter( m -> m.attributes().get("operation").equals(operation.getKey()) && m.attributes().get("purpose").equals(purpose.getKey()) - && m.attributes().get("repo_name").equals(repository) && m.attributes().get("repo_type").equals("azure") ) .findFirst() @@ -470,8 +462,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa + operation.getKey() + " and purpose=" + purpose.getKey() - + " and repo_name=" - + repository + " in " + measurements ) diff --git a/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java b/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java index bd21f208faac4..ab3f3ee4f3728 100644 --- a/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java +++ b/modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java @@ -402,10 +402,7 @@ public void testMetrics() throws Exception { ) ); metrics.forEach(metric -> { - assertThat( - metric.attributes(), - allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose")) - ); + assertThat(metric.attributes(), allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("operation"), hasKey("purpose"))); final AzureBlobStore.Operation operation = AzureBlobStore.Operation.fromKey((String) metric.attributes().get("operation")); final AzureBlobStore.StatsKey statsKey = new AzureBlobStore.StatsKey( operation, diff --git a/modules/repository-s3/build.gradle b/modules/repository-s3/build.gradle index 346a458a65f85..59dfa6b9aace2 100644 --- a/modules/repository-s3/build.gradle +++ b/modules/repository-s3/build.gradle @@ -12,6 +12,7 @@ import org.elasticsearch.gradle.internal.test.InternalClusterTestPlugin */ apply plugin: 'elasticsearch.internal-yaml-rest-test' apply plugin: 'elasticsearch.internal-cluster-test' +apply plugin: 'elasticsearch.internal-java-rest-test' esplugin { description 'The S3 repository plugin adds S3 repositories' @@ -48,6 +49,10 @@ dependencies { yamlRestTestImplementation project(':test:fixtures:minio-fixture') internalClusterTestImplementation project(':test:fixtures:minio-fixture') + javaRestTestImplementation project(":test:framework") + javaRestTestImplementation project(':test:fixtures:s3-fixture') + javaRestTestImplementation project(':modules:repository-s3') + yamlRestTestRuntimeOnly "org.slf4j:slf4j-simple:${versions.slf4j}" internalClusterTestRuntimeOnly "org.slf4j:slf4j-simple:${versions.slf4j}" } diff --git a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index bb8a452e21771..d9480abf21687 100644 --- a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -300,10 +300,7 @@ public void testMetrics() throws Exception { ) ); metrics.forEach(metric -> { - assertThat( - metric.attributes(), - allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose")) - ); + assertThat(metric.attributes(), allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("operation"), hasKey("purpose"))); final S3BlobStore.Operation operation = S3BlobStore.Operation.parse((String) metric.attributes().get("operation")); final S3BlobStore.StatsKey statsKey = new S3BlobStore.StatsKey( operation, diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestIT.java new file mode 100644 index 0000000000000..ead2cb36ad150 --- /dev/null +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestIT.java @@ -0,0 +1,98 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.repositories.s3; + +import fixture.s3.S3HttpFixture; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.MutableSettingsProvider; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.equalTo; + +public class RepositoryS3RestIT extends ESRestTestCase { + + private static final String BUCKET = "RepositoryS3JavaRestTest-bucket"; + private static final String BASE_PATH = "RepositoryS3JavaRestTest-base-path"; + + public static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, "ignored"); + + private static final MutableSettingsProvider keystoreSettings = new MutableSettingsProvider(); + + public static ElasticsearchCluster cluster = ElasticsearchCluster.local() + .module("repository-s3") + .keystore(keystoreSettings) + .setting("s3.client.default.endpoint", s3Fixture::getAddress) + .build(); + + @ClassRule + public static TestRule ruleChain = RuleChain.outerRule(s3Fixture).around(cluster); + + @Override + protected String getTestRestCluster() { + return cluster.getHttpAddresses(); + } + + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/116811") + public void testReloadCredentialsFromKeystore() throws IOException { + // Register repository (?verify=false because we don't have access to the blob store yet) + final var repositoryName = randomIdentifier(); + registerRepository( + repositoryName, + S3Repository.TYPE, + false, + Settings.builder().put("bucket", BUCKET).put("base_path", BASE_PATH).build() + ); + final var verifyRequest = new Request("POST", "/_snapshot/" + repositoryName + "/_verify"); + + // Set up initial credentials + final var accessKey1 = randomIdentifier(); + s3Fixture.setAccessKey(accessKey1); + keystoreSettings.put("s3.client.default.access_key", accessKey1); + keystoreSettings.put("s3.client.default.secret_key", randomIdentifier()); + cluster.updateStoredSecureSettings(); + assertOK(client().performRequest(new Request("POST", "/_nodes/reload_secure_settings"))); + + // Check access using initial credentials + assertOK(client().performRequest(verifyRequest)); + + // Rotate credentials in blob store + final var accessKey2 = randomValueOtherThan(accessKey1, ESTestCase::randomIdentifier); + s3Fixture.setAccessKey(accessKey2); + + // Ensure that initial credentials now invalid + final var accessDeniedException2 = expectThrows(ResponseException.class, () -> client().performRequest(verifyRequest)); + assertThat(accessDeniedException2.getResponse().getStatusLine().getStatusCode(), equalTo(500)); + assertThat( + accessDeniedException2.getMessage(), + allOf(containsString("Bad access key"), containsString("Status Code: 403"), containsString("Error Code: AccessDenied")) + ); + + // Set up refreshed credentials + keystoreSettings.put("s3.client.default.access_key", accessKey2); + cluster.updateStoredSecureSettings(); + assertOK(client().performRequest(new Request("POST", "/_nodes/reload_secure_settings"))); + + // Check access using refreshed credentials + assertOK(client().performRequest(verifyRequest)); + } + +} diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java index da357dc09ab95..7407522651e55 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java @@ -327,8 +327,6 @@ private Map metricAttributes(String action) { return Map.of( "repo_type", S3Repository.TYPE, - "repo_name", - blobStore.getRepositoryMetadata().name(), "operation", Operation.GET_OBJECT.getKey(), "purpose", diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java index b292dc5872994..ac49cffc1e0da 100644 --- a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java @@ -1106,7 +1106,7 @@ private List getRetryHistogramMeasurements() { } private Map metricAttributes(String action) { - return Map.of("repo_type", "s3", "repo_name", "repository", "operation", "GetObject", "purpose", "Indices", "action", action); + return Map.of("repo_type", "s3", "operation", "GetObject", "purpose", "Indices", "action", action); } /** diff --git a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/http/netty4/Netty4IncrementalRequestHandlingIT.java b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/http/netty4/Netty4IncrementalRequestHandlingIT.java index 647d38c626c74..1e84f65cdd842 100644 --- a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/http/netty4/Netty4IncrementalRequestHandlingIT.java +++ b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/http/netty4/Netty4IncrementalRequestHandlingIT.java @@ -210,7 +210,9 @@ public void testServerCloseConnectionMidStream() throws Exception { // terminate connection on server and wait resources are released handler.channel.request().getHttpChannel().close(); assertBusy(() -> { - assertNull(handler.stream.buf()); + // Cannot be simplified to assertNull. + // assertNull requires object to not fail on toString() method, but closing buffer can + assertTrue(handler.stream.buf() == null); assertTrue(handler.streamClosed); }); } diff --git a/muted-tests.yml b/muted-tests.yml index d73c143bd1cd6..a9dda9ac80782 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -150,12 +150,6 @@ tests: - class: org.elasticsearch.xpack.restart.CoreFullClusterRestartIT method: testSnapshotRestore {cluster=UPGRADED} issue: https://github.com/elastic/elasticsearch/issues/111799 -- class: org.elasticsearch.xpack.restart.CoreFullClusterRestartIT - method: testSnapshotRestore {cluster=OLD} - issue: https://github.com/elastic/elasticsearch/issues/111774 -- class: org.elasticsearch.upgrades.FullClusterRestartIT - method: testSnapshotRestore {cluster=OLD} - issue: https://github.com/elastic/elasticsearch/issues/111777 - class: org.elasticsearch.xpack.ml.integration.DatafeedJobsRestIT method: testLookbackWithIndicesOptions issue: https://github.com/elastic/elasticsearch/issues/116127 @@ -177,9 +171,6 @@ tests: - class: org.elasticsearch.search.basic.SearchWithRandomDisconnectsIT method: testSearchWithRandomDisconnects issue: https://github.com/elastic/elasticsearch/issues/116175 -- class: org.elasticsearch.search.basic.SearchWhileRelocatingIT - method: testSearchAndRelocateConcurrentlyRandomReplicas - issue: https://github.com/elastic/elasticsearch/issues/116145 - class: org.elasticsearch.xpack.deprecation.DeprecationHttpIT method: testDeprecatedSettingsReturnWarnings issue: https://github.com/elastic/elasticsearch/issues/108628 @@ -221,35 +212,26 @@ tests: - class: org.elasticsearch.upgrades.SearchStatesIT method: testCanMatch issue: https://github.com/elastic/elasticsearch/issues/116618 -- class: org.elasticsearch.packaging.test.ArchiveGenerateInitialCredentialsTests - method: test20NoAutoGenerationWhenAutoConfigurationDisabled - issue: https://github.com/elastic/elasticsearch/issues/116619 -- class: org.elasticsearch.packaging.test.BootstrapCheckTests - method: test20RunWithBootstrapChecks - issue: https://github.com/elastic/elasticsearch/issues/116620 -- class: org.elasticsearch.packaging.test.DockerTests - method: test011SecurityEnabledStatus - issue: https://github.com/elastic/elasticsearch/issues/116628 - class: org.elasticsearch.reservedstate.service.RepositoriesFileSettingsIT method: testSettingsApplied issue: https://github.com/elastic/elasticsearch/issues/116694 - class: org.elasticsearch.snapshots.SnapshotShutdownIT method: testRestartNodeDuringSnapshot issue: https://github.com/elastic/elasticsearch/issues/116730 -- class: org.elasticsearch.action.search.SearchRequestTests - method: testSerializationConstants - issue: https://github.com/elastic/elasticsearch/issues/116752 - class: org.elasticsearch.xpack.security.authc.ldap.ActiveDirectoryGroupsResolverTests issue: https://github.com/elastic/elasticsearch/issues/116182 -- class: org.elasticsearch.http.netty4.Netty4IncrementalRequestHandlingIT - method: testServerCloseConnectionMidStream - issue: https://github.com/elastic/elasticsearch/issues/116774 - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=snapshot/20_operator_privileges_disabled/Operator only settings can be set and restored by non-operator user when operator privileges is disabled} issue: https://github.com/elastic/elasticsearch/issues/116775 - class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT method: test {p0=synonyms/90_synonyms_reloading_for_synset/Reload analyzers for specific synonym set} issue: https://github.com/elastic/elasticsearch/issues/116777 +- class: org.elasticsearch.repositories.s3.RepositoryS3RestIT + method: testReloadCredentialsFromKeystore + issue: https://github.com/elastic/elasticsearch/issues/116811 +- class: org.elasticsearch.http.netty4.Netty4IncrementalRequestHandlingIT + method: testClientConnectionCloseMidStream + issue: https://github.com/elastic/elasticsearch/issues/116815 # Examples: # diff --git a/qa/entitlements/build.gradle b/qa/entitlements/build.gradle index 2621d2731f411..9a5058a3b11ac 100644 --- a/qa/entitlements/build.gradle +++ b/qa/entitlements/build.gradle @@ -18,21 +18,8 @@ esplugin { classname 'org.elasticsearch.test.entitlements.EntitlementsCheckPlugin' } -configurations { - entitlementBridge { - canBeConsumed = false - } -} - dependencies { clusterPlugins project(':qa:entitlements') - entitlementBridge project(':libs:entitlement:bridge') -} - -tasks.named('javaRestTest') { - systemProperty "tests.entitlement-bridge.jar-name", configurations.entitlementBridge.singleFile.getName() - usesDefaultDistribution() - systemProperty "tests.security.manager", "false" } tasks.named("javadoc").configure { diff --git a/qa/entitlements/src/javaRestTest/java/org/elasticsearch/test/entitlements/EntitlementsIT.java b/qa/entitlements/src/javaRestTest/java/org/elasticsearch/test/entitlements/EntitlementsIT.java index a62add89c51e6..8b3629527f918 100644 --- a/qa/entitlements/src/javaRestTest/java/org/elasticsearch/test/entitlements/EntitlementsIT.java +++ b/qa/entitlements/src/javaRestTest/java/org/elasticsearch/test/entitlements/EntitlementsIT.java @@ -10,9 +10,7 @@ package org.elasticsearch.test.entitlements; import org.elasticsearch.client.Request; -import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.cluster.ElasticsearchCluster; -import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.ClassRule; @@ -20,21 +18,13 @@ import static org.hamcrest.Matchers.containsString; -@ESTestCase.WithoutSecurityManager public class EntitlementsIT extends ESRestTestCase { - private static final String ENTITLEMENT_BRIDGE_JAR_NAME = System.getProperty("tests.entitlement-bridge.jar-name"); - @ClassRule public static ElasticsearchCluster cluster = ElasticsearchCluster.local() - .distribution(DistributionType.INTEG_TEST) .plugin("entitlement-qa") .systemProperty("es.entitlements.enabled", "true") .setting("xpack.security.enabled", "false") - .jvmArg("-Djdk.attach.allowAttachSelf=true") - .jvmArg("-XX:+EnableDynamicAgentLoading") - .jvmArg("--patch-module=java.base=lib/entitlement-bridge/" + ENTITLEMENT_BRIDGE_JAR_NAME) - .jvmArg("--add-exports=java.base/org.elasticsearch.entitlement.bridge=org.elasticsearch.entitlement") .build(); @Override diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java index fcca3f9a4700c..daadf936ae841 100644 --- a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java +++ b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java @@ -81,6 +81,7 @@ import static org.elasticsearch.test.MapMatcher.matchesMap; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -90,6 +91,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.startsWith; /** * Tests to run before and after a full cluster restart. This is run twice, @@ -1277,12 +1279,16 @@ private void checkSnapshot(String snapshotName, int count, String tookOnVersion, assertEquals(singletonList(snapshotName), XContentMapValues.extractValue("snapshots.snapshot", snapResponse)); assertEquals(singletonList("SUCCESS"), XContentMapValues.extractValue("snapshots.state", snapResponse)); // the format can change depending on the ES node version running & this test code running + // and if there's an in-progress release that hasn't been published yet, + // which could affect the top range of the index release version + String firstReleaseVersion = tookOnIndexVersion.toReleaseVersion().split("-")[0]; assertThat( - XContentMapValues.extractValue("snapshots.version", snapResponse), + (Iterable) XContentMapValues.extractValue("snapshots.version", snapResponse), anyOf( - equalTo(List.of(tookOnVersion)), - equalTo(List.of(tookOnIndexVersion.toString())), - equalTo(List.of(tookOnIndexVersion.toReleaseVersion())) + contains(tookOnVersion), + contains(tookOnIndexVersion.toString()), + contains(firstReleaseVersion), + contains(startsWith(firstReleaseVersion + "-")) ) ); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileRelocatingIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileRelocatingIT.java index 0d06856ca1088..4799b4bec0c8d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileRelocatingIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileRelocatingIT.java @@ -64,6 +64,8 @@ private void testSearchAndRelocateConcurrently(final int numberOfReplicas) throw } indexRandom(true, indexBuilders.toArray(new IndexRequestBuilder[indexBuilders.size()])); assertHitCount(prepareSearch(), (numDocs)); + // hold a copy of the node names before a new node is potentially added later + String[] nodeNamesBeforeClusterResize = internalCluster().getNodeNames(); final int numIters = scaledRandomIntBetween(5, 20); for (int i = 0; i < numIters; i++) { final AtomicBoolean stop = new AtomicBoolean(false); @@ -76,34 +78,37 @@ private void testSearchAndRelocateConcurrently(final int numberOfReplicas) throw public void run() { try { while (stop.get() == false) { - assertResponse(prepareSearch().setSize(numDocs), response -> { - if (response.getHits().getTotalHits().value() != numDocs) { - // if we did not search all shards but had no serious failures that is potentially fine - // if only the hit-count is wrong. this can happen if the cluster-state is behind when the - // request comes in. It's a small window but a known limitation. - if (response.getTotalShards() != response.getSuccessfulShards() - && Stream.of(response.getShardFailures()) - .allMatch(ssf -> ssf.getCause() instanceof NoShardAvailableActionException)) { - nonCriticalExceptions.add( - "Count is " - + response.getHits().getTotalHits().value() - + " but " - + numDocs - + " was expected. " - + formatShardStatus(response) - ); - } else { - assertHitCount(response, numDocs); + assertResponse( + client(randomFrom(nodeNamesBeforeClusterResize)).prepareSearch().setSize(numDocs), + response -> { + if (response.getHits().getTotalHits().value() != numDocs) { + // if we did not search all shards but had no serious failures that is potentially fine + // if only the hit-count is wrong. this can happen if the cluster-state is behind when the + // request comes in. It's a small window but a known limitation. + if (response.getTotalShards() != response.getSuccessfulShards() + && Stream.of(response.getShardFailures()) + .allMatch(ssf -> ssf.getCause() instanceof NoShardAvailableActionException)) { + nonCriticalExceptions.add( + "Count is " + + response.getHits().getTotalHits().value() + + " but " + + numDocs + + " was expected. " + + formatShardStatus(response) + ); + } else { + assertHitCount(response, numDocs); + } } - } - final SearchHits sh = response.getHits(); - assertThat( - "Expected hits to be the same size the actual hits array", - sh.getTotalHits().value(), - equalTo((long) (sh.getHits().length)) - ); - }); + final SearchHits sh = response.getHits(); + assertThat( + "Expected hits to be the same size the actual hits array", + sh.getTotalHits().value(), + equalTo((long) (sh.getHits().length)) + ); + } + ); // this is the more critical but that we hit the actual hit array has a different size than the // actual number of hits. } diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index c4aea73cc6141..eaf62d1e57e66 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -58,7 +58,7 @@ * sort them according to the provided order. This can be useful for instance to ensure that shards that contain recent * data are executed first when sorting by descending timestamp. */ -final class CanMatchPreFilterSearchPhase extends SearchPhase { +final class CanMatchPreFilterSearchPhase { private final Logger logger; private final SearchRequest request; @@ -92,7 +92,6 @@ final class CanMatchPreFilterSearchPhase extends SearchPhase { CoordinatorRewriteContextProvider coordinatorRewriteContextProvider, ActionListener> listener ) { - super("can_match"); this.logger = logger; this.searchTransportService = searchTransportService; this.nodeIdToConnection = nodeIdToConnection; @@ -128,12 +127,6 @@ private static boolean assertSearchCoordinationThread() { return ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH_COORDINATION); } - @Override - public void run() { - assert assertSearchCoordinationThread(); - runCoordinatorRewritePhase(); - } - // tries to pre-filter shards based on information that's available to the coordinator // without having to reach out to the actual shards private void runCoordinatorRewritePhase() { @@ -189,7 +182,7 @@ private void consumeResult(boolean canMatch, ShardSearchRequest request) { private void checkNoMissingShards(GroupShardsIterator shards) { assert assertSearchCoordinationThread(); - doCheckNoMissingShards(getName(), request, shards); + SearchPhase.doCheckNoMissingShards("can_match", request, shards, SearchPhase::makeMissingShardsError); } private Map> groupByNode(GroupShardsIterator shards) { @@ -318,7 +311,7 @@ public boolean isForceExecution() { @Override public void onFailure(Exception e) { if (logger.isDebugEnabled()) { - logger.debug(() -> format("Failed to execute [%s] while running [%s] phase", request, getName()), e); + logger.debug(() -> format("Failed to execute [%s] while running [can_match] phase", request), e); } onPhaseFailure("round", e); } @@ -370,7 +363,6 @@ public CanMatchNodeRequest.Shard buildShardLevelRequest(SearchShardIterator shar ); } - @Override public void start() { if (getNumShards() == 0) { finishPhase(); @@ -381,20 +373,21 @@ public void start() { @Override public void onFailure(Exception e) { if (logger.isDebugEnabled()) { - logger.debug(() -> format("Failed to execute [%s] while running [%s] phase", request, getName()), e); + logger.debug(() -> format("Failed to execute [%s] while running [can_match] phase", request), e); } onPhaseFailure("start", e); } @Override protected void doRun() { - CanMatchPreFilterSearchPhase.this.run(); + assert assertSearchCoordinationThread(); + runCoordinatorRewritePhase(); } }); } public void onPhaseFailure(String msg, Exception cause) { - listener.onFailure(new SearchPhaseExecutionException(getName(), msg, cause, ShardSearchFailure.EMPTY_ARRAY)); + listener.onFailure(new SearchPhaseExecutionException("can_match", msg, cause, ShardSearchFailure.EMPTY_ARRAY)); } public Transport.Connection getConnection(SendingTarget sendingTarget) { diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/SearchPhase.java index e4fef357cb4e9..d91ea85e2fa97 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchPhase.java @@ -15,8 +15,8 @@ import org.elasticsearch.transport.Transport; import java.io.IOException; -import java.io.UncheckedIOException; import java.util.Objects; +import java.util.function.Function; /** * Base class for all individual search phases like collecting distributed frequencies, fetching documents, querying shards. @@ -35,21 +35,26 @@ public String getName() { return name; } - public void start() { - try { - run(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + protected String missingShardsErrorMessage(StringBuilder missingShards) { + return makeMissingShardsError(missingShards); } - protected String missingShardsErrorMessage(StringBuilder missingShards) { + protected static String makeMissingShardsError(StringBuilder missingShards) { return "Search rejected due to missing shards [" + missingShards + "]. Consider using `allow_partial_search_results` setting to bypass this error."; } protected void doCheckNoMissingShards(String phaseName, SearchRequest request, GroupShardsIterator shardsIts) { + doCheckNoMissingShards(phaseName, request, shardsIts, this::missingShardsErrorMessage); + } + + protected static void doCheckNoMissingShards( + String phaseName, + SearchRequest request, + GroupShardsIterator shardsIts, + Function makeErrorMessage + ) { assert request.allowPartialSearchResults() != null : "SearchRequest missing setting for allowPartialSearchResults"; if (request.allowPartialSearchResults() == false) { final StringBuilder missingShards = new StringBuilder(); @@ -65,7 +70,7 @@ protected void doCheckNoMissingShards(String phaseName, SearchRequest request, G } if (missingShards.isEmpty() == false) { // Status red - shard is missing all copies and would produce partial results for an index search - final String msg = missingShardsErrorMessage(missingShards); + final String msg = makeErrorMessage.apply(missingShards); throw new SearchPhaseExecutionException(phaseName, msg, null, ShardSearchFailure.EMPTY_ARRAY); } } diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java index eee65134eae33..7ba4a7ce59869 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java @@ -148,7 +148,7 @@ private final class OpenPointInTimePhase implements TransportSearchAction.Search } @Override - public SearchPhase newSearchPhase( + public void runNewSearchPhase( SearchTask task, SearchRequest searchRequest, Executor executor, @@ -166,7 +166,7 @@ public SearchPhase newSearchPhase( // that is signaled to the local can match through the SearchShardIterator#prefiltered flag. Local shards do need to go // through the local can match phase. if (SearchService.canRewriteToMatchNone(searchRequest.source())) { - return new CanMatchPreFilterSearchPhase( + new CanMatchPreFilterSearchPhase( logger, searchTransportService, connectionLookup, @@ -180,7 +180,7 @@ public SearchPhase newSearchPhase( false, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), listener.delegateFailureAndWrap( - (searchResponseActionListener, searchShardIterators) -> openPointInTimePhase( + (searchResponseActionListener, searchShardIterators) -> runOpenPointInTimePhase( task, searchRequest, executor, @@ -191,11 +191,11 @@ public SearchPhase newSearchPhase( aliasFilter, concreteIndexBoosts, clusters - ).start() + ) ) - ); + ).start(); } else { - return openPointInTimePhase( + runOpenPointInTimePhase( task, searchRequest, executor, @@ -210,7 +210,7 @@ public SearchPhase newSearchPhase( } } - SearchPhase openPointInTimePhase( + void runOpenPointInTimePhase( SearchTask task, SearchRequest searchRequest, Executor executor, @@ -224,7 +224,7 @@ SearchPhase openPointInTimePhase( ) { assert searchRequest.getMaxConcurrentShardRequests() == pitRequest.maxConcurrentShardRequests() : searchRequest.getMaxConcurrentShardRequests() + " != " + pitRequest.maxConcurrentShardRequests(); - return new AbstractSearchAsyncAction<>( + new AbstractSearchAsyncAction<>( actionName, logger, namedWriteableRegistry, @@ -243,7 +243,6 @@ SearchPhase openPointInTimePhase( searchRequest.getMaxConcurrentShardRequests(), clusters ) { - @Override protected String missingShardsErrorMessage(StringBuilder missingShards) { return "[open_point_in_time] action requires all shards to be available. Missing shards: [" + missingShards @@ -290,7 +289,7 @@ public void run() { boolean buildPointInTimeFromSearchResults() { return true; } - }; + }.start(); } } diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 9aab5d005b1bb..4bca7a562fc38 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -1297,7 +1297,7 @@ private void executeSearch( localShardIterators.size() + remoteShardIterators.size(), defaultPreFilterShardSize ); - searchPhaseProvider.newSearchPhase( + searchPhaseProvider.runNewSearchPhase( task, searchRequest, asyncSearchExecutor, @@ -1310,7 +1310,7 @@ private void executeSearch( preFilterSearchShards, threadPool, clusters - ).start(); + ); } Executor asyncSearchExecutor(final String[] indices) { @@ -1414,7 +1414,7 @@ static GroupShardsIterator mergeShardsIterators( } interface SearchPhaseProvider { - SearchPhase newSearchPhase( + void runNewSearchPhase( SearchTask task, SearchRequest searchRequest, Executor executor, @@ -1438,7 +1438,7 @@ private class AsyncSearchActionProvider implements SearchPhaseProvider { } @Override - public SearchPhase newSearchPhase( + public void runNewSearchPhase( SearchTask task, SearchRequest searchRequest, Executor executor, @@ -1455,7 +1455,7 @@ public SearchPhase newSearchPhase( if (preFilter) { // only for aggs we need to contact shards even if there are no matches boolean requireAtLeastOneMatch = searchRequest.source() != null && searchRequest.source().aggregations() != null; - return new CanMatchPreFilterSearchPhase( + new CanMatchPreFilterSearchPhase( logger, searchTransportService, connectionLookup, @@ -1468,8 +1468,8 @@ public SearchPhase newSearchPhase( task, requireAtLeastOneMatch, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), - listener.delegateFailureAndWrap( - (l, iters) -> newSearchPhase( + listener.delegateFailureAndWrap((l, iters) -> { + runNewSearchPhase( task, searchRequest, executor, @@ -1482,9 +1482,10 @@ public SearchPhase newSearchPhase( false, threadPool, clusters - ).start() - ) - ); + ); + }) + ).start(); + return; } // for synchronous CCS minimize_roundtrips=false, use the CCSSingleCoordinatorSearchProgressListener // (AsyncSearchTask will not return SearchProgressListener.NOOP, since it uses its own progress listener @@ -1505,7 +1506,7 @@ public SearchPhase newSearchPhase( ); boolean success = false; try { - final SearchPhase searchPhase; + final AbstractSearchAsyncAction searchPhase; if (searchRequest.searchType() == DFS_QUERY_THEN_FETCH) { searchPhase = new SearchDfsQueryThenFetchAsyncAction( logger, @@ -1547,7 +1548,7 @@ public SearchPhase newSearchPhase( ); } success = true; - return searchPhase; + searchPhase.start(); } finally { if (success == false) { queryResultConsumer.close(); diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java index f418b5617b2a1..d8b57972d604f 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java @@ -146,7 +146,7 @@ public void searchShards(Task task, SearchShardsRequest searchShardsRequest, Act if (SearchService.canRewriteToMatchNone(searchRequest.source()) == false) { delegate.onResponse(new SearchShardsResponse(toGroups(shardIts), clusterState.nodes().getAllNodes(), aliasFilters)); } else { - var canMatchPhase = new CanMatchPreFilterSearchPhase(logger, searchTransportService, (clusterAlias, node) -> { + new CanMatchPreFilterSearchPhase(logger, searchTransportService, (clusterAlias, node) -> { assert Objects.equals(clusterAlias, searchShardsRequest.clusterAlias()); return transportService.getConnection(clusterState.nodes().get(node)); }, @@ -160,8 +160,7 @@ public void searchShards(Task task, SearchShardsRequest searchShardsRequest, Act false, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), delegate.map(its -> new SearchShardsResponse(toGroups(its), clusterState.nodes().getAllNodes(), aliasFilters)) - ); - canMatchPhase.start(); + ).start(); } }) ); diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java index 1c89d3bf259b5..aa92f395b20d2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -167,7 +167,8 @@ public void process(IndexRequest indexRequest) { // generate id if not already provided final String id = indexRequest.id(); if (id == null) { - if (creationVersion.onOrAfter(IndexVersions.TIME_BASED_K_ORDERED_DOC_ID) && indexMode == IndexMode.LOGSDB) { + if (creationVersion.between(IndexVersions.TIME_BASED_K_ORDERED_DOC_ID_BACKPORT, IndexVersions.UPGRADE_TO_LUCENE_10_0_0) + || creationVersion.onOrAfter(IndexVersions.TIME_BASED_K_ORDERED_DOC_ID) && indexMode == IndexMode.LOGSDB) { indexRequest.autoGenerateTimeBasedId(); } else { indexRequest.autoGenerateId(); diff --git a/server/src/main/java/org/elasticsearch/common/TimeBasedKOrderedUUIDGenerator.java b/server/src/main/java/org/elasticsearch/common/TimeBasedKOrderedUUIDGenerator.java index 9c97cb8fe7e85..7ea58ee326a79 100644 --- a/server/src/main/java/org/elasticsearch/common/TimeBasedKOrderedUUIDGenerator.java +++ b/server/src/main/java/org/elasticsearch/common/TimeBasedKOrderedUUIDGenerator.java @@ -10,7 +10,7 @@ package org.elasticsearch.common; import java.nio.ByteBuffer; -import java.util.Base64; +import java.util.function.Supplier; /** * Generates a base64-encoded, k-ordered UUID string optimized for compression and efficient indexing. @@ -28,18 +28,25 @@ * The result is a compact base64-encoded string, optimized for efficient compression of the _id field in an inverted index. */ public class TimeBasedKOrderedUUIDGenerator extends TimeBasedUUIDGenerator { - private static final Base64.Encoder BASE_64_NO_PADDING = Base64.getEncoder().withoutPadding(); + + public TimeBasedKOrderedUUIDGenerator( + final Supplier timestampSupplier, + final Supplier sequenceIdSupplier, + final Supplier macAddressSupplier + ) { + super(timestampSupplier, sequenceIdSupplier, macAddressSupplier); + } @Override public String getBase64UUID() { - final int sequenceId = this.sequenceNumber.incrementAndGet() & 0x00FF_FFFF; + final int sequenceId = sequenceNumber.incrementAndGet() & 0x00FF_FFFF; // Calculate timestamp to ensure ordering and avoid backward movement in case of time shifts. // Uses AtomicLong to guarantee that timestamp increases even if the system clock moves backward. // If the sequenceId overflows (reaches 0 within the same millisecond), the timestamp is incremented // to ensure strict ordering. long timestamp = this.lastTimestamp.accumulateAndGet( - currentTimeMillis(), + timestampSupplier.get(), sequenceId == 0 ? (lastTimestamp, currentTimeMillis) -> Math.max(lastTimestamp, currentTimeMillis) + 1 : Math::max ); @@ -68,6 +75,6 @@ public String getBase64UUID() { assert buffer.position() == uuidBytes.length; - return BASE_64_NO_PADDING.encodeToString(uuidBytes); + return Strings.BASE_64_NO_PADDING_URL_ENCODER.encodeToString(uuidBytes); } } diff --git a/server/src/main/java/org/elasticsearch/common/TimeBasedUUIDGenerator.java b/server/src/main/java/org/elasticsearch/common/TimeBasedUUIDGenerator.java index 2ed979ae66ffa..9da878fd4af64 100644 --- a/server/src/main/java/org/elasticsearch/common/TimeBasedUUIDGenerator.java +++ b/server/src/main/java/org/elasticsearch/common/TimeBasedUUIDGenerator.java @@ -11,6 +11,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; /** * These are essentially flake ids but we use 6 (not 8) bytes for timestamp, and use 3 (not 2) bytes for sequence number. We also reorder @@ -19,15 +20,14 @@ * For more information about flake ids, check out * https://archive.fo/2015.07.08-082503/http://www.boundary.com/blog/2012/01/flake-a-decentralized-k-ordered-unique-id-generator-in-erlang/ */ - class TimeBasedUUIDGenerator implements UUIDGenerator { // We only use bottom 3 bytes for the sequence number. Paranoia: init with random int so that if JVM/OS/machine goes down, clock slips // backwards, and JVM comes back up, we are less likely to be on the same sequenceNumber at the same time: - protected final AtomicInteger sequenceNumber = new AtomicInteger(SecureRandomHolder.INSTANCE.nextInt()); + protected final AtomicInteger sequenceNumber; + protected final AtomicLong lastTimestamp; - // Used to ensure clock moves forward: - protected final AtomicLong lastTimestamp = new AtomicLong(0); + protected final Supplier timestampSupplier; private static final byte[] SECURE_MUNGED_ADDRESS = MacAddressProvider.getSecureMungedAddress(); @@ -35,18 +35,26 @@ class TimeBasedUUIDGenerator implements UUIDGenerator { assert SECURE_MUNGED_ADDRESS.length == 6; } - // protected for testing - protected long currentTimeMillis() { - return System.currentTimeMillis(); + static final int SIZE_IN_BYTES = 15; + private final byte[] macAddress; + + TimeBasedUUIDGenerator( + final Supplier timestampSupplier, + final Supplier sequenceIdSupplier, + final Supplier macAddressSupplier + ) { + this.timestampSupplier = timestampSupplier; + // NOTE: getting the mac address every time using the supplier is expensive, hence we cache it. + this.macAddress = macAddressSupplier.get(); + this.sequenceNumber = new AtomicInteger(sequenceIdSupplier.get()); + // Used to ensure clock moves forward: + this.lastTimestamp = new AtomicLong(0); } - // protected for testing protected byte[] macAddress() { - return SECURE_MUNGED_ADDRESS; + return macAddress; } - static final int SIZE_IN_BYTES = 15; - @Override public String getBase64UUID() { final int sequenceId = sequenceNumber.incrementAndGet() & 0xffffff; @@ -55,7 +63,7 @@ public String getBase64UUID() { // still vulnerable if we are shut down, clock goes backwards, and we restart... for this we // randomize the sequenceNumber on init to decrease chance of collision: long timestamp = this.lastTimestamp.accumulateAndGet( - currentTimeMillis(), + timestampSupplier.get(), // Always force the clock to increment whenever sequence number is 0, in case we have a long // time-slip backwards: sequenceId == 0 ? (lastTimestamp, currentTimeMillis) -> Math.max(lastTimestamp, currentTimeMillis) + 1 : Math::max diff --git a/server/src/main/java/org/elasticsearch/common/UUIDs.java b/server/src/main/java/org/elasticsearch/common/UUIDs.java index 0f73b8172c10f..ebcb375bc01bc 100644 --- a/server/src/main/java/org/elasticsearch/common/UUIDs.java +++ b/server/src/main/java/org/elasticsearch/common/UUIDs.java @@ -12,13 +12,29 @@ import org.elasticsearch.common.settings.SecureString; import java.util.Random; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +/** + * Utility class for generating various types of UUIDs. + */ public class UUIDs { + private static final AtomicInteger sequenceNumber = new AtomicInteger(SecureRandomHolder.INSTANCE.nextInt()); + public static final Supplier DEFAULT_TIMESTAMP_SUPPLIER = System::currentTimeMillis; + public static final Supplier DEFAULT_SEQUENCE_ID_SUPPLIER = sequenceNumber::incrementAndGet; + public static final Supplier DEFAULT_MAC_ADDRESS_SUPPLIER = MacAddressProvider::getSecureMungedAddress; + private static final UUIDGenerator RANDOM_UUID_GENERATOR = new RandomBasedUUIDGenerator(); + private static final UUIDGenerator TIME_BASED_K_ORDERED_GENERATOR = new TimeBasedKOrderedUUIDGenerator( + DEFAULT_TIMESTAMP_SUPPLIER, + DEFAULT_SEQUENCE_ID_SUPPLIER, + DEFAULT_MAC_ADDRESS_SUPPLIER + ); - private static final RandomBasedUUIDGenerator RANDOM_UUID_GENERATOR = new RandomBasedUUIDGenerator(); - - private static final UUIDGenerator TIME_BASED_K_ORDERED_GENERATOR = new TimeBasedKOrderedUUIDGenerator(); - private static final UUIDGenerator TIME_UUID_GENERATOR = new TimeBasedUUIDGenerator(); + private static final UUIDGenerator TIME_UUID_GENERATOR = new TimeBasedUUIDGenerator( + DEFAULT_TIMESTAMP_SUPPLIER, + DEFAULT_SEQUENCE_ID_SUPPLIER, + DEFAULT_MAC_ADDRESS_SUPPLIER + ); /** * The length of a UUID string generated by {@link #base64UUID}. diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersions.java b/server/src/main/java/org/elasticsearch/index/IndexVersions.java index 9264b9e1c3a20..5746bea12a2d8 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexVersions.java +++ b/server/src/main/java/org/elasticsearch/index/IndexVersions.java @@ -130,6 +130,7 @@ private static Version parseUnchecked(String version) { public static final IndexVersion ENABLE_IGNORE_ABOVE_LOGSDB = def(8_517_00_0, Version.LUCENE_9_12_0); public static final IndexVersion ADD_ROLE_MAPPING_CLEANUP_MIGRATION = def(8_518_00_0, Version.LUCENE_9_12_0); public static final IndexVersion LOGSDB_DEFAULT_IGNORE_DYNAMIC_BEYOND_LIMIT_BACKPORT = def(8_519_00_0, Version.LUCENE_9_12_0); + public static final IndexVersion TIME_BASED_K_ORDERED_DOC_ID_BACKPORT = def(8_520_00_0, Version.LUCENE_9_12_0); public static final IndexVersion UPGRADE_TO_LUCENE_10_0_0 = def(9_000_00_0, Version.LUCENE_10_0_0); public static final IndexVersion LOGSDB_DEFAULT_IGNORE_DYNAMIC_BEYOND_LIMIT = def(9_001_00_0, Version.LUCENE_10_0_0); public static final IndexVersion TIME_BASED_K_ORDERED_DOC_ID = def(9_002_00_0, Version.LUCENE_10_0_0); diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesMetrics.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesMetrics.java index 2cd6e2b11ef7a..3a210199065b7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesMetrics.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesMetrics.java @@ -127,16 +127,7 @@ public static Map createAttributesMap( OperationPurpose purpose, String operation ) { - return Map.of( - "repo_type", - repositoryMetadata.type(), - "repo_name", - repositoryMetadata.name(), - "operation", - operation, - "purpose", - purpose.getKey() - ); + return Map.of("repo_type", repositoryMetadata.type(), "operation", operation, "purpose", purpose.getKey()); } } diff --git a/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java b/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java index cc5e96327b241..f55e3740aaa8f 100644 --- a/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java +++ b/server/src/main/java/org/elasticsearch/threadpool/ThreadPool.java @@ -1062,13 +1062,6 @@ public static boolean assertCurrentThreadPool(String... permittedThreadPoolNames return true; } - public static boolean assertTestThreadPool() { - final var threadName = Thread.currentThread().getName(); - final var executorName = EsExecutors.executorName(threadName); - assert threadName.startsWith("TEST-") || threadName.startsWith("LuceneTestCase") : threadName + " is not a test thread"; - return true; - } - public static boolean assertInSystemContext(ThreadPool threadPool) { final var threadName = Thread.currentThread().getName(); assert threadName.startsWith("TEST-") || threadName.startsWith("LuceneTestCase") || threadPool.getThreadContext().isSystemContext() diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java index 526961d74bf52..0c11123960622 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java @@ -16,12 +16,9 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.ArrayUtils; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.TermQueryBuilder; @@ -105,23 +102,6 @@ public void testSerialization() throws Exception { assertNotSame(deserializedRequest, searchRequest); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // this can be removed when the affected transport version constants are collapsed - public void testSerializationConstants() throws Exception { - SearchRequest searchRequest = createSearchRequest(); - - // something serialized with previous version to remove, should read correctly with the reversion - try (BytesStreamOutput output = new BytesStreamOutput()) { - output.setTransportVersion(TransportVersionUtils.getPreviousVersion(TransportVersions.REMOVE_MIN_COMPATIBLE_SHARD_NODE)); - searchRequest.writeTo(output); - try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) { - in.setTransportVersion(TransportVersions.REVERT_REMOVE_MIN_COMPATIBLE_SHARD_NODE); - SearchRequest copiedRequest = new SearchRequest(in); - assertEquals(copiedRequest, searchRequest); - assertEquals(copiedRequest.hashCode(), searchRequest.hashCode()); - } - } - } - public void testSerializationMultiKNN() throws Exception { SearchRequest searchRequest = createSearchRequest(); if (searchRequest.source() == null) { diff --git a/server/src/test/java/org/elasticsearch/common/TimeBasedUUIDGeneratorTests.java b/server/src/test/java/org/elasticsearch/common/TimeBasedUUIDGeneratorTests.java new file mode 100644 index 0000000000000..964683a1972ba --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/TimeBasedUUIDGeneratorTests.java @@ -0,0 +1,270 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.common; + +import org.elasticsearch.test.ESTestCase; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Base64; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.IntStream; + +public class TimeBasedUUIDGeneratorTests extends ESTestCase { + + public void testTimeBasedUUIDGeneration() { + assertUUIDFormat(createGenerator(() -> Instant.now().toEpochMilli(), () -> 0, new TestRandomMacAddressSupplier()), 100_000); + } + + public void testTimeBasedUUIDUniqueness() { + assertUUIDUniqueness(createGenerator(() -> Instant.now().toEpochMilli(), () -> 0, new TestRandomMacAddressSupplier()), 100_000); + } + + public void testTimeBasedUUIDSequenceOverflow() { + // The assumption here is that our system will not generate more than 1000 UUIDs within the same millisecond. + // The sequence ID is set close to its max value (0x00FF_FFFF) to quickly trigger an overflow. + // However, since we are generating only 1000 UUIDs, the timestamp is expected to change at least once, + // ensuring uniqueness even if the sequence ID wraps around. + assertEquals( + 1000, + generateUUIDs( + createGenerator(() -> Instant.now().toEpochMilli(), () -> 0x00FF_FFFF - 10, new TestRandomMacAddressSupplier()), + 1000 + ).size() + ); + } + + public void testTimeBasedUUIDClockReset() { + // Simulate a clock that resets itself after reaching a threshold. + final Supplier unreliableClock = new TestClockResetTimestampSupplier( + Instant.now(), + 1, + 50, + ChronoUnit.MILLIS, + Instant.now().plus(100, ChronoUnit.MILLIS) + ); + final UUIDGenerator generator = createGenerator(unreliableClock, () -> 0, new TestRandomMacAddressSupplier()); + + final Set beforeReset = generateUUIDs(generator, 5_000); + final Set afterReset = generateUUIDs(generator, 5_000); + + // Ensure all UUIDs are unique, even after the clock resets. + assertEquals(5_000, beforeReset.size()); + assertEquals(5_000, afterReset.size()); + beforeReset.addAll(afterReset); + assertEquals(10_000, beforeReset.size()); + } + + public void testKOrderedUUIDGeneration() { + assertUUIDFormat(createKOrderedGenerator(() -> Instant.now().toEpochMilli(), () -> 0, new TestRandomMacAddressSupplier()), 100_000); + } + + public void testKOrderedUUIDUniqueness() { + assertUUIDUniqueness( + createKOrderedGenerator(() -> Instant.now().toEpochMilli(), () -> 0, new TestRandomMacAddressSupplier()), + 100_000 + ); + } + + public void testKOrderedUUIDSequenceOverflow() { + final UUIDGenerator generator = createKOrderedGenerator( + () -> Instant.now().toEpochMilli(), + () -> 0x00FF_FFFF - 10, + new TestRandomMacAddressSupplier() + ); + final Set uuids = generateUUIDs(generator, 1000); + + // The assumption here is that our system will not generate more than 1000 UUIDs within the same millisecond. + // The sequence ID is set close to its max value (0x00FF_FFFF) to quickly trigger an overflow. + // However, since we are generating only 1000 UUIDs, the timestamp is expected to change at least once, + // ensuring uniqueness even if the sequence ID wraps around. + assertEquals(1000, uuids.size()); + } + + public void testUUIDEncodingDecoding() { + testUUIDEncodingDecodingHelper( + Instant.parse("2024-11-13T10:12:43Z").toEpochMilli(), + 12345, + new TestRandomMacAddressSupplier().get() + ); + } + + public void testUUIDEncodingDecodingWithRandomValues() { + testUUIDEncodingDecodingHelper( + randomInstantBetween(Instant.now().minus(1, ChronoUnit.DAYS), Instant.now()).toEpochMilli(), + randomIntBetween(0, 0x00FF_FFFF), + new TestRandomMacAddressSupplier().get() + ); + } + + private void testUUIDEncodingDecodingHelper(final long timestamp, final int sequenceId, final byte[] macAddress) { + final TestTimeBasedKOrderedUUIDDecoder decoder = new TestTimeBasedKOrderedUUIDDecoder( + createKOrderedGenerator(() -> timestamp, () -> sequenceId, () -> macAddress).getBase64UUID() + ); + + // The sequence ID is incremented by 1 when generating the UUID. + assertEquals("Sequence ID does not match", sequenceId + 1, decoder.decodeSequenceId()); + // Truncate the timestamp to milliseconds to match the UUID generation granularity. + assertEquals( + "Timestamp does not match", + Instant.ofEpochMilli(timestamp).truncatedTo(ChronoUnit.MILLIS), + Instant.ofEpochMilli(decoder.decodeTimestamp()).truncatedTo(ChronoUnit.MILLIS) + ); + assertArrayEquals("MAC address does not match", macAddress, decoder.decodeMacAddress()); + } + + private void assertUUIDUniqueness(final UUIDGenerator generator, final int count) { + assertEquals(count, generateUUIDs(generator, count).size()); + } + + private Set generateUUIDs(final UUIDGenerator generator, final int count) { + return IntStream.range(0, count).mapToObj(i -> generator.getBase64UUID()).collect(HashSet::new, Set::add, Set::addAll); + } + + private void assertUUIDFormat(final UUIDGenerator generator, final int count) { + IntStream.range(0, count).forEach(i -> { + final String uuid = generator.getBase64UUID(); + assertNotNull(uuid); + assertEquals(20, uuid.length()); + assertFalse(uuid.contains("+")); + assertFalse(uuid.contains("/")); + assertFalse(uuid.contains("=")); + }); + } + + private UUIDGenerator createGenerator( + final Supplier timestampSupplier, + final Supplier sequenceIdSupplier, + final Supplier macAddressSupplier + ) { + return new TimeBasedUUIDGenerator(timestampSupplier, sequenceIdSupplier, macAddressSupplier); + } + + private UUIDGenerator createKOrderedGenerator( + final Supplier timestampSupplier, + final Supplier sequenceIdSupplier, + final Supplier macAddressSupplier + ) { + return new TimeBasedKOrderedUUIDGenerator(timestampSupplier, sequenceIdSupplier, macAddressSupplier); + } + + private static class TestRandomMacAddressSupplier implements Supplier { + private final byte[] macAddress = new byte[] { randomByte(), randomByte(), randomByte(), randomByte(), randomByte(), randomByte() }; + + @Override + public byte[] get() { + return macAddress; + } + } + + /** + * A {@link Supplier} implementation that simulates a clock that can move forward or backward in time. + * This supplier provides timestamps in milliseconds since the epoch, adjusting based on a given delta + * until a reset threshold is reached. After crossing the threshold, the timestamp moves backwards by a reset delta. + */ + private static class TestClockResetTimestampSupplier implements Supplier { + private Instant currentTime; + private final long delta; + private final long resetDelta; + private final ChronoUnit unit; + private final Instant resetThreshold; + + /** + * Constructs a new {@link TestClockResetTimestampSupplier}. + * + * @param startTime The initial starting time. + * @param delta The amount of time to add to the current time in each forward step. + * @param resetDelta The amount of time to subtract once the reset threshold is reached. + * @param unit The unit of time for both delta and resetDelta. + * @param resetThreshold The threshold after which the time is reset backwards. + */ + TestClockResetTimestampSupplier( + final Instant startTime, + final long delta, + final long resetDelta, + final ChronoUnit unit, + final Instant resetThreshold + ) { + this.currentTime = startTime; + this.delta = delta; + this.resetDelta = resetDelta; + this.unit = unit; + this.resetThreshold = resetThreshold; + } + + /** + * Provides the next timestamp in milliseconds since the epoch. + * If the current time is before the reset threshold, it advances the time by the delta. + * Otherwise, it subtracts the reset delta. + * + * @return The current time in milliseconds since the epoch. + */ + @Override + public Long get() { + if (currentTime.isBefore(resetThreshold)) { + currentTime = currentTime.plus(delta, unit); + } else { + currentTime = currentTime.minus(resetDelta, unit); + } + return currentTime.toEpochMilli(); + } + } + + /** + * A utility class to decode the K-ordered UUID extracting the original timestamp, MAC address and sequence ID. + */ + private static class TestTimeBasedKOrderedUUIDDecoder { + + private final byte[] decodedBytes; + + /** + * Constructs a new {@link TestTimeBasedKOrderedUUIDDecoder} using a base64-encoded UUID string. + * + * @param base64UUID The base64-encoded UUID string to decode. + */ + TestTimeBasedKOrderedUUIDDecoder(final String base64UUID) { + this.decodedBytes = Base64.getUrlDecoder().decode(base64UUID); + } + + /** + * Decodes the timestamp from the UUID using the following bytes: + * 0 (most significant), 1, 2, 3, 11, 13 (least significant). + * + * @return The decoded timestamp in milliseconds. + */ + public long decodeTimestamp() { + return ((long) (decodedBytes[0] & 0xFF) << 40) | ((long) (decodedBytes[1] & 0xFF) << 32) | ((long) (decodedBytes[2] & 0xFF) + << 24) | ((long) (decodedBytes[3] & 0xFF) << 16) | ((long) (decodedBytes[11] & 0xFF) << 8) | (decodedBytes[13] & 0xFF); + } + + /** + * Decodes the MAC address from the UUID using bytes 4 to 9. + * + * @return The decoded MAC address as a byte array. + */ + public byte[] decodeMacAddress() { + byte[] macAddress = new byte[6]; + System.arraycopy(decodedBytes, 4, macAddress, 0, 6); + return macAddress; + } + + /** + * Decodes the sequence ID from the UUID using bytes: + * 10 (most significant), 12 (middle), 14 (least significant). + * + * @return The decoded sequence ID. + */ + public int decodeSequenceId() { + return ((decodedBytes[10] & 0xFF) << 16) | ((decodedBytes[12] & 0xFF) << 8) | (decodedBytes[14] & 0xFF); + } + } +} diff --git a/server/src/test/java/org/elasticsearch/common/UUIDTests.java b/server/src/test/java/org/elasticsearch/common/UUIDTests.java index 9fbeaf1c6c081..71c705f5df511 100644 --- a/server/src/test/java/org/elasticsearch/common/UUIDTests.java +++ b/server/src/test/java/org/elasticsearch/common/UUIDTests.java @@ -27,26 +27,37 @@ import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; +import java.util.Base64; import java.util.HashSet; import java.util.Random; import java.util.Set; +import java.util.function.Supplier; public class UUIDTests extends ESTestCase { - static UUIDGenerator timeUUIDGen = new TimeBasedUUIDGenerator(); + static final Base64.Decoder BASE_64_URL_DECODER = Base64.getUrlDecoder(); + static UUIDGenerator timeUUIDGen = new TimeBasedUUIDGenerator( + UUIDs.DEFAULT_TIMESTAMP_SUPPLIER, + UUIDs.DEFAULT_SEQUENCE_ID_SUPPLIER, + UUIDs.DEFAULT_MAC_ADDRESS_SUPPLIER + ); static UUIDGenerator randomUUIDGen = new RandomBasedUUIDGenerator(); - static UUIDGenerator kOrderedUUIDGen = new TimeBasedKOrderedUUIDGenerator(); + static UUIDGenerator kOrderedUUIDGen = new TimeBasedKOrderedUUIDGenerator( + UUIDs.DEFAULT_TIMESTAMP_SUPPLIER, + UUIDs.DEFAULT_SEQUENCE_ID_SUPPLIER, + UUIDs.DEFAULT_MAC_ADDRESS_SUPPLIER + ); public void testRandomUUID() { - verifyUUIDSet(100000, randomUUIDGen); + verifyUUIDSet(100000, randomUUIDGen).forEach(this::verifyUUIDIsUrlSafe); } public void testTimeUUID() { - verifyUUIDSet(100000, timeUUIDGen); + verifyUUIDSet(100000, timeUUIDGen).forEach(this::verifyUUIDIsUrlSafe); } public void testKOrderedUUID() { - verifyUUIDSet(100000, kOrderedUUIDGen); + verifyUUIDSet(100000, kOrderedUUIDGen).forEach(this::verifyUUIDIsUrlSafe); } public void testThreadedRandomUUID() { @@ -143,6 +154,7 @@ public void testUUIDThreaded(UUIDGenerator uuidSource) { globalSet.addAll(runner.uuidSet); } assertEquals(count * uuids, globalSet.size()); + globalSet.forEach(this::verifyUUIDIsUrlSafe); } private static double testCompression(final UUIDGenerator generator, int numDocs, int numDocsPerSecond, int numNodes, Logger logger) @@ -158,35 +170,25 @@ private static double testCompression(final UUIDGenerator generator, int numDocs UUIDGenerator uuidSource = generator; if (generator instanceof TimeBasedUUIDGenerator) { if (generator instanceof TimeBasedKOrderedUUIDGenerator) { - uuidSource = new TimeBasedKOrderedUUIDGenerator() { + uuidSource = new TimeBasedKOrderedUUIDGenerator(new Supplier<>() { double currentTimeMillis = TestUtil.nextLong(random(), 0L, 10000000000L); @Override - protected long currentTimeMillis() { + public Long get() { currentTimeMillis += intervalBetweenDocs * 2 * r.nextDouble(); return (long) currentTimeMillis; } - - @Override - protected byte[] macAddress() { - return RandomPicks.randomFrom(r, macAddresses); - } - }; + }, () -> 0, () -> RandomPicks.randomFrom(r, macAddresses)); } else { - uuidSource = new TimeBasedUUIDGenerator() { + uuidSource = new TimeBasedUUIDGenerator(new Supplier<>() { double currentTimeMillis = TestUtil.nextLong(random(), 0L, 10000000000L); @Override - protected long currentTimeMillis() { + public Long get() { currentTimeMillis += intervalBetweenDocs * 2 * r.nextDouble(); return (long) currentTimeMillis; } - - @Override - protected byte[] macAddress() { - return RandomPicks.randomFrom(r, macAddresses); - } - }; + }, () -> 0, () -> RandomPicks.randomFrom(r, macAddresses)); } } @@ -237,4 +239,13 @@ public void testStringLength() { private static int getUnpaddedBase64StringLength(int sizeInBytes) { return (int) Math.ceil(sizeInBytes * 4.0 / 3.0); } + + private void verifyUUIDIsUrlSafe(final String uuid) { + assertFalse("UUID should not contain padding characters: " + uuid, uuid.contains("=")); + try { + BASE_64_URL_DECODER.decode(uuid); + } catch (IllegalArgumentException e) { + throw new AssertionError("UUID is not a valid Base64 URL-safe encoded string: " + uuid); + } + } } diff --git a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixture.java b/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixture.java index 29123d6f2e7eb..421478a53e6bc 100644 --- a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixture.java +++ b/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixture.java @@ -26,10 +26,10 @@ public class S3HttpFixture extends ExternalResource { private HttpServer server; - private boolean enabled; + private final boolean enabled; private final String bucket; private final String basePath; - protected final String accessKey; + protected volatile String accessKey; public S3HttpFixture() { this(true); @@ -98,4 +98,8 @@ private static InetSocketAddress resolveAddress(String address, int port) { throw new RuntimeException(e); } } + + public void setAccessKey(String accessKey) { + this.accessKey = accessKey; + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestLegacyFeatures.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestLegacyFeatures.java index 194dfc057b84f..e43aa940a4881 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestLegacyFeatures.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestLegacyFeatures.java @@ -92,14 +92,6 @@ public class RestTestLegacyFeatures implements FeatureSpecification { @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) public static final NodeFeature ML_NLP_SUPPORTED = new NodeFeature("ml.nlp_supported"); - /* - * Starting with 8.11, cluster state has minimum system index mappings versions (#99307) and the system index mappings upgrade service - * started using them to determine when to update mappings for system indices. See https://github.com/elastic/elasticsearch/pull/99668 - */ - public static final NodeFeature MAPPINGS_UPGRADE_SERVICE_USES_MAPPINGS_VERSION = new NodeFeature( - "mappings.upgrade_service_uses_mappings_version" - ); - // YAML public static final NodeFeature REST_ELASTIC_PRODUCT_HEADER_PRESENT = new NodeFeature("action.rest.product_header_present"); @@ -134,8 +126,7 @@ public Map getHistoricalFeatures() { entry(DATA_STREAMS_SUPPORTED, Version.V_7_9_0), entry(NEW_DATA_STREAMS_INDEX_NAME_FORMAT, Version.V_7_11_0), entry(DISABLE_FIELD_NAMES_FIELD_REMOVED, Version.V_8_0_0), - entry(ML_NLP_SUPPORTED, Version.V_8_0_0), - entry(MAPPINGS_UPGRADE_SERVICE_USES_MAPPINGS_VERSION, Version.V_8_11_0) + entry(ML_NLP_SUPPORTED, Version.V_8_0_0) ); } } diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index ec1bf13bd993b..717cf96ad6a92 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -59,6 +59,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.util.function.Predicate.not; import static org.elasticsearch.test.cluster.local.distribution.DistributionType.DEFAULT; import static org.elasticsearch.test.cluster.util.OS.WINDOWS; @@ -755,18 +756,16 @@ private Map getEnvironmentVariables() { } String heapSize = System.getProperty("tests.heap.size", "512m"); - final String esJavaOpts = Stream.of( - "-Xms" + heapSize, - "-Xmx" + heapSize, - "-ea", - "-esa", - System.getProperty("tests.jvm.argline", ""), - featureFlagProperties, - systemProperties, - jvmArgs, - debugArgs - ).filter(s -> s.isEmpty() == false).collect(Collectors.joining(" ")); + List serverOpts = List.of("-Xms" + heapSize, "-Xmx" + heapSize, debugArgs, featureFlagProperties); + List commonOpts = List.of("-ea", "-esa", System.getProperty("tests.jvm.argline", ""), systemProperties, jvmArgs); + + String esJavaOpts = Stream.concat(serverOpts.stream(), commonOpts.stream()) + .filter(not(String::isEmpty)) + .collect(Collectors.joining(" ")); + String cliJavaOpts = commonOpts.stream().filter(not(String::isEmpty)).collect(Collectors.joining(" ")); + environment.put("ES_JAVA_OPTS", esJavaOpts); + environment.put("CLI_JAVA_OPTS", cliJavaOpts); return environment; } diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheMetrics.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheMetrics.java index a253b6bdd2360..0fb4267745cb8 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheMetrics.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheMetrics.java @@ -115,24 +115,16 @@ public LongHistogram getCacheMissLoadTimes() { * * @param bytesCopied The number of bytes copied * @param copyTimeNanos The time taken to copy the bytes in nanoseconds - * @param index The index being loaded - * @param shardId The ID of the shard being loaded * @param cachePopulationReason The reason for the cache being populated * @param cachePopulationSource The source from which the data is being loaded */ public void recordCachePopulationMetrics( int bytesCopied, long copyTimeNanos, - String index, - int shardId, CachePopulationReason cachePopulationReason, CachePopulationSource cachePopulationSource ) { Map metricAttributes = Map.of( - INDEX_ATTRIBUTE_KEY, - index, - SHARD_ID_ATTRIBUTE_KEY, - shardId, CACHE_POPULATION_REASON_ATTRIBUTE_KEY, cachePopulationReason.name(), CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY, diff --git a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/BlobCacheMetricsTests.java b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/BlobCacheMetricsTests.java index ea9d0b7356f0e..435798ba93a8b 100644 --- a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/BlobCacheMetricsTests.java +++ b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/BlobCacheMetricsTests.java @@ -30,15 +30,11 @@ public void createMetrics() { public void testRecordCachePopulationMetricsRecordsThroughput() { int mebiBytesSent = randomIntBetween(1, 4); int secondsTaken = randomIntBetween(1, 5); - String indexName = randomIdentifier(); - int shardId = randomIntBetween(0, 10); BlobCacheMetrics.CachePopulationReason cachePopulationReason = randomFrom(BlobCacheMetrics.CachePopulationReason.values()); CachePopulationSource cachePopulationSource = randomFrom(CachePopulationSource.values()); metrics.recordCachePopulationMetrics( Math.toIntExact(ByteSizeValue.ofMb(mebiBytesSent).getBytes()), TimeUnit.SECONDS.toNanos(secondsTaken), - indexName, - shardId, cachePopulationReason, cachePopulationSource ); @@ -48,32 +44,28 @@ public void testRecordCachePopulationMetricsRecordsThroughput() { .getMeasurements(InstrumentType.DOUBLE_HISTOGRAM, "es.blob_cache.population.throughput.histogram") .get(0); assertEquals(throughputMeasurement.getDouble(), (double) mebiBytesSent / secondsTaken, 0.0); - assertExpectedAttributesPresent(throughputMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource); + assertExpectedAttributesPresent(throughputMeasurement, cachePopulationReason, cachePopulationSource); // bytes counter Measurement totalBytesMeasurement = recordingMeterRegistry.getRecorder() .getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.bytes.total") .get(0); assertEquals(totalBytesMeasurement.getLong(), ByteSizeValue.ofMb(mebiBytesSent).getBytes()); - assertExpectedAttributesPresent(totalBytesMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource); + assertExpectedAttributesPresent(totalBytesMeasurement, cachePopulationReason, cachePopulationSource); // time counter Measurement totalTimeMeasurement = recordingMeterRegistry.getRecorder() .getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.time.total") .get(0); assertEquals(totalTimeMeasurement.getLong(), TimeUnit.SECONDS.toMillis(secondsTaken)); - assertExpectedAttributesPresent(totalTimeMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource); + assertExpectedAttributesPresent(totalTimeMeasurement, cachePopulationReason, cachePopulationSource); } private static void assertExpectedAttributesPresent( Measurement measurement, - int shardId, - String indexName, BlobCacheMetrics.CachePopulationReason cachePopulationReason, CachePopulationSource cachePopulationSource ) { - assertEquals(measurement.attributes().get(BlobCacheMetrics.SHARD_ID_ATTRIBUTE_KEY), shardId); - assertEquals(measurement.attributes().get(BlobCacheMetrics.INDEX_ATTRIBUTE_KEY), indexName); assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_REASON_ATTRIBUTE_KEY), cachePopulationReason.name()); assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY), cachePopulationSource.name()); } diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMaxFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMaxFactory.java new file mode 100644 index 0000000000000..ba7de22b1b821 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMaxFactory.java @@ -0,0 +1,146 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BlockFactory; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.SourceOperator; +import org.elasticsearch.search.MultiValueMode; + +import java.io.IOException; +import java.util.List; +import java.util.function.Function; + +/** + * Factory that generates an operator that finds the max value of a field using the {@link LuceneMinMaxOperator}. + */ +public final class LuceneMaxFactory extends LuceneOperator.Factory { + + public enum NumberType implements LuceneMinMaxOperator.NumberType { + INTEGER { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantIntBlockWith(Math.toIntExact(result), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantIntBlockWith(Integer.MIN_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToInt(bytes, 0); + } + }, + FLOAT { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantFloatBlockWith(NumericUtils.sortableIntToFloat(Math.toIntExact(result)), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantFloatBlockWith(-Float.MAX_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToInt(bytes, 0); + } + }, + LONG { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantLongBlockWith(result, pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantLongBlockWith(Long.MIN_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToLong(bytes, 0); + } + }, + DOUBLE { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantDoubleBlockWith(NumericUtils.sortableLongToDouble(result), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantDoubleBlockWith(-Double.MAX_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToLong(bytes, 0); + } + }; + + public final NumericDocValues multiValueMode(SortedNumericDocValues sortedNumericDocValues) { + return MultiValueMode.MAX.select(sortedNumericDocValues); + } + + public final long fromPointValues(PointValues pointValues) throws IOException { + return bytesToLong(pointValues.getMaxPackedValue()); + } + + public final long evaluate(long value1, long value2) { + return Math.max(value1, value2); + } + + abstract long bytesToLong(byte[] bytes); + } + + private final String fieldName; + private final NumberType numberType; + + public LuceneMaxFactory( + List contexts, + Function queryFunction, + DataPartitioning dataPartitioning, + int taskConcurrency, + String fieldName, + NumberType numberType, + int limit + ) { + super(contexts, queryFunction, dataPartitioning, taskConcurrency, limit, ScoreMode.COMPLETE_NO_SCORES); + this.fieldName = fieldName; + this.numberType = numberType; + } + + @Override + public SourceOperator get(DriverContext driverContext) { + return new LuceneMinMaxOperator(driverContext.blockFactory(), sliceQueue, fieldName, numberType, limit, Long.MIN_VALUE); + } + + @Override + public String describe() { + return "LuceneMaxOperator[type = " + + numberType.name() + + ", dataPartitioning = " + + dataPartitioning + + ", fieldName = " + + fieldName + + ", limit = " + + limit + + "]"; + } +} diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinFactory.java new file mode 100644 index 0000000000000..e3c6c8310373d --- /dev/null +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinFactory.java @@ -0,0 +1,146 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BlockFactory; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.SourceOperator; +import org.elasticsearch.search.MultiValueMode; + +import java.io.IOException; +import java.util.List; +import java.util.function.Function; + +/** + * Factory that generates an operator that finds the min value of a field using the {@link LuceneMinMaxOperator}. + */ +public final class LuceneMinFactory extends LuceneOperator.Factory { + + public enum NumberType implements LuceneMinMaxOperator.NumberType { + INTEGER { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantIntBlockWith(Math.toIntExact(result), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantIntBlockWith(Integer.MAX_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToInt(bytes, 0); + } + }, + FLOAT { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantFloatBlockWith(NumericUtils.sortableIntToFloat(Math.toIntExact(result)), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantFloatBlockWith(Float.POSITIVE_INFINITY, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToInt(bytes, 0); + } + }, + LONG { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantLongBlockWith(result, pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantLongBlockWith(Long.MAX_VALUE, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToLong(bytes, 0); + } + }, + DOUBLE { + @Override + public Block buildResult(BlockFactory blockFactory, long result, int pageSize) { + return blockFactory.newConstantDoubleBlockWith(NumericUtils.sortableLongToDouble(result), pageSize); + } + + @Override + public Block buildEmptyResult(BlockFactory blockFactory, int pageSize) { + return blockFactory.newConstantDoubleBlockWith(Double.POSITIVE_INFINITY, pageSize); + } + + @Override + long bytesToLong(byte[] bytes) { + return NumericUtils.sortableBytesToLong(bytes, 0); + } + }; + + public final NumericDocValues multiValueMode(SortedNumericDocValues sortedNumericDocValues) { + return MultiValueMode.MIN.select(sortedNumericDocValues); + } + + public final long fromPointValues(PointValues pointValues) throws IOException { + return bytesToLong(pointValues.getMinPackedValue()); + } + + public final long evaluate(long value1, long value2) { + return Math.min(value1, value2); + } + + abstract long bytesToLong(byte[] bytes); + } + + private final String fieldName; + private final NumberType numberType; + + public LuceneMinFactory( + List contexts, + Function queryFunction, + DataPartitioning dataPartitioning, + int taskConcurrency, + String fieldName, + NumberType numberType, + int limit + ) { + super(contexts, queryFunction, dataPartitioning, taskConcurrency, limit, ScoreMode.COMPLETE_NO_SCORES); + this.fieldName = fieldName; + this.numberType = numberType; + } + + @Override + public SourceOperator get(DriverContext driverContext) { + return new LuceneMinMaxOperator(driverContext.blockFactory(), sliceQueue, fieldName, numberType, limit, Long.MAX_VALUE); + } + + @Override + public String describe() { + return "LuceneMinOperator[type = " + + numberType.name() + + ", dataPartitioning = " + + dataPartitioning + + ", fieldName = " + + fieldName + + ", limit = " + + limit + + "]"; + } +} diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinMaxOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinMaxOperator.java new file mode 100644 index 0000000000000..c41c31345df4e --- /dev/null +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinMaxOperator.java @@ -0,0 +1,179 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.search.LeafCollector; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.util.Bits; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BlockFactory; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.search.MultiValueMode; + +import java.io.IOException; + +/** + * Operator that finds the min or max value of a field using Lucene searches + * and returns always one entry that mimics the min/max aggregation internal state: + * 1. the min/max with a type depending on the {@link NumberType} (The initial value if no doc is seen) + * 2. a bool flag (seen) that is true if at least one document has been matched, otherwise false + *

+ * It works for fields that index data using lucene {@link PointValues} and/or {@link SortedNumericDocValues}. + * It assumes that {@link SortedNumericDocValues} are always present. + */ +final class LuceneMinMaxOperator extends LuceneOperator { + + sealed interface NumberType permits LuceneMinFactory.NumberType, LuceneMaxFactory.NumberType { + + /** Extract the competitive value from the {@link PointValues} */ + long fromPointValues(PointValues pointValues) throws IOException; + + /** Wraps the provided {@link SortedNumericDocValues} with a {@link MultiValueMode} */ + NumericDocValues multiValueMode(SortedNumericDocValues sortedNumericDocValues); + + /** Return the competitive value between {@code value1} and {@code value2} */ + long evaluate(long value1, long value2); + + /** Build the corresponding block */ + Block buildResult(BlockFactory blockFactory, long result, int pageSize); + + /** Build the corresponding block */ + Block buildEmptyResult(BlockFactory blockFactory, int pageSize); + } + + private static final int PAGE_SIZE = 1; + + private boolean seen = false; + private int remainingDocs; + private long result; + + private final NumberType numberType; + + private final String fieldName; + + LuceneMinMaxOperator( + BlockFactory blockFactory, + LuceneSliceQueue sliceQueue, + String fieldName, + NumberType numberType, + int limit, + long initialResult + ) { + super(blockFactory, PAGE_SIZE, sliceQueue); + this.remainingDocs = limit; + this.numberType = numberType; + this.fieldName = fieldName; + this.result = initialResult; + } + + @Override + public boolean isFinished() { + return doneCollecting || remainingDocs == 0; + } + + @Override + public void finish() { + doneCollecting = true; + } + + @Override + public Page getCheckedOutput() throws IOException { + if (isFinished()) { + assert remainingDocs <= 0 : remainingDocs; + return null; + } + final long start = System.nanoTime(); + try { + final LuceneScorer scorer = getCurrentOrLoadNextScorer(); + // no scorer means no more docs + if (scorer == null) { + remainingDocs = 0; + } else { + final LeafReader reader = scorer.leafReaderContext().reader(); + final Query query = scorer.weight().getQuery(); + if (query == null || query instanceof MatchAllDocsQuery) { + final PointValues pointValues = reader.getPointValues(fieldName); + // only apply shortcut if we are visiting all documents, otherwise we need to trigger the search + // on doc values as that's the order they are visited without push down. + if (pointValues != null && pointValues.getDocCount() >= remainingDocs) { + final Bits liveDocs = reader.getLiveDocs(); + if (liveDocs == null) { + // In data partitioning, we might have got the same segment previous + // to this but with a different document range. And we're totally ignoring that range. + // We're just reading the min/max from the segment. That's sneaky, but it makes sense. + // And if we get another slice in the same segment we may as well skip it - + // we've already looked. + if (scorer.position() == 0) { + seen = true; + result = numberType.evaluate(result, numberType.fromPointValues(pointValues)); + if (remainingDocs != NO_LIMIT) { + remainingDocs -= pointValues.getDocCount(); + } + } + scorer.markAsDone(); + } + } + } + if (scorer.isDone() == false) { + // could not apply shortcut, trigger the search + final NumericDocValues values = numberType.multiValueMode(reader.getSortedNumericDocValues(fieldName)); + final LeafCollector leafCollector = new LeafCollector() { + @Override + public void setScorer(Scorable scorer) {} + + @Override + public void collect(int doc) throws IOException { + assert remainingDocs > 0; + remainingDocs--; + if (values.advanceExact(doc)) { + seen = true; + result = numberType.evaluate(result, values.longValue()); + } + } + }; + scorer.scoreNextRange(leafCollector, reader.getLiveDocs(), remainingDocs); + } + } + + Page page = null; + // emit only one page + if (remainingDocs <= 0 && pagesEmitted == 0) { + pagesEmitted++; + Block result = null; + BooleanBlock seen = null; + try { + result = this.seen + ? numberType.buildResult(blockFactory, this.result, PAGE_SIZE) + : numberType.buildEmptyResult(blockFactory, PAGE_SIZE); + seen = blockFactory.newConstantBooleanBlockWith(this.seen, PAGE_SIZE); + page = new Page(PAGE_SIZE, result, seen); + } finally { + if (page == null) { + Releasables.closeExpectNoException(result, seen); + } + } + } + return page; + } finally { + processingNanos += System.nanoTime() - start; + } + } + + @Override + protected void describe(StringBuilder sb) { + sb.append(", remainingDocs=").append(remainingDocs); + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxDoubleOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxDoubleOperatorTests.java new file mode 100644 index 0000000000000..4cb113457b23f --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxDoubleOperatorTests.java @@ -0,0 +1,88 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.DoubleField; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MaxDoubleAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.DoubleBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; + +public class LuceneMaxDoubleOperatorTests extends LuceneMaxOperatorTestCase { + + @Override + public LuceneMaxFactory.NumberType getNumberType() { + return LuceneMaxFactory.NumberType.DOUBLE; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + double max = -Double.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new DoubleField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, NumericUtils.doubleToSortableLong(newValue())); + } + + private double newValue() { + final double value = randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true); + max = Math.max(max, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(DoubleBlock.class)); + final DoubleBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getDouble(0), equalTo(-Double.MAX_VALUE)); + } else { + assertThat(db.getDouble(0), lessThanOrEqualTo(max)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MaxDoubleAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMaxValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(DoubleBlock.class)); + final DoubleBlock db = (DoubleBlock) block; + if (exactResult) { + assertThat(db.getDouble(0), equalTo(max)); + } else { + assertThat(db.getDouble(0), lessThanOrEqualTo(max)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxFloatOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxFloatOperatorTests.java new file mode 100644 index 0000000000000..4a009a2d84c66 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxFloatOperatorTests.java @@ -0,0 +1,88 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.FloatField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MaxFloatAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.FloatBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; + +public class LuceneMaxFloatOperatorTests extends LuceneMaxOperatorTestCase { + + @Override + public LuceneMaxFactory.NumberType getNumberType() { + return LuceneMaxFactory.NumberType.FLOAT; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + float max = -Float.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new FloatField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + private float newValue() { + final float value = randomFloatBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true); + max = Math.max(max, value); + return value; + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, NumericUtils.floatToSortableInt(newValue())); + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(FloatBlock.class)); + final FloatBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getFloat(0), equalTo(-Float.MAX_VALUE)); + } else { + assertThat(db.getFloat(0), lessThanOrEqualTo(max)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MaxFloatAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMaxValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(FloatBlock.class)); + final FloatBlock fb = (FloatBlock) block; + if (exactResult) { + assertThat(fb.getFloat(0), equalTo(max)); + } else { + assertThat(fb.getFloat(0), lessThanOrEqualTo(max)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxIntOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxIntOperatorTests.java new file mode 100644 index 0000000000000..a6118481ca43d --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxIntOperatorTests.java @@ -0,0 +1,87 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.IntField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MaxIntAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.IntBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; + +public class LuceneMaxIntOperatorTests extends LuceneMaxOperatorTestCase { + + @Override + public LuceneMaxFactory.NumberType getNumberType() { + return LuceneMaxFactory.NumberType.INTEGER; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + int max = Integer.MIN_VALUE; + + @Override + public IndexableField newPointField() { + return new IntField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + private int newValue() { + final int value = randomInt(); + max = Math.max(max, value); + return value; + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, newValue()); + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(IntBlock.class)); + final IntBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getInt(0), equalTo(Integer.MIN_VALUE)); + } else { + assertThat(db.getInt(0), lessThanOrEqualTo(max)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MaxIntAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMaxValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(IntBlock.class)); + final IntBlock ib = (IntBlock) block; + if (exactResult) { + assertThat(ib.getInt(0), equalTo(max)); + } else { + assertThat(ib.getInt(0), lessThanOrEqualTo(max)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxLongOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxLongOperatorTests.java new file mode 100644 index 0000000000000..894c8e862123e --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxLongOperatorTests.java @@ -0,0 +1,87 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.LongField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MaxLongAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.lessThanOrEqualTo; + +public class LuceneMaxLongOperatorTests extends LuceneMaxOperatorTestCase { + + @Override + public LuceneMaxFactory.NumberType getNumberType() { + return LuceneMaxFactory.NumberType.LONG; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + long max = Long.MIN_VALUE; + + @Override + public IndexableField newPointField() { + return new LongField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, newValue()); + } + + private long newValue() { + final long value = randomLong(); + max = Math.max(max, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(LongBlock.class)); + final LongBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getLong(0), equalTo(Long.MIN_VALUE)); + } else { + assertThat(db.getLong(0), lessThanOrEqualTo(max)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MaxLongAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMaxValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(LongBlock.class)); + final LongBlock lb = (LongBlock) block; + if (exactResult) { + assertThat(lb.getLong(0), equalTo(max)); + } else { + assertThat(lb.getLong(0), lessThanOrEqualTo(max)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxOperatorTestCase.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxOperatorTestCase.java new file mode 100644 index 0000000000000..f5214dccbd00c --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMaxOperatorTestCase.java @@ -0,0 +1,210 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Document; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.NoMergePolicy; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.index.RandomIndexWriter; +import org.elasticsearch.common.breaker.CircuitBreakingException; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.AnyOperatorTestCase; +import org.elasticsearch.compute.operator.Driver; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.OperatorTestCase; +import org.elasticsearch.compute.operator.TestResultPageSinkOperator; +import org.elasticsearch.core.IOUtils; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.indices.CrankyCircuitBreakerService; +import org.hamcrest.Matcher; +import org.junit.After; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Supplier; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.matchesRegex; + +public abstract class LuceneMaxOperatorTestCase extends AnyOperatorTestCase { + + protected interface NumberTypeTest { + + IndexableField newPointField(); + + IndexableField newDocValuesField(); + + void assertPage(Page page); + + AggregatorFunction newAggregatorFunction(DriverContext context); + + void assertMaxValue(Block block, boolean exactResult); + + } + + protected abstract NumberTypeTest getNumberTypeTest(); + + protected abstract LuceneMaxFactory.NumberType getNumberType(); + + protected static final String FIELD_NAME = "field"; + private final Directory directory = newDirectory(); + private IndexReader reader; + + @After + public void closeIndex() throws IOException { + IOUtils.close(reader, directory); + } + + @Override + protected LuceneMaxFactory simple() { + return simple(getNumberTypeTest(), randomFrom(DataPartitioning.values()), between(1, 10_000), 100); + } + + private LuceneMaxFactory simple(NumberTypeTest numberTypeTest, DataPartitioning dataPartitioning, int numDocs, int limit) { + final boolean enableShortcut = randomBoolean(); + final boolean enableMultiValue = randomBoolean(); + final int commitEvery = Math.max(1, numDocs / 10); + try ( + RandomIndexWriter writer = new RandomIndexWriter( + random(), + directory, + newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE) + ) + ) { + + for (int d = 0; d < numDocs; d++) { + final var numValues = enableMultiValue ? randomIntBetween(1, 5) : 1; + final var doc = new Document(); + for (int i = 0; i < numValues; i++) { + if (enableShortcut) { + doc.add(numberTypeTest.newPointField()); + } else { + doc.add(numberTypeTest.newDocValuesField()); + } + } + writer.addDocument(doc); + if (d % commitEvery == 0) { + writer.commit(); + } + } + reader = writer.getReader(); + } catch (IOException e) { + throw new RuntimeException(e); + } + + final ShardContext ctx = new LuceneSourceOperatorTests.MockShardContext(reader, 0); + final Query query; + if (enableShortcut && randomBoolean()) { + query = new MatchAllDocsQuery(); + } else { + query = SortedNumericDocValuesField.newSlowRangeQuery(FIELD_NAME, Long.MIN_VALUE, Long.MAX_VALUE); + } + return new LuceneMaxFactory(List.of(ctx), c -> query, dataPartitioning, between(1, 8), FIELD_NAME, getNumberType(), limit); + } + + public void testSimple() { + testSimple(this::driverContext); + } + + public void testSimpleWithCranky() { + try { + testSimple(this::crankyDriverContext); + logger.info("cranky didn't break"); + } catch (CircuitBreakingException e) { + logger.info("broken", e); + assertThat(e.getMessage(), equalTo(CrankyCircuitBreakerService.ERROR_MESSAGE)); + } + } + + private void testSimple(Supplier contexts) { + int size = between(1_000, 20_000); + int limit = randomBoolean() ? between(10, size) : Integer.MAX_VALUE; + testMax(contexts, size, limit); + } + + public void testEmpty() { + testEmpty(this::driverContext); + } + + public void testEmptyWithCranky() { + try { + testEmpty(this::crankyDriverContext); + logger.info("cranky didn't break"); + } catch (CircuitBreakingException e) { + logger.info("broken", e); + assertThat(e.getMessage(), equalTo(CrankyCircuitBreakerService.ERROR_MESSAGE)); + } + } + + private void testEmpty(Supplier contexts) { + int limit = randomBoolean() ? between(10, 10000) : Integer.MAX_VALUE; + testMax(contexts, 0, limit); + } + + private void testMax(Supplier contexts, int size, int limit) { + DataPartitioning dataPartitioning = randomFrom(DataPartitioning.values()); + NumberTypeTest numberTypeTest = getNumberTypeTest(); + LuceneMaxFactory factory = simple(numberTypeTest, dataPartitioning, size, limit); + List results = new CopyOnWriteArrayList<>(); + List drivers = new ArrayList<>(); + int taskConcurrency = between(1, 8); + for (int i = 0; i < taskConcurrency; i++) { + DriverContext ctx = contexts.get(); + drivers.add(new Driver(ctx, factory.get(ctx), List.of(), new TestResultPageSinkOperator(results::add), () -> {})); + } + OperatorTestCase.runDriver(drivers); + assertThat(results.size(), lessThanOrEqualTo(taskConcurrency)); + + try (AggregatorFunction aggregatorFunction = numberTypeTest.newAggregatorFunction(contexts.get())) { + for (Page page : results) { + assertThat(page.getPositionCount(), is(1)); // one row + assertThat(page.getBlockCount(), is(2)); // two blocks + numberTypeTest.assertPage(page); + aggregatorFunction.addIntermediateInput(page); + } + + final Block[] result = new Block[1]; + try { + aggregatorFunction.evaluateFinal(result, 0, contexts.get()); + if (result[0].areAllValuesNull() == false) { + boolean exactResult = size <= limit; + numberTypeTest.assertMaxValue(result[0], exactResult); + } + } finally { + Releasables.close(result); + } + } + } + + @Override + protected final Matcher expectedToStringOfSimple() { + return matchesRegex("LuceneMinMaxOperator\\[maxPageSize = \\d+, remainingDocs=100]"); + } + + @Override + protected final Matcher expectedDescriptionOfSimple() { + return matchesRegex( + "LuceneMaxOperator\\[type = " + + getNumberType().name() + + ", dataPartitioning = (DOC|SHARD|SEGMENT), fieldName = " + + FIELD_NAME + + ", limit = 100]" + ); + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinDoubleOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinDoubleOperatorTests.java new file mode 100644 index 0000000000000..5fef2d4897030 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinDoubleOperatorTests.java @@ -0,0 +1,88 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.DoubleField; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MinDoubleAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.DoubleBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.instanceOf; + +public class LuceneMinDoubleOperatorTests extends LuceneMinOperatorTestCase { + + @Override + public LuceneMinFactory.NumberType getNumberType() { + return LuceneMinFactory.NumberType.DOUBLE; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + double min = Double.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new DoubleField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, NumericUtils.doubleToSortableLong(newValue())); + } + + private double newValue() { + final double value = randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true); + min = Math.min(min, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(DoubleBlock.class)); + final DoubleBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getDouble(0), equalTo(Double.POSITIVE_INFINITY)); + } else { + assertThat(db.getDouble(0), greaterThanOrEqualTo(min)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MinDoubleAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMinValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(DoubleBlock.class)); + final DoubleBlock db = (DoubleBlock) block; + if (exactResult) { + assertThat(db.getDouble(0), equalTo(min)); + } else { + assertThat(db.getDouble(0), greaterThanOrEqualTo(min)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinFloatOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinFloatOperatorTests.java new file mode 100644 index 0000000000000..41c8751c08a96 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinFloatOperatorTests.java @@ -0,0 +1,89 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.FloatField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MinFloatAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.FloatBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.instanceOf; + +public class LuceneMinFloatOperatorTests extends LuceneMinOperatorTestCase { + + @Override + public LuceneMinFactory.NumberType getNumberType() { + return LuceneMinFactory.NumberType.FLOAT; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + float min = Float.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new FloatField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, NumericUtils.floatToSortableInt(newValue())); + } + + private float newValue() { + final float value = randomFloatBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true); + min = Math.min(min, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(FloatBlock.class)); + final FloatBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + final float v = db.getFloat(0); + if (bb.getBoolean(0) == false) { + assertThat(db.getFloat(0), equalTo(Float.POSITIVE_INFINITY)); + } else { + assertThat(db.getFloat(0), greaterThanOrEqualTo(min)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MinFloatAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMinValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(FloatBlock.class)); + final FloatBlock fb = (FloatBlock) block; + if (exactResult) { + assertThat(fb.getFloat(0), equalTo(min)); + } else { + assertThat(fb.getFloat(0), greaterThanOrEqualTo(min)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinIntegerOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinIntegerOperatorTests.java new file mode 100644 index 0000000000000..5d2c867f4f660 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinIntegerOperatorTests.java @@ -0,0 +1,87 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.IntField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MinIntAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.IntBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.instanceOf; + +public class LuceneMinIntegerOperatorTests extends LuceneMinOperatorTestCase { + + @Override + public LuceneMinFactory.NumberType getNumberType() { + return LuceneMinFactory.NumberType.INTEGER; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + int min = Integer.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new IntField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, newValue()); + } + + private int newValue() { + final int value = randomInt(); + min = Math.min(min, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(IntBlock.class)); + IntBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getInt(0), equalTo(Integer.MAX_VALUE)); + } else { + assertThat(db.getInt(0), greaterThanOrEqualTo(min)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MinIntAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMinValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(IntBlock.class)); + final IntBlock ib = (IntBlock) block; + if (exactResult) { + assertThat(ib.getInt(0), equalTo(min)); + } else { + assertThat(ib.getInt(0), greaterThanOrEqualTo(min)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinLongOperatorTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinLongOperatorTests.java new file mode 100644 index 0000000000000..15c34f5853ae2 --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinLongOperatorTests.java @@ -0,0 +1,87 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Field; +import org.apache.lucene.document.LongField; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.aggregation.MinLongAggregatorFunctionSupplier; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BooleanBlock; +import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.DriverContext; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.instanceOf; + +public class LuceneMinLongOperatorTests extends LuceneMinOperatorTestCase { + + @Override + public LuceneMinFactory.NumberType getNumberType() { + return LuceneMinFactory.NumberType.LONG; + } + + @Override + protected NumberTypeTest getNumberTypeTest() { + return new NumberTypeTest() { + + long min = Long.MAX_VALUE; + + @Override + public IndexableField newPointField() { + return new LongField(FIELD_NAME, newValue(), randomFrom(Field.Store.values())); + } + + @Override + public IndexableField newDocValuesField() { + return new SortedNumericDocValuesField(FIELD_NAME, newValue()); + } + + private long newValue() { + final long value = randomLong(); + min = Math.min(min, value); + return value; + } + + @Override + public void assertPage(Page page) { + assertThat(page.getBlock(0), instanceOf(LongBlock.class)); + final LongBlock db = page.getBlock(0); + assertThat(page.getBlock(1), instanceOf(BooleanBlock.class)); + final BooleanBlock bb = page.getBlock(1); + if (bb.getBoolean(0) == false) { + assertThat(db.getLong(0), equalTo(Long.MAX_VALUE)); + } else { + assertThat(db.getLong(0), greaterThanOrEqualTo(min)); + } + } + + @Override + public AggregatorFunction newAggregatorFunction(DriverContext context) { + return new MinLongAggregatorFunctionSupplier(List.of(0, 1)).aggregator(context); + } + + @Override + public void assertMinValue(Block block, boolean exactResult) { + assertThat(block, instanceOf(LongBlock.class)); + final LongBlock lb = (LongBlock) block; + if (exactResult) { + assertThat(lb.getLong(0), equalTo(min)); + } else { + assertThat(lb.getLong(0), greaterThanOrEqualTo(min)); + } + } + }; + } +} diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinOperatorTestCase.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinOperatorTestCase.java new file mode 100644 index 0000000000000..493512bd83bec --- /dev/null +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/LuceneMinOperatorTestCase.java @@ -0,0 +1,210 @@ +/* + * 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.compute.lucene; + +import org.apache.lucene.document.Document; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.NoMergePolicy; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.index.RandomIndexWriter; +import org.elasticsearch.common.breaker.CircuitBreakingException; +import org.elasticsearch.compute.aggregation.AggregatorFunction; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.AnyOperatorTestCase; +import org.elasticsearch.compute.operator.Driver; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.OperatorTestCase; +import org.elasticsearch.compute.operator.TestResultPageSinkOperator; +import org.elasticsearch.core.IOUtils; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.indices.CrankyCircuitBreakerService; +import org.hamcrest.Matcher; +import org.junit.After; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Supplier; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.matchesRegex; + +public abstract class LuceneMinOperatorTestCase extends AnyOperatorTestCase { + + protected interface NumberTypeTest { + + IndexableField newPointField(); + + IndexableField newDocValuesField(); + + void assertPage(Page page); + + AggregatorFunction newAggregatorFunction(DriverContext context); + + void assertMinValue(Block block, boolean exactResult); + + } + + protected abstract NumberTypeTest getNumberTypeTest(); + + protected abstract LuceneMinFactory.NumberType getNumberType(); + + protected static final String FIELD_NAME = "field"; + private final Directory directory = newDirectory(); + private IndexReader reader; + + @After + public void closeIndex() throws IOException { + IOUtils.close(reader, directory); + } + + @Override + protected LuceneMinFactory simple() { + return simple(getNumberTypeTest(), randomFrom(DataPartitioning.values()), between(1, 10_000), 100); + } + + private LuceneMinFactory simple(NumberTypeTest numberTypeTest, DataPartitioning dataPartitioning, int numDocs, int limit) { + final boolean enableShortcut = randomBoolean(); + final boolean enableMultiValue = randomBoolean(); + final int commitEvery = Math.max(1, numDocs / 10); + try ( + RandomIndexWriter writer = new RandomIndexWriter( + random(), + directory, + newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE) + ) + ) { + + for (int d = 0; d < numDocs; d++) { + final var numValues = enableMultiValue ? randomIntBetween(1, 5) : 1; + final var doc = new Document(); + for (int i = 0; i < numValues; i++) { + if (enableShortcut) { + doc.add(numberTypeTest.newPointField()); + } else { + doc.add(numberTypeTest.newDocValuesField()); + } + } + writer.addDocument(doc); + if (d % commitEvery == 0) { + writer.commit(); + } + } + reader = writer.getReader(); + } catch (IOException e) { + throw new RuntimeException(e); + } + + final ShardContext ctx = new LuceneSourceOperatorTests.MockShardContext(reader, 0); + final Query query; + if (enableShortcut && randomBoolean()) { + query = new MatchAllDocsQuery(); + } else { + query = SortedNumericDocValuesField.newSlowRangeQuery(FIELD_NAME, Long.MIN_VALUE, Long.MAX_VALUE); + } + return new LuceneMinFactory(List.of(ctx), c -> query, dataPartitioning, between(1, 8), FIELD_NAME, getNumberType(), limit); + } + + public void testSimple() { + testSimple(this::driverContext); + } + + public void testSimpleWithCranky() { + try { + testSimple(this::crankyDriverContext); + logger.info("cranky didn't break"); + } catch (CircuitBreakingException e) { + logger.info("broken", e); + assertThat(e.getMessage(), equalTo(CrankyCircuitBreakerService.ERROR_MESSAGE)); + } + } + + private void testSimple(Supplier contexts) { + int size = between(1_000, 20_000); + int limit = randomBoolean() ? between(10, size) : Integer.MAX_VALUE; + testMin(contexts, size, limit); + } + + public void testEmpty() { + testEmpty(this::driverContext); + } + + public void testEmptyWithCranky() { + try { + testEmpty(this::crankyDriverContext); + logger.info("cranky didn't break"); + } catch (CircuitBreakingException e) { + logger.info("broken", e); + assertThat(e.getMessage(), equalTo(CrankyCircuitBreakerService.ERROR_MESSAGE)); + } + } + + private void testEmpty(Supplier contexts) { + int limit = randomBoolean() ? between(10, 10000) : Integer.MAX_VALUE; + testMin(contexts, 0, limit); + } + + private void testMin(Supplier contexts, int size, int limit) { + DataPartitioning dataPartitioning = randomFrom(DataPartitioning.values()); + NumberTypeTest numberTypeTest = getNumberTypeTest(); + LuceneMinFactory factory = simple(numberTypeTest, dataPartitioning, size, limit); + List results = new CopyOnWriteArrayList<>(); + List drivers = new ArrayList<>(); + int taskConcurrency = between(1, 8); + for (int i = 0; i < taskConcurrency; i++) { + DriverContext ctx = contexts.get(); + drivers.add(new Driver(ctx, factory.get(ctx), List.of(), new TestResultPageSinkOperator(results::add), () -> {})); + } + OperatorTestCase.runDriver(drivers); + assertThat(results.size(), lessThanOrEqualTo(taskConcurrency)); + + try (AggregatorFunction aggregatorFunction = numberTypeTest.newAggregatorFunction(contexts.get())) { + for (Page page : results) { + assertThat(page.getPositionCount(), is(1)); // one row + assertThat(page.getBlockCount(), is(2)); // two blocks + numberTypeTest.assertPage(page); + aggregatorFunction.addIntermediateInput(page); + } + + final Block[] result = new Block[1]; + try { + aggregatorFunction.evaluateFinal(result, 0, contexts.get()); + if (result[0].areAllValuesNull() == false) { + boolean exactResult = size <= limit; + numberTypeTest.assertMinValue(result[0], exactResult); + } + } finally { + Releasables.close(result); + } + } + } + + @Override + protected final Matcher expectedToStringOfSimple() { + return matchesRegex("LuceneMinMaxOperator\\[maxPageSize = \\d+, remainingDocs=100]"); + } + + @Override + protected final Matcher expectedDescriptionOfSimple() { + return matchesRegex( + "LuceneMinOperator\\[type = " + + getNumberType().name() + + ", dataPartitioning = (DOC|SHARD|SEGMENT), fieldName = " + + FIELD_NAME + + ", limit = 100]" + ); + } +} diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec index a53777cff7c71..a6e1a771374ca 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec @@ -663,7 +663,7 @@ required_capability: fn_bit_length FROM airports | WHERE country == "India" | KEEP city -| EVAL fn_length=LENGTH(city), fn_bit_length = BIT_LENGTH(city) +| EVAL fn_length = LENGTH(city), fn_bit_length = BIT_LENGTH(city) // end::bitLength[] | SORT city | LIMIT 3 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec index fc2350491db91..592b06107c8b5 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec @@ -379,7 +379,7 @@ required_capability: fn_byte_length FROM airports | WHERE country == "India" | KEEP city -| EVAL fn_length=LENGTH(city), fn_byte_length = BYTE_LENGTH(city) +| EVAL fn_length = LENGTH(city), fn_byte_length = BYTE_LENGTH(city) // end::byteLength[] | SORT city | LIMIT 3 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..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 @@ -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; @@ -21,12 +22,16 @@ 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.index.query.QueryBuilder; +import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.AbstractMultiClustersTestCase; import org.elasticsearch.test.InternalTestCluster; 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; @@ -35,30 +40,36 @@ 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; 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.instanceOf; 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 +101,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 +113,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 +139,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 +152,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,171 +179,695 @@ public void testSuccessfulPathways() { } } - public void testSearchesWhereMissingIndicesAreSpecified() { - Map testClusterInfo = setupTwoClusters(); + 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 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); 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"); + 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, 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))); + try { + // missing concrete local index is fatal + { + 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]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [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 not fatal when skip_unavailable=true (as long as an index matches on another cluster) + { + 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(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, 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, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0) + ) + ); + } + } - assertThat(executionInfo.clusterAliases(), equalTo(Set.of(REMOTE_CLUSTER, LOCAL_CLUSTER))); + // since there is at least one matching index in the query, the missing wildcarded local index is not an error + { + 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(); + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster( + REMOTE_CLUSTER_1, + remoteIndexName, + 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)); + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + // LIMIT 0 searches always have total shards = 0 + new ExpectedCluster(REMOTE_CLUSTER_1, remoteIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); + } + } - 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)); + // since at least one index of the query matches on some cluster, a wildcarded index on skip_un=true is not an error + { + 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(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + assertExpectedClustersForMissingIndicesTests( + executionInfo, + List.of( + new ExpectedCluster(LOCAL_CLUSTER, localIndexName, 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, localIndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 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)); - } + // 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]")); - // 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"))); + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, 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)); + // 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*]")); + + 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 - 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*]")); - 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,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 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*]")); - // 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"))); + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*,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)); + // 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]")); - 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("Unknown index [cluster-a:nomatch,nomatch]")); + } - 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)); + // 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]")); - 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)); + 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 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 = 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(); + // 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_1, "*", 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) + // )); + // } + } + + // tests with three clusters --- + + // 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 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(); + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster( + REMOTE_CLUSTER_2, + remote2IndexName, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote2NumShards + ) + ) + ); + } + + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); + } + } + + // 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 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(); + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster( + REMOTE_CLUSTER_2, + remote2IndexName, + EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, + remote2NumShards + ) + ) + ); + } + + 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, "nomatch*", EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0), + new ExpectedCluster(REMOTE_CLUSTER_1, "nomatch*", EsqlExecutionInfo.Cluster.Status.SKIPPED, 0), + new ExpectedCluster(REMOTE_CLUSTER_2, remote2IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); + } + } + } 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)) { - EsqlExecutionInfo executionInfo = resp.getExecutionInfo(); - List> values = getValuesList(resp); - assertThat(values, hasSize(1)); - assertThat(values.get(0), equalTo(List.of(45L))); + public void testSearchesAgainstNonMatchingIndicesWithSkipUnavailableFalse() { + 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"); - assertNotNull(executionInfo); - assertThat(executionInfo.isCrossClusterSearch(), is(true)); - long overallTookMillis = executionInfo.overallTook().millis(); - assertThat(overallTookMillis, greaterThanOrEqualTo(0L)); - assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + createIndexAliases(numClusters); + setSkipUnavailable(REMOTE_CLUSTER_1, false); + setSkipUnavailable(REMOTE_CLUSTER_2, false); - 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_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.getSuccessfulShards(), equalTo(0)); - assertThat(remoteCluster.getSkippedShards(), equalTo(0)); - assertThat(remoteCluster.getFailedShards(), equalTo(0)); + 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]")); + } - 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 is fatal when skip_unavailable=false + { + 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 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(); + assertThat(executionInfo.isCrossClusterSearch(), is(true)); + assertThat(executionInfo.includeCCSMetadata(), equalTo(responseExpectMeta)); + 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, + remote1IndexName, + 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)); + 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 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, remote1IndexName, EsqlExecutionInfo.Cluster.Status.SUCCESSFUL, 0) + ) + ); + } + } + + // query is fatal since cluster-a has skip_unavailable=false and has no matching indices + { + 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*]")); + + 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 + { + 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 - 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*]")); + + 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]")); + } + + // Missing concrete index on skip_unavailable=false cluster is a fatal error, even when another index expression + // against that cluster matches + { + 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]")); + + // 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 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]")); + + 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 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*]")); + + String limit0 = q + " | LIMIT 0"; + e = expectThrows(VerificationException.class, () -> runQuery(limit0, requestIncludeMeta)); + assertThat(e.getDetailedMessage(), containsString("Unknown index [cluster-a:nomatch*]")); + } + } finally { + clearSkipUnavailable(); + } + } + + record ExpectedCluster(String clusterAlias, String indexExpression, EsqlExecutionInfo.Cluster.Status status, Integer totalShards) {} + + public void assertExpectedClustersForMissingIndicesTests(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())); + assertThat(msg, cluster.getFailures().size(), equalTo(1)); + assertThat(msg, cluster.getFailures().get(0).getCause(), instanceOf(VerificationException.class)); + String expectedMsg = "Unknown index [" + expectedCluster.indexExpression() + "]"; + assertThat(msg, cluster.getFailures().get(0).getCause().getMessage(), containsString(expectedMsg)); + } + // currently failed shards is always zero - change this once we start allowing partial data for individual shard failures + assertThat(msg, cluster.getFailedShards(), equalTo(0)); } } @@ -359,6 +894,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( @@ -376,9 +915,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)); @@ -395,6 +934,8 @@ public void testSearchesWhereNonExistentClusterIsSpecifiedWithWildcards() { assertThat(localCluster.getSuccessfulShards(), equalTo(localNumShards)); assertThat(localCluster.getSkippedShards(), equalTo(0)); assertThat(localCluster.getFailedShards(), equalTo(0)); + } finally { + clearSkipUnavailable(); } } @@ -403,10 +944,12 @@ 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() { setupTwoClusters(); - Tuple includeCCSMetadata = randomIncludeCCSMetadata(); Boolean requestIncludeMeta = includeCCSMetadata.v1(); boolean responseExpectMeta = includeCCSMetadata.v2(); @@ -427,9 +970,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)); @@ -449,66 +992,6 @@ public void testCCSExecutionOnSearchesWithLimit0() { assertThat(remoteCluster.getSkippedShards(), equalTo(0)); 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)); - } } public void testMetadataIndex() { @@ -536,7 +1019,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 +1054,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 +1076,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 +1104,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 +1137,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 +1187,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,22 +1275,37 @@ void waitForNoInitializingShards(Client client, TimeValue timeout, String... ind } Map setupTwoClusters() { - String localIndex = "logs-1"; + 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; int numShardsLocal = randomIntBetween(1, 5); - populateLocalIndices(localIndex, numShardsLocal); + populateLocalIndices(LOCAL_INDEX, numShardsLocal); - String remoteIndex = "logs-2"; int numShardsRemote = randomIntBetween(1, 5); - populateRemoteIndices(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, REMOTE_INDEX, numShardsRemote2); + clusterInfo.put("remote2.index", REMOTE_INDEX); + clusterInfo.put("remote2.num_shards", numShardsRemote2); + } - String skipUnavailableKey = Strings.format("cluster.remote.%s.skip_unavailable", REMOTE_CLUSTER); - Setting skipUnavailableSetting = cluster(REMOTE_CLUSTER).clusterService().getClusterSettings().get(skipUnavailableKey); + 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 +1314,98 @@ Map setupTwoClusters() { 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()); + } + } + + 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( @@ -831,8 +1421,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 +1435,23 @@ 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/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java index 632f52d163349..d399c826e0bf2 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.Expressions; import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; +import org.elasticsearch.xpack.esql.core.expression.NameId; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; import org.elasticsearch.xpack.esql.core.expression.TypeResolutions; import org.elasticsearch.xpack.esql.core.expression.function.Function; @@ -33,6 +34,7 @@ import org.elasticsearch.xpack.esql.expression.function.fulltext.FullTextFunction; import org.elasticsearch.xpack.esql.expression.function.fulltext.Match; import org.elasticsearch.xpack.esql.expression.function.fulltext.QueryString; +import org.elasticsearch.xpack.esql.expression.function.grouping.Categorize; import org.elasticsearch.xpack.esql.expression.function.grouping.GroupingFunction; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Neg; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals; @@ -56,10 +58,12 @@ import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -271,6 +275,7 @@ private static void checkAggregate(LogicalPlan p, Set failures) { r -> failures.add(fail(r, "the rate aggregate[{}] can only be used within the metrics command", r.sourceText())) ); } + checkCategorizeGrouping(agg, failures); } else { p.forEachExpression( GroupingFunction.class, @@ -279,6 +284,74 @@ private static void checkAggregate(LogicalPlan p, Set failures) { } } + /** + * Check CATEGORIZE grouping function usages. + *

+ * Some of those checks are temporary, until the required syntax or engine changes are implemented. + *

+ */ + private static void checkCategorizeGrouping(Aggregate agg, Set failures) { + // Forbid CATEGORIZE grouping function with other groupings + if (agg.groupings().size() > 1) { + agg.groupings().forEach(g -> { + g.forEachDown( + Categorize.class, + categorize -> failures.add( + fail(categorize, "cannot use CATEGORIZE grouping function [{}] with multiple groupings", categorize.sourceText()) + ) + ); + }); + } + + // Forbid CATEGORIZE grouping functions not being top level groupings + agg.groupings().forEach(g -> { + // Check all CATEGORIZE but the top level one + Alias.unwrap(g) + .children() + .forEach( + child -> child.forEachDown( + Categorize.class, + c -> failures.add( + fail(c, "CATEGORIZE grouping function [{}] can't be used within other expressions", c.sourceText()) + ) + ) + ); + }); + + // Forbid CATEGORIZE being used in the aggregations + agg.aggregates().forEach(a -> { + a.forEachDown( + Categorize.class, + categorize -> failures.add( + fail(categorize, "cannot use CATEGORIZE grouping function [{}] within the aggregations", categorize.sourceText()) + ) + ); + }); + + // Forbid CATEGORIZE being referenced in the aggregation functions + Map categorizeByAliasId = new HashMap<>(); + agg.groupings().forEach(g -> { + g.forEachDown(Alias.class, alias -> { + if (alias.child() instanceof Categorize categorize) { + categorizeByAliasId.put(alias.id(), categorize); + } + }); + }); + agg.aggregates() + .forEach(a -> a.forEachDown(AggregateFunction.class, aggregate -> aggregate.forEachDown(Attribute.class, attribute -> { + var categorize = categorizeByAliasId.get(attribute.id()); + if (categorize != null) { + failures.add( + fail( + attribute, + "cannot reference CATEGORIZE grouping function [{}] within the aggregations", + attribute.sourceText() + ) + ); + } + }))); + } + private static void checkRateAggregates(Expression expr, int nestedLevel, Set failures) { if (expr instanceof AggregateFunction) { nestedLevel++; @@ -688,7 +761,7 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Set f plan, condition, Match.class, - lp -> (lp instanceof Limit == false), + lp -> (lp instanceof Limit == false) && (lp instanceof Aggregate == false), m -> "[" + m.functionName() + "] " + m.functionType(), failures ); 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..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 @@ -12,22 +12,37 @@ 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) { + /** + * @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, + 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); } + /** + * 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, Collections.emptyMap()); + return valid(index, index.concreteIndices(), 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 +54,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 +87,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; @@ -75,10 +98,17 @@ 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 resolvedIndices() { + return resolvedIndices; + } + @Override public boolean equals(Object obj) { if (obj == null || obj.getClass() != getClass()) { @@ -87,16 +117,29 @@ 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 public String toString() { - return invalid != null ? invalid : index.name(); + return invalid != null + ? invalid + : "IndexResolution{" + + "index=" + + index + + ", invalid='" + + invalid + + '\'' + + ", resolvedIndices=" + + resolvedIndices + + ", unavailableClusters=" + + unavailableClusters + + '}'; } } 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 ffad379001ed0..76de337ded5c6 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/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 504689fdac39b..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 @@ -309,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 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..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 @@ -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) { @@ -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. */ @@ -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,39 +180,91 @@ static void updateExecutionInfoWithUnavailableClusters(EsqlExecutionInfo execInf } } - // visible for testing 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.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 + * 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; /* * 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) { - // 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() - ); + 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 { + // 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); + if (failure != null) { + builder.setFailures(List.of(failure)); + } + return builder.build(); + }); + } } + if (fatalErrorMessage != null) { + throw new VerificationException(fatalErrorMessage); + } + } + + // visible for testing + static boolean missingIndicesIsFatal(String clusterAlias, EsqlExecutionInfo executionInfo) { + // 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 executionInfo.getCluster(clusterAlias).isSkipUnavailable() == false; + } + + private static boolean concreteIndexRequested(String indexExpression) { + for (String expr : indexExpression.split(",")) { + if (expr.charAt(0) == '<' || expr.startsWith("-<")) { + // skip date math expressions + continue; + } + if (expr.indexOf('*') < 0) { + return true; + } + } + return false; } // visible for testing 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/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index d9225d266c213..8b364a603405c 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 @@ -1183,6 +1183,10 @@ public void testMatchFunctionNotAllowedAfterCommands() throws Exception { "1:24: [MATCH] function cannot be used after LIMIT", error("from test | limit 10 | where match(first_name, \"Anna\")") ); + assertEquals( + "1:47: [MATCH] function cannot be used after STATS", + error("from test | STATS c = AVG(salary) BY gender | where match(gender, \"F\")") + ); } public void testMatchFunctionAndOperatorHaveCorrectErrorMessages() throws Exception { @@ -1733,6 +1737,68 @@ public void testIntervalAsString() { ); } + public void testCategorizeSingleGrouping() { + query("from test | STATS COUNT(*) BY CATEGORIZE(first_name)"); + query("from test | STATS COUNT(*) BY cat = CATEGORIZE(first_name)"); + + assertEquals( + "1:31: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] with multiple groupings", + error("from test | STATS COUNT(*) BY CATEGORIZE(first_name), emp_no") + ); + assertEquals( + "1:39: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] with multiple groupings", + error("FROM test | STATS COUNT(*) BY emp_no, CATEGORIZE(first_name)") + ); + assertEquals( + "1:35: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] with multiple groupings", + error("FROM test | STATS COUNT(*) BY a = CATEGORIZE(first_name), b = emp_no") + ); + assertEquals( + "1:31: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] with multiple groupings\n" + + "line 1:55: cannot use CATEGORIZE grouping function [CATEGORIZE(last_name)] with multiple groupings", + error("FROM test | STATS COUNT(*) BY CATEGORIZE(first_name), CATEGORIZE(last_name)") + ); + assertEquals( + "1:31: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] with multiple groupings", + error("FROM test | STATS COUNT(*) BY CATEGORIZE(first_name), CATEGORIZE(first_name)") + ); + } + + public void testCategorizeNestedGrouping() { + query("from test | STATS COUNT(*) BY CATEGORIZE(LENGTH(first_name)::string)"); + + assertEquals( + "1:40: CATEGORIZE grouping function [CATEGORIZE(first_name)] can't be used within other expressions", + error("FROM test | STATS COUNT(*) BY MV_COUNT(CATEGORIZE(first_name))") + ); + assertEquals( + "1:31: CATEGORIZE grouping function [CATEGORIZE(first_name)] can't be used within other expressions", + error("FROM test | STATS COUNT(*) BY CATEGORIZE(first_name)::datetime") + ); + } + + public void testCategorizeWithinAggregations() { + query("from test | STATS MV_COUNT(cat), COUNT(*) BY cat = CATEGORIZE(first_name)"); + + assertEquals( + "1:25: cannot use CATEGORIZE grouping function [CATEGORIZE(first_name)] within the aggregations", + error("FROM test | STATS COUNT(CATEGORIZE(first_name)) BY CATEGORIZE(first_name)") + ); + + assertEquals( + "1:25: cannot reference CATEGORIZE grouping function [cat] within the aggregations", + error("FROM test | STATS COUNT(cat) BY cat = CATEGORIZE(first_name)") + ); + assertEquals( + "1:30: cannot reference CATEGORIZE grouping function [cat] within the aggregations", + error("FROM test | STATS SUM(LENGTH(cat::keyword) + LENGTH(last_name)) BY cat = CATEGORIZE(first_name)") + ); + assertEquals( + "1:25: cannot reference CATEGORIZE grouping function [`CATEGORIZE(first_name)`] within the aggregations", + error("FROM test | STATS COUNT(`CATEGORIZE(first_name)`) BY CATEGORIZE(first_name)") + ); + } + private void query(String query) { defaultAnalyzer.analyze(parser.createStatement(query)); } 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..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 @@ -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; @@ -228,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); @@ -266,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); @@ -293,7 +295,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*", @@ -302,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); @@ -341,8 +343,12 @@ 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); + IndexResolution indexResolution = IndexResolution.valid(esIndex, esIndex.concreteIndices(), Map.of(remote1Alias, failure)); + VerificationException ve = expectThrows( + VerificationException.class, + () -> EsqlSessionCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, indexResolution) + ); + assertThat(ve.getDetailedMessage(), containsString("Unknown index [remote2:mylogs1,mylogs2,logs*]")); } } @@ -579,4 +585,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)); + } + + } } 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; + } +} 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") diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index d11cb7521976a..a979d614fe38f 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -26,7 +26,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -56,9 +58,72 @@ public class EmailService extends NotificationService { (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope) ); + private static final List ALLOW_ALL_DEFAULT = List.of("*"); + private static final Setting> SETTING_DOMAIN_ALLOWLIST = Setting.stringListSetting( "xpack.notification.email.account.domain_allowlist", - List.of("*"), + ALLOW_ALL_DEFAULT, + new Setting.Validator<>() { + @Override + public void validate(List value) { + // Ignored + } + + @Override + @SuppressWarnings("unchecked") + public void validate(List value, Map, Object> settings) { + List recipientAllowPatterns = (List) settings.get(SETTING_RECIPIENT_ALLOW_PATTERNS); + if (value.equals(ALLOW_ALL_DEFAULT) == false && recipientAllowPatterns.equals(ALLOW_ALL_DEFAULT) == false) { + throw new IllegalArgumentException( + "Cannot set both [" + + SETTING_RECIPIENT_ALLOW_PATTERNS.getKey() + + "] and [" + + SETTING_DOMAIN_ALLOWLIST.getKey() + + "] to a non [\"*\"] value at the same time." + ); + } + } + + @Override + public Iterator> settings() { + List> settingRecipientAllowPatterns = List.of(SETTING_RECIPIENT_ALLOW_PATTERNS); + return settingRecipientAllowPatterns.iterator(); + } + }, + Property.Dynamic, + Property.NodeScope + ); + + private static final Setting> SETTING_RECIPIENT_ALLOW_PATTERNS = Setting.stringListSetting( + "xpack.notification.email.recipient_allowlist", + ALLOW_ALL_DEFAULT, + new Setting.Validator<>() { + @Override + public void validate(List value) { + // Ignored + } + + @Override + @SuppressWarnings("unchecked") + public void validate(List value, Map, Object> settings) { + List domainAllowList = (List) settings.get(SETTING_DOMAIN_ALLOWLIST); + if (value.equals(ALLOW_ALL_DEFAULT) == false && domainAllowList.equals(ALLOW_ALL_DEFAULT) == false) { + throw new IllegalArgumentException( + "Connect set both [" + + SETTING_RECIPIENT_ALLOW_PATTERNS.getKey() + + "] and [" + + SETTING_DOMAIN_ALLOWLIST.getKey() + + "] to a non [\"*\"] value at the same time." + ); + } + } + + @Override + public Iterator> settings() { + List> settingDomainAllowlist = List.of(SETTING_DOMAIN_ALLOWLIST); + return settingDomainAllowlist.iterator(); + } + }, Property.Dynamic, Property.NodeScope ); @@ -167,6 +232,7 @@ public class EmailService extends NotificationService { private final CryptoService cryptoService; private final SSLService sslService; private volatile Set allowedDomains; + private volatile Set allowedRecipientPatterns; @SuppressWarnings("this-escape") public EmailService(Settings settings, @Nullable CryptoService cryptoService, SSLService sslService, ClusterSettings clusterSettings) { @@ -192,7 +258,9 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, SS clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_SEND_PARTIAL, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WAIT_ON_QUIT, (s, o) -> {}, (s, o) -> {}); this.allowedDomains = new HashSet<>(SETTING_DOMAIN_ALLOWLIST.get(settings)); + this.allowedRecipientPatterns = new HashSet<>(SETTING_RECIPIENT_ALLOW_PATTERNS.get(settings)); clusterSettings.addSettingsUpdateConsumer(SETTING_DOMAIN_ALLOWLIST, this::updateAllowedDomains); + clusterSettings.addSettingsUpdateConsumer(SETTING_RECIPIENT_ALLOW_PATTERNS, this::updateAllowedRecipientPatterns); // do an initial load reload(settings); } @@ -201,6 +269,10 @@ void updateAllowedDomains(List newDomains) { this.allowedDomains = new HashSet<>(newDomains); } + void updateAllowedRecipientPatterns(List newPatterns) { + this.allowedRecipientPatterns = new HashSet<>(newPatterns); + } + @Override protected Account createAccount(String name, Settings accountSettings) { Account.Config config = new Account.Config(name, accountSettings, getSmtpSslSocketFactory(), logger); @@ -228,33 +300,47 @@ public EmailSent send(Email email, Authentication auth, Profile profile, String "failed to send email with subject [" + email.subject() + "] and recipient domains " - + getRecipientDomains(email) + + getRecipients(email, true) + ", one or more recipients is not specified in the domain allow list setting [" + SETTING_DOMAIN_ALLOWLIST.getKey() + "]." ); } + if (recipientAddressInAllowList(email, this.allowedRecipientPatterns) == false) { + throw new IllegalArgumentException( + "failed to send email with subject [" + + email.subject() + + "] and recipients " + + getRecipients(email, false) + + ", one or more recipients is not specified in the domain allow list setting [" + + SETTING_RECIPIENT_ALLOW_PATTERNS.getKey() + + "]." + ); + } return send(email, auth, profile, account); } // Visible for testing - static Set getRecipientDomains(Email email) { - return Stream.concat( + static Set getRecipients(Email email, boolean domainsOnly) { + var stream = Stream.concat( Optional.ofNullable(email.to()).map(addrs -> Arrays.stream(addrs.toArray())).orElse(Stream.empty()), Stream.concat( Optional.ofNullable(email.cc()).map(addrs -> Arrays.stream(addrs.toArray())).orElse(Stream.empty()), Optional.ofNullable(email.bcc()).map(addrs -> Arrays.stream(addrs.toArray())).orElse(Stream.empty()) ) - ) - .map(InternetAddress::getAddress) - // Pull out only the domain of the email address, so foo@bar.com -> bar.com - .map(emailAddress -> emailAddress.substring(emailAddress.lastIndexOf('@') + 1)) - .collect(Collectors.toSet()); + ).map(InternetAddress::getAddress); + + if (domainsOnly) { + // Pull out only the domain of the email address, so foo@bar.com becomes bar.com + stream = stream.map(emailAddress -> emailAddress.substring(emailAddress.lastIndexOf('@') + 1)); + } + + return stream.collect(Collectors.toSet()); } // Visible for testing static boolean recipientDomainsInAllowList(Email email, Set allowedDomainSet) { - if (allowedDomainSet.size() == 0) { + if (allowedDomainSet.isEmpty()) { // Nothing is allowed return false; } @@ -262,12 +348,29 @@ static boolean recipientDomainsInAllowList(Email email, Set allowedDomai // Don't bother checking, because there is a wildcard all return true; } - final Set domains = getRecipientDomains(email); + final Set domains = getRecipients(email, true); final Predicate matchesAnyAllowedDomain = domain -> allowedDomainSet.stream() .anyMatch(allowedDomain -> Regex.simpleMatch(allowedDomain, domain, true)); return domains.stream().allMatch(matchesAnyAllowedDomain); } + // Visible for testing + static boolean recipientAddressInAllowList(Email email, Set allowedRecipientPatterns) { + if (allowedRecipientPatterns.isEmpty()) { + // Nothing is allowed + return false; + } + if (allowedRecipientPatterns.contains("*")) { + // Don't bother checking, because there is a wildcard all + return true; + } + + final Set recipients = getRecipients(email, false); + final Predicate matchesAnyAllowedRecipient = recipient -> allowedRecipientPatterns.stream() + .anyMatch(pattern -> Regex.simpleMatch(pattern, recipient, true)); + return recipients.stream().allMatch(matchesAnyAllowedRecipient); + } + private static EmailSent send(Email email, Authentication auth, Profile profile, Account account) throws MessagingException { assert account != null; try { @@ -304,6 +407,7 @@ private static List> getDynamicSettings() { return Arrays.asList( SETTING_DEFAULT_ACCOUNT, SETTING_DOMAIN_ALLOWLIST, + SETTING_RECIPIENT_ALLOW_PATTERNS, SETTING_PROFILE, SETTING_EMAIL_DEFAULTS, SETTING_SMTP_AUTH, diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java index a0ce8b18d8a96..4a668d0f9817a 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java @@ -69,6 +69,31 @@ public void testSend() throws Exception { assertThat(sent.account(), is("account1")); } + public void testDomainAndRecipientAllowCantBeSetAtSameTime() { + Settings settings = Settings.builder() + .putList("xpack.notification.email.account.domain_allowlist", "bar.com") + .putList("xpack.notification.email.recipient_allowlist", "*-user@potato.com") + .build(); + + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new EmailService( + settings, + null, + mock(SSLService.class), + new ClusterSettings(Settings.EMPTY, new HashSet<>(EmailService.getSettings())) + ) + ); + + assertThat( + e.getMessage(), + containsString( + "Cannot set both [xpack.notification.email.recipient_allowlist] and " + + "[xpack.notification.email.account.domain_allowlist] to a non [\"*\"] value at the same time." + ) + ); + } + public void testAccountSmtpPropertyConfiguration() { Settings settings = Settings.builder() .put("xpack.notification.email.account.account1.smtp.host", "localhost") @@ -140,7 +165,7 @@ public void testExtractDomains() throws Exception { Collections.emptyMap() ); assertThat( - EmailService.getRecipientDomains(email), + EmailService.getRecipients(email, true), containsInAnyOrder("bar.com", "eggplant.com", "example.com", "another.com", "bcc.com") ); @@ -158,7 +183,7 @@ public void testExtractDomains() throws Exception { "htmlbody", Collections.emptyMap() ); - assertThat(EmailService.getRecipientDomains(email), containsInAnyOrder("bar.com", "eggplant.com", "example.com")); + assertThat(EmailService.getRecipients(email, true), containsInAnyOrder("bar.com", "eggplant.com", "example.com")); } public void testAllowedDomain() throws Exception { @@ -322,6 +347,264 @@ public void testChangeDomainAllowListSetting() throws UnsupportedEncodingExcepti assertThat(e2.getMessage(), containsString("port out of range")); } + public void testRecipientAddressInAllowList_EmptyAllowedPatterns() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of(); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(false)); + } + + public void testRecipientAddressInAllowList_WildcardPattern() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("*"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(true)); + } + + public void testRecipientAddressInAllowList_SpecificPattern() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("foo@bar.com"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(false)); + } + + public void testRecipientAddressInAllowList_MultiplePatterns() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("foo@bar.com", "baz@potato.com"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(true)); + } + + public void testRecipientAddressInAllowList_MixedCasePatterns() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("FOO@BAR.COM", "BAZ@POTATO.COM"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(true)); + } + + public void testRecipientAddressInAllowList_PartialWildcardPrefixPattern() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("foo@*", "baz@*"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(true)); + } + + public void testRecipientAddressInAllowList_PartialWildcardSuffixPattern() throws UnsupportedEncodingException { + Email email = createTestEmail("foo@bar.com", "baz@potato.com"); + Set allowedPatterns = Set.of("*@bar.com", "*@potato.com"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(true)); + } + + public void testRecipientAddressInAllowList_DisallowedCCAddressesFails() throws UnsupportedEncodingException { + Email email = new Email( + "id", + new Email.Address("sender@domain.com", "Sender"), + createAddressList("foo@bar.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + createAddressList("cc@allowed.com", "cc@notallowed.com"), + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + Set allowedPatterns = Set.of("foo@bar.com", "cc@allowed.com"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(false)); + } + + public void testRecipientAddressInAllowList_DisallowedBCCAddressesFails() throws UnsupportedEncodingException { + Email email = new Email( + "id", + new Email.Address("sender@domain.com", "Sender"), + createAddressList("foo@bar.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + null, + createAddressList("bcc@allowed.com", "bcc@notallowed.com"), + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + Set allowedPatterns = Set.of("foo@bar.com", "bcc@allowed.com"); + assertThat(EmailService.recipientAddressInAllowList(email, allowedPatterns), is(false)); + } + + public void testAllowedRecipient() throws Exception { + Email email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + null, + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("*"))); + assertFalse(EmailService.recipientAddressInAllowList(email, Set.of())); + assertFalse(EmailService.recipientAddressInAllowList(email, Set.of(""))); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("foo@other.com", "*o@bar.com"))); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("buzz@other.com", "*.com"))); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("*.CoM"))); + + // Invalid email in CC doesn't blow up + email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + createAddressList("badEmail"), + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + assertFalse(EmailService.recipientAddressInAllowList(email, Set.of("*@other.com", "*iii@bar.com"))); + + // Check CC + email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + createAddressList("thing@other.com"), + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("*@other.com", "*@bar.com"))); + assertFalse(EmailService.recipientAddressInAllowList(email, Set.of("*oo@bar.com"))); + + // Check BCC + email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com"), + null, + createAddressList("thing@other.com"), + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + assertTrue(EmailService.recipientAddressInAllowList(email, Set.of("*@other.com", "*@bar.com"))); + assertFalse(EmailService.recipientAddressInAllowList(email, Set.of("*oo@bar.com"))); + } + + public void testSendEmailWithRecipientNotInAllowList() throws Exception { + service.updateAllowedRecipientPatterns(Collections.singletonList(randomFrom("*@bar.*", "*@bar.com", "*b*"))); + Email email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com", "non-whitelisted@invalid.com"), + null, + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + when(account.name()).thenReturn("account1"); + Authentication auth = new Authentication("user", new Secret("passwd".toCharArray())); + Profile profile = randomFrom(Profile.values()); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> service.send(email, auth, profile, "account1")); + assertThat( + e.getMessage(), + containsString( + "failed to send email with subject [subject] and recipients [non-whitelisted@invalid.com, foo@bar.com], " + + "one or more recipients is not specified in the domain allow list setting " + + "[xpack.notification.email.recipient_allowlist]." + ) + ); + } + + public void testChangeRecipientAllowListSetting() throws UnsupportedEncodingException, MessagingException { + Settings settings = Settings.builder() + .put("xpack.notification.email.account.account1.foo", "bar") + // Setting a random SMTP server name and an invalid port so that sending emails is guaranteed to fail: + .put("xpack.notification.email.account.account1.smtp.host", randomAlphaOfLength(10)) + .put("xpack.notification.email.account.account1.smtp.port", -100) + .putList("xpack.notification.email.recipient_allowlist", "*oo@bar.com") + .build(); + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, new HashSet<>(EmailService.getSettings())); + EmailService emailService = new EmailService(settings, null, mock(SSLService.class), clusterSettings); + Email email = new Email( + "id", + new Email.Address("foo@bar.com", "Mr. Foo Man"), + createAddressList("foo@bar.com", "baz@potato.com"), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList("foo@bar.com", "non-whitelisted@invalid.com"), + null, + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + when(account.name()).thenReturn("account1"); + Authentication auth = new Authentication("user", new Secret("passwd".toCharArray())); + Profile profile = randomFrom(Profile.values()); + + // This send will fail because one of the recipients ("non-whitelisted@invalid.com") is in a domain that is not in the allowed list + IllegalArgumentException e1 = expectThrows( + IllegalArgumentException.class, + () -> emailService.send(email, auth, profile, "account1") + ); + assertThat( + e1.getMessage(), + containsString( + "failed to send email with subject [subject] and recipients [non-whitelisted@invalid.com, foo@bar.com], " + + "one or more recipients is not specified in the domain allow list setting " + + "[xpack.notification.email.recipient_allowlist]." + ) + ); + + // Now dynamically add "invalid.com" to the list of allowed domains: + Settings newSettings = Settings.builder() + .putList("xpack.notification.email.recipient_allowlist", "*@bar.com", "*@invalid.com") + .build(); + clusterSettings.applySettings(newSettings); + // Still expect an exception because we're not actually sending the email, but it's no longer because the domain isn't allowed: + IllegalArgumentException e2 = expectThrows( + IllegalArgumentException.class, + () -> emailService.send(email, auth, profile, "account1") + ); + assertThat(e2.getMessage(), containsString("port out of range")); + } + + private Email createTestEmail(String... recipients) throws UnsupportedEncodingException { + return new Email( + "id", + new Email.Address("sender@domain.com", "Sender"), + createAddressList(recipients), + randomFrom(Email.Priority.values()), + ZonedDateTime.now(), + createAddressList(recipients), + null, + null, + "subject", + "body", + "htmlbody", + Collections.emptyMap() + ); + } + private static Email.AddressList createAddressList(String... emails) throws UnsupportedEncodingException { List addresses = new ArrayList<>(); for (String email : emails) { diff --git a/x-pack/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/xpack/restart/WatcherMappingUpdateIT.java b/x-pack/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/xpack/restart/WatcherMappingUpdateIT.java deleted file mode 100644 index fee6910fcf6c0..0000000000000 --- a/x-pack/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/xpack/restart/WatcherMappingUpdateIT.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * 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.restart; - -import com.carrotsearch.randomizedtesting.annotations.Name; - -import org.apache.http.util.EntityUtils; -import org.elasticsearch.Build; -import org.elasticsearch.client.Request; -import org.elasticsearch.client.RequestOptions; -import org.elasticsearch.client.Response; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.core.UpdateForV9; -import org.elasticsearch.test.rest.RestTestLegacyFeatures; -import org.elasticsearch.upgrades.FullClusterRestartUpgradeStatus; -import org.junit.Before; - -import java.nio.charset.StandardCharsets; -import java.util.Base64; -import java.util.concurrent.TimeUnit; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; - -@UpdateForV9(owner = UpdateForV9.Owner.DATA_MANAGEMENT) -// Remove the whole test suite (superseded by SystemIndexMappingUpdateServiceIT#testSystemIndexManagerUpgradesMappings) -public class WatcherMappingUpdateIT extends AbstractXpackFullClusterRestartTestCase { - - public WatcherMappingUpdateIT(@Name("cluster") FullClusterRestartUpgradeStatus upgradeStatus) { - super(upgradeStatus); - } - - @Before - public void setup() { - // This test is superseded by SystemIndexMappingUpdateServiceIT#testSystemIndexManagerUpgradesMappings for newer versions - assumeFalse( - "Starting from 8.11, the mappings upgrade service uses mappings versions instead of node versions", - clusterHasFeature(RestTestLegacyFeatures.MAPPINGS_UPGRADE_SERVICE_USES_MAPPINGS_VERSION) - ); - } - - @Override - protected Settings restClientSettings() { - String token = "Basic " + Base64.getEncoder().encodeToString("test_user:x-pack-test-password".getBytes(StandardCharsets.UTF_8)); - return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).build(); - } - - public void testMappingsAreUpdated() throws Exception { - if (isRunningAgainstOldCluster()) { - // post a watch - Request putWatchRequest = new Request("PUT", "_watcher/watch/log_error_watch"); - putWatchRequest.setJsonEntity(""" - { - "trigger" : { - "schedule" : { "interval" : "10s" } - }, - "input" : { - "search" : { - "request" : { - "indices" : [ "logs" ], - "body" : { - "query" : { - "match" : { "message": "error" } - } - } - } - } - } - } - """); - client().performRequest(putWatchRequest); - - assertMappingVersion(".watches", getOldClusterVersion()); - } else { - assertMappingVersion(".watches", Build.current().version()); - } - } - - private void assertMappingVersion(String index, String clusterVersion) throws Exception { - assertBusy(() -> { - Request mappingRequest = new Request("GET", index + "/_mappings"); - mappingRequest.setOptions(getWarningHandlerOptions(index)); - Response response = client().performRequest(mappingRequest); - String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - assertThat(responseBody, containsString("\"version\":\"" + clusterVersion + "\"")); - }, 60L, TimeUnit.SECONDS); - } - - private void assertNoMappingVersion(String index) throws Exception { - assertBusy(() -> { - Request mappingRequest = new Request("GET", index + "/_mappings"); - assert isRunningAgainstOldCluster(); - mappingRequest.setOptions(getWarningHandlerOptions(index)); - Response response = client().performRequest(mappingRequest); - String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - assertThat(responseBody, not(containsString("\"version\":\""))); - }, 60L, TimeUnit.SECONDS); - } - - private RequestOptions.Builder getWarningHandlerOptions(String index) { - return RequestOptions.DEFAULT.toBuilder() - .setWarningsHandler(w -> w.size() > 0 && w.contains(getWatcherSystemIndexWarning(index)) == false); - } - - private String getWatcherSystemIndexWarning(String index) { - return "this request accesses system indices: [" - + index - + "], but in a future major version, " - + "direct access to system indices will be prevented by default"; - } -}