From da50207faa8a2e8819a184d2e22529d5ceea49ca Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 10 May 2024 08:47:40 +0100 Subject: [PATCH 01/11] Handle must_not clauses when disabling the weight matches highlighting mode (#108453) This change makes sure we check all queries, even the must_not ones, to decide if we should disable weight matches highlighting or not. Closes #101667 Closes #106693 --- .../test/search.highlight/10_unified.yml | 115 ++++++++++++------ .../uhighlight/CustomUnifiedHighlighter.java | 3 +- 2 files changed, 82 insertions(+), 36 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/10_unified.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/10_unified.yml index 3ae8f8b09aa4a..ca1d22e4a1ce7 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/10_unified.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.highlight/10_unified.yml @@ -14,12 +14,26 @@ setup: "postings": "type": "text" "index_options": "offsets" + "nested": + "type": "nested" + "properties": + "text": + "type": "text" + "vectors": + "type": "dense_vector" + "dims": 2 + "index": true + "similarity": "l2_norm" + - do: index: index: test id: "1" body: "text" : "The quick brown fox is brown." + "nested": + "text": "The quick brown fox is brown." + "vectors": [1, 2] - do: indices.refresh: {} @@ -43,6 +57,7 @@ teardown: "query" : { "multi_match" : { "query" : "quick brown fox", "fields" : [ "text*"] } }, "highlight" : { "type" : "unified", "fields" : { "*" : {} } } } + - length: { hits.hits.0.highlight: 3 } - match: {hits.hits.0.highlight.text.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown."} @@ -58,6 +73,7 @@ teardown: "query" : { "combined_fields" : { "query" : "quick brown fox", "fields" : [ "text*"] } }, "highlight" : { "type" : "unified", "fields" : { "*" : {} } } } + - length: { hits.hits.0.highlight: 3 } - match: {hits.hits.0.highlight.text.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown."} @@ -72,11 +88,13 @@ teardown: search: body: { "query": { "multi_match": { "query": "quick brown fox", "type": "phrase", "fields": [ "text*" ] } }, - "highlight": { "type": "unified", "fields": { "*": { } } } } + "highlight": { "type": "unified", "fields": { "*": { } } } + } - - match: { hits.hits.0.highlight.text.0: "The quick brown fox is brown." } - - match: { hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown." } - - match: { hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown." } + - length: { hits.hits.0.highlight: 3 } + - match: { hits.hits.0.highlight.text.0: "The quick brown fox is brown." } + - match: { hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown." } + - match: { hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown." } - do: indices.put_settings: @@ -90,6 +108,7 @@ teardown: "query" : { "multi_match" : { "query" : "quick brown fox", "type": "phrase", "fields" : [ "text*"] } }, "highlight" : { "type" : "unified", "fields" : { "*" : {} } } } + - length: { hits.hits.0.highlight: 3 } - match: {hits.hits.0.highlight.text.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown."} - match: {hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown."} @@ -100,43 +119,69 @@ teardown: reason: 'kNN was not correctly skipped until 8.12' - do: - indices.create: - index: test-highlighting-knn - body: - mappings: - "properties": - "vectors": - "type": "dense_vector" - "dims": 2 - "index": true - "similarity": "l2_norm" - "text": - "type": "text" - "fields": - "fvh": - "type": "text" - "term_vector": "with_positions_offsets" - "postings": - "type": "text" - "index_options": "offsets" - - do: - index: - index: test-highlighting-knn - id: "1" - body: - "text" : "The quick brown fox is brown." - "vectors": [1, 2] + search: + index: test + body: { + "query": { "multi_match": { "query": "quick brown fox", "type": "phrase", "fields": [ "text*" ] } }, + "highlight": { "type": "unified", "fields": { "text*": { } } }, + "knn": { "field": "vectors", "query_vector": [1, 2], "k": 10, "num_candidates": 10 } } + + - length: { hits.hits.0.highlight: 3 } + - match: { hits.hits.0.highlight.text.0: "The quick brown fox is brown." } + - match: { hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown." } + - match: { hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown." } + +--- +"Test nested queries automatically disable weighted mode": + - requires: + cluster_features: "gte_v8.15.0" + reason: 'nested was not correctly skipped until 8.15' + - do: - indices.refresh: {} + search: + index: test + body: { + "query": { + "nested": { + "path": "nested", + "query": { + "multi_match": { + "query": "quick brown fox", + "type": "phrase", + "fields": [ "nested.text" ] + } + } + } + }, + "highlight": { "type": "unified", "fields": { "*": { } } } + } + + - length: { hits.hits.0.highlight: 1 } + - match: { hits.hits.0.highlight.nested\.text.0: "The quick brown fox is brown." } - do: search: - index: test-highlighting-knn + index: test body: { - "query": { "multi_match": { "query": "quick brown fox", "type": "phrase", "fields": [ "text*" ] } }, - "highlight": { "type": "unified", "fields": { "*": { } } }, - "knn": { "field": "vectors", "query_vector": [1, 2], "k": 10, "num_candidates": 10 } } + "query": { + "bool": { + "must_not": { + "nested": { + "path": "nested", + "query": { + "multi_match": { "query": "quick red fox", "type": "phrase", "fields": [ "nested.text" ] } + } + } + }, + "should": { + "multi_match": { "query": "quick brown fox", "type": "phrase", "fields": [ "text*" ] } + } + } + }, + "highlight": { "type": "unified", "fields": { "text*": { } } } + } + - length: { hits.hits.0.highlight: 3 } - match: { hits.hits.0.highlight.text.0: "The quick brown fox is brown." } - match: { hits.hits.0.highlight.text\.fvh.0: "The quick brown fox is brown." } - match: { hits.hits.0.highlight.text\.postings.0: "The quick brown fox is brown." } diff --git a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java index 5c1381f730013..c29e248b1a689 100644 --- a/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java +++ b/server/src/main/java/org/elasticsearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java @@ -293,7 +293,8 @@ public QueryVisitor getSubVisitor(BooleanClause.Occur occur, Query parent) { if (parent instanceof ESToParentBlockJoinQuery) { hasUnknownLeaf[0] = true; } - return super.getSubVisitor(occur, parent); + // we want to visit all queries, including those within the must_not clauses. + return this; } }); return hasUnknownLeaf[0]; From bc37ecfbafefd7cb84976cd17a8129bd7c24afac Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 10 May 2024 09:48:37 +0100 Subject: [PATCH 02/11] Specify some parameters as always supported by capabilities (#108461) --- .../java/org/elasticsearch/rest/BaseRestHandler.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index 70801cdef560b..b142e4d567c04 100644 --- a/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -76,13 +76,18 @@ public final long getUsageCount() { @Override public abstract List routes(); + private static final Set ALWAYS_SUPPORTED = Set.of("format", "filter_path", "pretty", "human"); + @Override public final void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { // check if the query has any parameters that are not in the supported set (if declared) Set supported = supportedQueryParameters(); - if (supported != null && supported.containsAll(request.params().keySet()) == false) { - Set unsupported = Sets.difference(request.params().keySet(), supported); - throw new IllegalArgumentException(unrecognized(request, unsupported, supported, "parameter")); + if (supported != null) { + var allSupported = Sets.union(ALWAYS_SUPPORTED, supported); + if (allSupported.containsAll(request.params().keySet()) == false) { + Set unsupported = Sets.difference(request.params().keySet(), allSupported); + throw new IllegalArgumentException(unrecognized(request, unsupported, allSupported, "parameter")); + } } // prepare the request for execution; has the side effect of touching the request parameters From 0eae05633684c6b2c974cd0272713bf52c4ab66d Mon Sep 17 00:00:00 2001 From: Tim Grein Date: Fri, 10 May 2024 11:10:06 +0200 Subject: [PATCH 03/11] [Inference API] Add AzureOpenAiCompletionServiceSettings and AzureOpenAiCompletionTaskSettings to InferenceNamedWriteablesProvider (#108491) --- .../InferenceNamedWriteablesProvider.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java index 8d01b25aa2795..41bef3521cdf2 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java @@ -26,6 +26,8 @@ import org.elasticsearch.xpack.core.inference.results.TextEmbeddingByteResults; import org.elasticsearch.xpack.core.inference.results.TextEmbeddingResults; import org.elasticsearch.xpack.inference.services.azureopenai.AzureOpenAiSecretSettings; +import org.elasticsearch.xpack.inference.services.azureopenai.completion.AzureOpenAiCompletionServiceSettings; +import org.elasticsearch.xpack.inference.services.azureopenai.completion.AzureOpenAiCompletionTaskSettings; import org.elasticsearch.xpack.inference.services.azureopenai.embeddings.AzureOpenAiEmbeddingsServiceSettings; import org.elasticsearch.xpack.inference.services.azureopenai.embeddings.AzureOpenAiEmbeddingsTaskSettings; import org.elasticsearch.xpack.inference.services.cohere.CohereServiceSettings; @@ -237,6 +239,21 @@ public static List getNamedWriteables() { ) ); + namedWriteables.add( + new NamedWriteableRegistry.Entry( + ServiceSettings.class, + AzureOpenAiCompletionServiceSettings.NAME, + AzureOpenAiCompletionServiceSettings::new + ) + ); + namedWriteables.add( + new NamedWriteableRegistry.Entry( + TaskSettings.class, + AzureOpenAiCompletionTaskSettings.NAME, + AzureOpenAiCompletionTaskSettings::new + ) + ); + return namedWriteables; } } From 2541ce9c4d37191f43cfc0be3c9462adbb8dc1fb Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Fri, 10 May 2024 11:47:31 +0200 Subject: [PATCH 04/11] Log skipped prevoting as INFO (#108411) Relates ES-6576 --- .../org/elasticsearch/cluster/coordination/Coordinator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java index 156ba88a7d2b1..daff05f0fb19b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/Coordinator.java @@ -1781,7 +1781,7 @@ public void run() { final var nodeEligibility = localNodeMayWinElection(lastAcceptedState, electionStrategy); if (nodeEligibility.mayWin() == false) { assert nodeEligibility.reason().isEmpty() == false; - logger.trace( + logger.info( "skip prevoting as local node may not win election ({}): {}", nodeEligibility.reason(), lastAcceptedState.coordinationMetadata() From 2e0f8d087c370c43d258c2e1ac4e5ac91a2a9c2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Fri, 10 May 2024 11:58:34 +0200 Subject: [PATCH 05/11] Add a SIMD (AVX2) optimised vector distance function for int7 on x64 (#108088) * Adding support for x64 to native vec library * Fix: aarch64 sqr7u dims * Fix: add symbol stripping (deb lintian) --------- Co-authored-by: Chris Hegarty <62058229+ChrisHegarty@users.noreply.github.com> Co-authored-by: Elastic Machine --- docs/changelog/108088.yaml | 5 + libs/native/libraries/build.gradle | 2 +- .../nativeaccess/PosixNativeAccess.java | 10 +- .../VectorSimilarityFunctionsTests.java | 4 +- libs/vec/native/Dockerfile | 5 +- libs/vec/native/build.gradle | 76 +++++++-- libs/vec/native/publish_vec_binaries.sh | 16 +- libs/vec/native/src/vec/c/{ => aarch64}/vec.c | 2 +- libs/vec/native/src/vec/c/amd64/vec.c | 150 ++++++++++++++++++ libs/vec/native/src/vec/headers/vec.h | 2 +- .../vec/AbstractVectorTestCase.java | 4 +- 11 files changed, 254 insertions(+), 22 deletions(-) create mode 100644 docs/changelog/108088.yaml rename libs/vec/native/src/vec/c/{ => aarch64}/vec.c (99%) create mode 100644 libs/vec/native/src/vec/c/amd64/vec.c diff --git a/docs/changelog/108088.yaml b/docs/changelog/108088.yaml new file mode 100644 index 0000000000000..95c58f6dc19f1 --- /dev/null +++ b/docs/changelog/108088.yaml @@ -0,0 +1,5 @@ +pr: 108088 +summary: Add a SIMD (AVX2) optimised vector distance function for int7 on x64 +area: "Search" +type: enhancement +issues: [] diff --git a/libs/native/libraries/build.gradle b/libs/native/libraries/build.gradle index 168eb533fea74..7a545787bbdae 100644 --- a/libs/native/libraries/build.gradle +++ b/libs/native/libraries/build.gradle @@ -18,7 +18,7 @@ configurations { } var zstdVersion = "1.5.5" -var vecVersion = "1.0.6" +var vecVersion = "1.0.8" repositories { exclusiveContent { diff --git a/libs/native/src/main/java/org/elasticsearch/nativeaccess/PosixNativeAccess.java b/libs/native/src/main/java/org/elasticsearch/nativeaccess/PosixNativeAccess.java index 56017d3a8a20a..c390cfc9289c6 100644 --- a/libs/native/src/main/java/org/elasticsearch/nativeaccess/PosixNativeAccess.java +++ b/libs/native/src/main/java/org/elasticsearch/nativeaccess/PosixNativeAccess.java @@ -45,7 +45,15 @@ public Optional getVectorSimilarityFunctions() { } static boolean isNativeVectorLibSupported() { - return Runtime.version().feature() >= 21 && isMacOrLinuxAarch64() && checkEnableSystemProperty(); + return Runtime.version().feature() >= 21 && (isMacOrLinuxAarch64() || isLinuxAmd64()) && checkEnableSystemProperty(); + } + + /** + * Returns true iff the architecture is x64 (amd64) and the OS Linux (the OS we currently support for the native lib). + */ + static boolean isLinuxAmd64() { + String name = System.getProperty("os.name"); + return (name.startsWith("Linux")) && System.getProperty("os.arch").equals("amd64"); } /** Returns true iff the OS is Mac or Linux, and the architecture is aarch64. */ diff --git a/libs/native/src/test/java/org/elasticsearch/nativeaccess/VectorSimilarityFunctionsTests.java b/libs/native/src/test/java/org/elasticsearch/nativeaccess/VectorSimilarityFunctionsTests.java index adf32874c04f1..8c4cbb688abcd 100644 --- a/libs/native/src/test/java/org/elasticsearch/nativeaccess/VectorSimilarityFunctionsTests.java +++ b/libs/native/src/test/java/org/elasticsearch/nativeaccess/VectorSimilarityFunctionsTests.java @@ -37,7 +37,9 @@ public boolean supported() { var arch = System.getProperty("os.arch"); var osName = System.getProperty("os.name"); - if (jdkVersion >= 21 && arch.equals("aarch64") && (osName.startsWith("Mac") || osName.equals("Linux"))) { + if (jdkVersion >= 21 + && ((arch.equals("aarch64") && (osName.startsWith("Mac") || osName.equals("Linux"))) + || (arch.equals("amd64") && osName.equals("Linux")))) { assertThat(vectorSimilarityFunctions, isPresent()); return true; } else { diff --git a/libs/vec/native/Dockerfile b/libs/vec/native/Dockerfile index 25dcf4d4854d0..66eb7e92ef479 100644 --- a/libs/vec/native/Dockerfile +++ b/libs/vec/native/Dockerfile @@ -4,6 +4,7 @@ RUN apt update RUN apt install -y gcc g++ openjdk-17-jdk COPY . /workspace WORKDIR /workspace -RUN ./gradlew --quiet --console=plain clean vecSharedLibrary +RUN ./gradlew --quiet --console=plain clean buildSharedLibrary +RUN strip --strip-unneeded build/output/libvec.so -CMD cat build/libs/vec/shared/libvec.so +CMD cat build/output/libvec.so diff --git a/libs/vec/native/build.gradle b/libs/vec/native/build.gradle index 6a658da0644b7..7edf46d406862 100644 --- a/libs/vec/native/build.gradle +++ b/libs/vec/native/build.gradle @@ -12,9 +12,10 @@ var os = org.gradle.internal.os.OperatingSystem.current() // To update this library run publish_vec_binaries.sh ( or ./gradlew vecSharedLibrary ) // Or // For local development, build the docker image with: -// docker build --platform linux/arm64 --progress=plain . +// docker build --platform linux/arm64 --progress=plain . (for aarch64) +// docker build --platform linux/amd64 --progress=plain . (for x64) // Grab the image id from the console output, then, e.g. -// docker run 9c9f36564c148b275aeecc42749e7b4580ded79dcf51ff6ccc008c8861e7a979 > build/libs/vec/shared/libvec.so +// docker run 9c9f36564c148b275aeecc42749e7b4580ded79dcf51ff6ccc008c8861e7a979 > build/libs/vec/shared/$arch/libvec.so // // To run tests and benchmarks on a locally built libvec, // 1. Temporarily comment out the download in libs/native/library/build.gradle @@ -30,26 +31,83 @@ var os = org.gradle.internal.os.OperatingSystem.current() group = 'org.elasticsearch' +def platformName = System.getProperty("os.arch"); + model { + platforms { + aarch64 { + architecture "aarch64" + } + amd64 { + architecture "x86-64" + } + } toolChains { gcc(Gcc) { target("aarch64") { cCompiler.executable = "/usr/bin/gcc" + cCompiler.withArguments { args -> args.addAll(["-O3", "-std=c99", "-march=armv8-a"]) } + } + target("amd64") { + cCompiler.executable = "/usr/bin/gcc" + cCompiler.withArguments { args -> args.addAll(["-O3", "-std=c99", "-march=core-avx2", "-Wno-incompatible-pointer-types"]) } } } - clang(Clang) - } - platforms { - aarch64 { - architecture "aarch64" + cl(VisualCpp) { + eachPlatform { toolchain -> + def platform = toolchain.getPlatform() + if (platform.name == "x64") { + cCompiler.withArguments { args -> args.addAll(["/O2", "/LD", "-march=core-avx2"]) } + } + } + } + clang(Clang) { + target("amd64") { + cCompiler.withArguments { args -> args.addAll(["-O3", "-std=c99", "-march=core-avx2"]) } + } } } components { vec(NativeLibrarySpec) { targetPlatform "aarch64" - binaries.withType(SharedLibraryBinarySpec) { - cCompiler.args "-O3", "-std=c99", "-march=armv8-a" + targetPlatform "amd64" + + sources { + c { + source { + srcDir "src/vec/c/${platformName}/" + include "*.c" + } + exportedHeaders { + srcDir "src/vec/headers/" + } + } + } + } + } +} + +tasks.register('buildSharedLibrary') { + description = 'Assembles native shared library for the host architecture' + if (platformName.equals("aarch64")) { + dependsOn tasks.vecAarch64SharedLibrary + doLast { + copy { + from tasks.linkVecAarch64SharedLibrary.outputs.files.files + into layout.buildDirectory.dir('output'); + duplicatesStrategy = 'INCLUDE' + } + } + } else if (platformName.equals("amd64")) { + dependsOn tasks.vecAmd64SharedLibrary + doLast { + copy { + from tasks.linkVecAmd64SharedLibrary.outputs.files.files + into layout.buildDirectory.dir('output'); + duplicatesStrategy = 'INCLUDE' } } + } else { + throw new GradleException("Unsupported platform: " + platformName) } } diff --git a/libs/vec/native/publish_vec_binaries.sh b/libs/vec/native/publish_vec_binaries.sh index e17690160e253..2ed6c750ab9e8 100755 --- a/libs/vec/native/publish_vec_binaries.sh +++ b/libs/vec/native/publish_vec_binaries.sh @@ -19,7 +19,7 @@ if [ -z "$ARTIFACTORY_API_KEY" ]; then exit 1; fi -VERSION="1.0.6" +VERSION="1.0.8" ARTIFACTORY_REPOSITORY="${ARTIFACTORY_REPOSITORY:-https://artifactory.elastic.dev/artifactory/elasticsearch-native/}" TEMP=$(mktemp -d) @@ -29,16 +29,22 @@ if curl -sS -I --fail --location "${ARTIFACTORY_REPOSITORY}/org/elasticsearch/ve fi echo 'Building Darwin binary...' -./gradlew --quiet --console=plain vecSharedLibrary +./gradlew --quiet --console=plain vecAarch64SharedLibrary echo 'Building Linux binary...' DOCKER_IMAGE=$(docker build --platform linux/arm64 --quiet .) -docker run $DOCKER_IMAGE > build/libs/vec/shared/libvec.so +docker run $DOCKER_IMAGE > build/libs/vec/shared/aarch64/libvec.so + +echo 'Building Linux x64 binary...' +DOCKER_IMAGE=$(docker build --platform linux/amd64 --quiet .) +docker run --platform linux/amd64 $DOCKER_IMAGE > build/libs/vec/shared/amd64/libvec.so mkdir -p $TEMP/darwin-aarch64 mkdir -p $TEMP/linux-aarch64 -cp build/libs/vec/shared/libvec.dylib $TEMP/darwin-aarch64/ -cp build/libs/vec/shared/libvec.so $TEMP/linux-aarch64/ +mkdir -p $TEMP/linux-x64 +cp build/libs/vec/shared/aarch64/libvec.dylib $TEMP/darwin-aarch64/ +cp build/libs/vec/shared/aarch64/libvec.so $TEMP/linux-aarch64/ +cp build/libs/vec/shared/amd64/libvec.so $TEMP/linux-x64/ echo 'Uploading to Artifactory...' (cd $TEMP && zip -rq - .) | curl -sS -X PUT -H "X-JFrog-Art-Api: ${ARTIFACTORY_API_KEY}" --data-binary @- --location "${ARTIFACTORY_REPOSITORY}/org/elasticsearch/vec/${VERSION}/vec-${VERSION}.zip" diff --git a/libs/vec/native/src/vec/c/vec.c b/libs/vec/native/src/vec/c/aarch64/vec.c similarity index 99% rename from libs/vec/native/src/vec/c/vec.c rename to libs/vec/native/src/vec/c/aarch64/vec.c index 05dfe64a3be9b..478e5e84d3859 100644 --- a/libs/vec/native/src/vec/c/vec.c +++ b/libs/vec/native/src/vec/c/aarch64/vec.c @@ -121,7 +121,7 @@ static inline int32_t sqr7u_inner(int8_t *a, int8_t *b, size_t dims) { EXPORT int32_t sqr7u(int8_t* a, int8_t* b, size_t dims) { int32_t res = 0; int i = 0; - if (i > SQR7U_STRIDE_BYTES_LEN) { + if (dims > SQR7U_STRIDE_BYTES_LEN) { i += dims & ~(SQR7U_STRIDE_BYTES_LEN - 1); res = sqr7u_inner(a, b, i); } diff --git a/libs/vec/native/src/vec/c/amd64/vec.c b/libs/vec/native/src/vec/c/amd64/vec.c new file mode 100644 index 0000000000000..c9a49ad2d1d4d --- /dev/null +++ b/libs/vec/native/src/vec/c/amd64/vec.c @@ -0,0 +1,150 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +#include +#include +#include "vec.h" + +#include +#include + +#ifndef DOT7U_STRIDE_BYTES_LEN +#define DOT7U_STRIDE_BYTES_LEN 32 // Must be a power of 2 +#endif + +#ifndef SQR7U_STRIDE_BYTES_LEN +#define SQR7U_STRIDE_BYTES_LEN 32 // Must be a power of 2 +#endif + +#ifdef _MSC_VER +#include +#elif __GNUC__ +#include +#elif __clang__ +#include +#endif + +// Multi-platform CPUID "intrinsic"; it takes as input a "functionNumber" (or "leaf", the eax registry). "Subleaf" +// is always 0. Output is stored in the passed output parameter: output[0] = eax, output[1] = ebx, output[2] = ecx, +// output[3] = edx +static inline void cpuid(int output[4], int functionNumber) { +#if defined(__GNUC__) || defined(__clang__) + // use inline assembly, Gnu/AT&T syntax + int a, b, c, d; + __asm("cpuid" : "=a"(a), "=b"(b), "=c"(c), "=d"(d) : "a"(functionNumber), "c"(0) : ); + output[0] = a; + output[1] = b; + output[2] = c; + output[3] = d; + +#elif defined (_MSC_VER) + __cpuidex(output, functionNumber, 0); +#else + #error Unsupported compiler +#endif +} + +// Utility function to horizontally add 8 32-bit integers +static inline int hsum_i32_8(const __m256i a) { + const __m128i sum128 = _mm_add_epi32(_mm256_castsi256_si128(a), _mm256_extractf128_si256(a, 1)); + const __m128i hi64 = _mm_unpackhi_epi64(sum128, sum128); + const __m128i sum64 = _mm_add_epi32(hi64, sum128); + const __m128i hi32 = _mm_shuffle_epi32(sum64, _MM_SHUFFLE(2, 3, 0, 1)); + return _mm_cvtsi128_si32(_mm_add_epi32(sum64, hi32)); +} + +EXPORT int vec_caps() { + int cpuInfo[4] = {-1}; + // Calling __cpuid with 0x0 as the function_id argument + // gets the number of the highest valid function ID. + cpuid(cpuInfo, 0); + int functionIds = cpuInfo[0]; + if (functionIds >= 7) { + cpuid(cpuInfo, 7); + int ebx = cpuInfo[1]; + // AVX2 flag is the 5th bit + // We assume that all processors that have AVX2 also have FMA3 + return (ebx & (1 << 5)) != 0; + } + return 0; +} + +static inline int32_t dot7u_inner(int8_t* a, int8_t* b, size_t dims) { + const __m256i ones = _mm256_set1_epi16(1); + + // Init accumulator(s) with 0 + __m256i acc1 = _mm256_setzero_si256(); + +#pragma GCC unroll 4 + for(int i = 0; i < dims; i += DOT7U_STRIDE_BYTES_LEN) { + // Load packed 8-bit integers + __m256i va1 = _mm256_loadu_si256(a + i); + __m256i vb1 = _mm256_loadu_si256(b + i); + + // Perform multiplication and create 16-bit values + // Vertically multiply each unsigned 8-bit integer from va with the corresponding + // 8-bit integer from vb, producing intermediate signed 16-bit integers. + const __m256i vab = _mm256_maddubs_epi16(va1, vb1); + // Horizontally add adjacent pairs of intermediate signed 16-bit integers, and pack the results. + acc1 = _mm256_add_epi32(_mm256_madd_epi16(ones, vab), acc1); + } + + // reduce (horizontally add all) + return hsum_i32_8(acc1); +} + +EXPORT int32_t dot7u(int8_t* a, int8_t* b, size_t dims) { + int32_t res = 0; + int i = 0; + if (dims > DOT7U_STRIDE_BYTES_LEN) { + i += dims & ~(DOT7U_STRIDE_BYTES_LEN - 1); + res = dot7u_inner(a, b, i); + } + for (; i < dims; i++) { + res += a[i] * b[i]; + } + return res; +} + +static inline int32_t sqr7u_inner(int8_t *a, int8_t *b, size_t dims) { + // Init accumulator(s) with 0 + __m256i acc1 = _mm256_setzero_si256(); + + const __m256i ones = _mm256_set1_epi16(1); + +#pragma GCC unroll 4 + for(int i = 0; i < dims; i += SQR7U_STRIDE_BYTES_LEN) { + // Load packed 8-bit integers + __m256i va1 = _mm256_loadu_si256(a + i); + __m256i vb1 = _mm256_loadu_si256(b + i); + + const __m256i dist1 = _mm256_sub_epi8(va1, vb1); + const __m256i abs_dist1 = _mm256_sign_epi8(dist1, dist1); + const __m256i sqr1 = _mm256_maddubs_epi16(abs_dist1, abs_dist1); + + acc1 = _mm256_add_epi32(_mm256_madd_epi16(ones, sqr1), acc1); + } + + // reduce (accumulate all) + return hsum_i32_8(acc1); +} + +EXPORT int32_t sqr7u(int8_t* a, int8_t* b, size_t dims) { + int32_t res = 0; + int i = 0; + if (dims > SQR7U_STRIDE_BYTES_LEN) { + i += dims & ~(SQR7U_STRIDE_BYTES_LEN - 1); + res = sqr7u_inner(a, b, i); + } + for (; i < dims; i++) { + int32_t dist = a[i] - b[i]; + res += dist * dist; + } + return res; +} + diff --git a/libs/vec/native/src/vec/headers/vec.h b/libs/vec/native/src/vec/headers/vec.h index 5d3806dfccbe6..49fa29ec6fae9 100644 --- a/libs/vec/native/src/vec/headers/vec.h +++ b/libs/vec/native/src/vec/headers/vec.h @@ -7,7 +7,7 @@ */ #ifdef _MSC_VER -#define EXPORT extern "C" __declspec(dllexport) +#define EXPORT __declspec(dllexport) #elif defined(__GNUC__) && !defined(__clang__) #define EXPORT __attribute__((externally_visible,visibility("default"))) #elif __clang__ diff --git a/libs/vec/src/test/java/org/elasticsearch/vec/AbstractVectorTestCase.java b/libs/vec/src/test/java/org/elasticsearch/vec/AbstractVectorTestCase.java index 771f665fb4084..13f2d5a03ec76 100644 --- a/libs/vec/src/test/java/org/elasticsearch/vec/AbstractVectorTestCase.java +++ b/libs/vec/src/test/java/org/elasticsearch/vec/AbstractVectorTestCase.java @@ -39,7 +39,9 @@ public static boolean supported() { var arch = System.getProperty("os.arch"); var osName = System.getProperty("os.name"); - if (jdkVersion >= 21 && arch.equals("aarch64") && (osName.startsWith("Mac") || osName.equals("Linux"))) { + if (jdkVersion >= 21 + && (arch.equals("aarch64") && (osName.startsWith("Mac") || osName.equals("Linux")) + || arch.equals("amd64") && osName.equals("Linux"))) { assertThat(factory, isPresent()); return true; } else { From d2d1357a334df228dd59878d844bf3870e1efc8b Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Fri, 10 May 2024 12:37:54 +0200 Subject: [PATCH 06/11] Expose capability checks for YAML REST tests (#108425) Co-authored-by: Simon Cooper --- .../rest-api-spec/api/capabilities.json | 47 ++++++++++ .../test/capabilities/10_basic.yml | 28 ++++++ .../SimpleNodesCapabilitiesIT.java | 10 +-- .../NodesCapabilitiesResponse.java | 10 ++- .../yaml/ClientYamlTestExecutionContext.java | 43 ++++++++- .../yaml/section/PrerequisiteSection.java | 90 ++++++++++++++++--- .../test/rest/yaml/section/Prerequisites.java | 20 ++++- .../section/PrerequisiteSectionTests.java | 83 ++++++++++++++++- 8 files changed, 307 insertions(+), 24 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/api/capabilities.json create mode 100644 rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/capabilities/10_basic.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/capabilities.json b/rest-api-spec/src/main/resources/rest-api-spec/api/capabilities.json new file mode 100644 index 0000000000000..28c341d9983cc --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/capabilities.json @@ -0,0 +1,47 @@ +{ + "capabilities": { + "documentation": { + "url": "https://www.elastic.co/guide/en/elasticsearch/reference/master/capabilities.html", + "description": "Checks if the specified combination of method, API, parameters, and arbitrary capabilities are supported" + }, + "stability": "experimental", + "visibility": "private", + "headers": { + "accept": [ + "application/json" + ] + }, + "url": { + "paths": [ + { + "path": "/_capabilities", + "methods": [ + "GET" + ] + } + ] + }, + "params": { + "method": { + "type": "enum", + "description": "REST method to check", + "options": [ + "GET", "HEAD", "POST", "PUT", "DELETE" + ], + "default": "GET" + }, + "path": { + "type": "string", + "description": "API path to check" + }, + "parameters": { + "type": "string", + "description": "Comma-separated list of API parameters to check" + }, + "capabilities": { + "type": "string", + "description": "Comma-separated list of arbitrary API capabilities to check" + } + } + } +} diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/capabilities/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/capabilities/10_basic.yml new file mode 100644 index 0000000000000..715e696bd1032 --- /dev/null +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/capabilities/10_basic.yml @@ -0,0 +1,28 @@ +--- +"Capabilities API": + + - requires: + capabilities: + - method: GET + path: /_capabilities + parameters: [method, path, parameters, capabilities] + capabilities: [] + reason: "capabilities api requires itself to be supported" + + - do: + capabilities: + method: GET + path: /_capabilities + parameters: method,path,parameters,capabilities + error_trace: false + + - match: { supported: true } + + - do: + capabilities: + method: GET + path: /_capabilities + parameters: unknown + error_trace: false + + - match: { supported: false } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/nodescapabilities/SimpleNodesCapabilitiesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/nodescapabilities/SimpleNodesCapabilitiesIT.java index 7e4ae040caeca..9b60044c94f70 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/nodescapabilities/SimpleNodesCapabilitiesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/nodescapabilities/SimpleNodesCapabilitiesIT.java @@ -15,8 +15,8 @@ import java.io.IOException; +import static org.elasticsearch.test.hamcrest.OptionalMatchers.isPresentWith; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0) public class SimpleNodesCapabilitiesIT extends ESIntegTestCase { @@ -31,25 +31,25 @@ public void testNodesCapabilities() throws IOException { NodesCapabilitiesResponse response = clusterAdmin().nodesCapabilities(new NodesCapabilitiesRequest().path("_capabilities")) .actionGet(); assertThat(response.getNodes(), hasSize(2)); - assertThat(response.isSupported(), is(true)); + assertThat(response.isSupported(), isPresentWith(true)); // check we support some parameters of the capabilities API response = clusterAdmin().nodesCapabilities(new NodesCapabilitiesRequest().path("_capabilities").parameters("method", "path")) .actionGet(); assertThat(response.getNodes(), hasSize(2)); - assertThat(response.isSupported(), is(true)); + assertThat(response.isSupported(), isPresentWith(true)); // check we don't support some other parameters of the capabilities API response = clusterAdmin().nodesCapabilities(new NodesCapabilitiesRequest().path("_capabilities").parameters("method", "invalid")) .actionGet(); assertThat(response.getNodes(), hasSize(2)); - assertThat(response.isSupported(), is(false)); + assertThat(response.isSupported(), isPresentWith(false)); // check we don't support a random invalid api // TODO this is not working yet - see https://github.com/elastic/elasticsearch/issues/107425 /*response = clusterAdmin().nodesCapabilities(new NodesCapabilitiesRequest().path("_invalid")) .actionGet(); assertThat(response.getNodes(), hasSize(2)); - assertThat(response.isSupported(), is(false));*/ + assertThat(response.isSupported(), isPresentWith(false));*/ } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/NodesCapabilitiesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/NodesCapabilitiesResponse.java index 63fdb9f7da08a..c2acbf65f6e57 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/NodesCapabilitiesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/NodesCapabilitiesResponse.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; +import java.util.Optional; public class NodesCapabilitiesResponse extends BaseNodesResponse implements ToXContentFragment { protected NodesCapabilitiesResponse(ClusterName clusterName, List nodes, List failures) { @@ -35,12 +36,15 @@ protected void writeNodesTo(StreamOutput out, List nodes) throws TransportAction.localOnly(); } - public boolean isSupported() { - return getNodes().isEmpty() == false && getNodes().stream().allMatch(NodeCapability::isSupported); + public Optional isSupported() { + // if there are any failures, we don't know if it is fully supported by all nodes in the cluster + if (hasFailures() || getNodes().isEmpty()) return Optional.empty(); + return Optional.of(getNodes().stream().allMatch(NodeCapability::isSupported)); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.field("supported", isSupported()); + Optional supported = isSupported(); + return builder.field("supported", supported.orElse(null)); } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java index 10bf2fb4b0a9f..4954065369ad9 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java @@ -16,7 +16,9 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.util.BytesRef; import org.elasticsearch.client.NodeSelector; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.util.Maps; import org.elasticsearch.test.rest.Stash; import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi; @@ -25,14 +27,19 @@ import org.elasticsearch.xcontent.XContentType; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.BiPredicate; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; + /** * Execution context passed across the REST tests. * Holds the REST client used to communicate with elasticsearch. @@ -122,7 +129,15 @@ public ClientYamlTestResponse callApi( ) throws IOException { // makes a copy of the parameters before modifying them for this specific request Map requestParams = new HashMap<>(params); - requestParams.putIfAbsent("error_trace", "true"); // By default ask for error traces, this my be overridden by params + requestParams.compute("error_trace", (k, v) -> { + if (v == null) { + return "true"; // By default ask for error traces, this my be overridden by params + } else if (v.equals("false")) { + return null; + } else { + return v; + } + }); for (Map.Entry entry : requestParams.entrySet()) { if (stash.containsStashedValue(entry.getValue())) { entry.setValue(stash.getValue(entry.getValue()).toString()); @@ -264,4 +279,30 @@ public ClientYamlTestCandidate getClientYamlTestCandidate() { public boolean clusterHasFeature(String featureId) { return testFeatureService.clusterHasFeature(featureId); } + + public Optional clusterHasCapabilities(String method, String path, String parametersString, String capabilitiesString) { + Map params = Maps.newMapWithExpectedSize(5); + params.put("method", method); + params.put("path", path); + if (Strings.hasLength(parametersString)) { + params.put("parameters", parametersString); + } + if (Strings.hasLength(capabilitiesString)) { + params.put("capabilities", capabilitiesString); + } + params.put("error_trace", "false"); // disable error trace + try { + ClientYamlTestResponse resp = callApi("capabilities", params, emptyList(), emptyMap()); + // anything other than 200 should result in an exception, handled below + assert resp.getStatusCode() == 200 : "Unknown response code " + resp.getStatusCode(); + return Optional.ofNullable(resp.evaluate("supported")); + } catch (ClientYamlTestResponseException responseException) { + if (responseException.getRestTestResponse().getStatusCode() / 100 == 4) { + return Optional.empty(); // we don't know, the capabilities API is unsupported + } + throw new UncheckedIOException(responseException); + } catch (IOException ioException) { + throw new UncheckedIOException(ioException); + } + } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSection.java index 1ee447da1f111..c12de7e1155a7 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSection.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -27,6 +28,7 @@ import java.util.function.Predicate; import static java.util.Collections.emptyList; +import static java.util.stream.Collectors.joining; /** * Represents a section where prerequisites to run a specific test section or suite are specified. It is possible to specify preconditions @@ -43,16 +45,23 @@ record KnownIssue(String clusterFeature, String fixedBy) { private static final Set FIELD_NAMES = Set.of("cluster_feature", "fixed_by"); } + record CapabilitiesCheck(String method, String path, String parameters, String capabilities) { + private static final Set FIELD_NAMES = Set.of("method", "path", "parameters", "capabilities"); + } + static class PrerequisiteSectionBuilder { - String skipVersionRange = null; String skipReason = null; - String requiresReason = null; - List requiredYamlRunnerFeatures = new ArrayList<>(); + String skipVersionRange = null; List skipOperatingSystems = new ArrayList<>(); List skipKnownIssues = new ArrayList<>(); String skipAwaitsFix = null; Set skipClusterFeatures = new HashSet<>(); + List skipCapabilities = new ArrayList<>(); + + String requiresReason = null; + List requiredYamlRunnerFeatures = new ArrayList<>(); Set requiredClusterFeatures = new HashSet<>(); + List requiredCapabilities = new ArrayList<>(); enum XPackRequired { NOT_SPECIFIED, @@ -116,11 +125,21 @@ public PrerequisiteSectionBuilder skipKnownIssue(KnownIssue knownIssue) { return this; } + public PrerequisiteSectionBuilder skipIfCapabilities(CapabilitiesCheck capabilitiesCheck) { + skipCapabilities.add(capabilitiesCheck); + return this; + } + public PrerequisiteSectionBuilder requireClusterFeature(String featureName) { requiredClusterFeatures.add(featureName); return this; } + public PrerequisiteSectionBuilder requireCapabilities(CapabilitiesCheck capabilitiesCheck) { + requiredCapabilities.add(capabilitiesCheck); + return this; + } + public PrerequisiteSectionBuilder skipIfOs(String osName) { this.skipOperatingSystems.add(osName); return this; @@ -128,13 +147,15 @@ public PrerequisiteSectionBuilder skipIfOs(String osName) { void validate(XContentLocation contentLocation) { if ((Strings.isEmpty(skipVersionRange)) - && requiredYamlRunnerFeatures.isEmpty() && skipOperatingSystems.isEmpty() - && xpackRequired == XPackRequired.NOT_SPECIFIED - && requiredClusterFeatures.isEmpty() && skipClusterFeatures.isEmpty() + && skipCapabilities.isEmpty() && skipKnownIssues.isEmpty() - && Strings.isEmpty(skipAwaitsFix)) { + && Strings.isEmpty(skipAwaitsFix) + && xpackRequired == XPackRequired.NOT_SPECIFIED + && requiredYamlRunnerFeatures.isEmpty() + && requiredCapabilities.isEmpty() + && requiredClusterFeatures.isEmpty()) { // TODO separate the validation for requires / skip when dropping parsing of legacy fields, e.g. features in skip throw new ParsingException(contentLocation, "at least one predicate is mandatory within a skip or requires section"); } @@ -143,11 +164,12 @@ void validate(XContentLocation contentLocation) { && (Strings.isEmpty(skipVersionRange) && skipOperatingSystems.isEmpty() && skipClusterFeatures.isEmpty() + && skipCapabilities.isEmpty() && skipKnownIssues.isEmpty()) == false) { throw new ParsingException(contentLocation, "reason is mandatory within this skip section"); } - if (Strings.isEmpty(requiresReason) && (requiredClusterFeatures.isEmpty() == false)) { + if (Strings.isEmpty(requiresReason) && ((requiredClusterFeatures.isEmpty() && requiredCapabilities.isEmpty()) == false)) { throw new ParsingException(contentLocation, "reason is mandatory within this requires section"); } @@ -190,6 +212,13 @@ public PrerequisiteSection build() { if (xpackRequired == XPackRequired.YES) { requiresCriteriaList.add(Prerequisites.hasXPack()); } + if (requiredClusterFeatures.isEmpty() == false) { + requiresCriteriaList.add(Prerequisites.requireClusterFeatures(requiredClusterFeatures)); + } + if (requiredCapabilities.isEmpty() == false) { + requiresCriteriaList.add(Prerequisites.requireCapabilities(requiredCapabilities)); + } + if (xpackRequired == XPackRequired.NO) { skipCriteriaList.add(Prerequisites.hasXPack()); } @@ -199,12 +228,12 @@ public PrerequisiteSection build() { if (skipOperatingSystems.isEmpty() == false) { skipCriteriaList.add(Prerequisites.skipOnOsList(skipOperatingSystems)); } - if (requiredClusterFeatures.isEmpty() == false) { - requiresCriteriaList.add(Prerequisites.requireClusterFeatures(requiredClusterFeatures)); - } if (skipClusterFeatures.isEmpty() == false) { skipCriteriaList.add(Prerequisites.skipOnClusterFeatures(skipClusterFeatures)); } + if (skipCapabilities.isEmpty() == false) { + skipCriteriaList.add(Prerequisites.skipCapabilities(skipCapabilities)); + } if (skipKnownIssues.isEmpty() == false) { skipCriteriaList.add(Prerequisites.skipOnKnownIssue(skipKnownIssues)); } @@ -287,6 +316,7 @@ static void parseSkipSection(XContentParser parser, PrerequisiteSectionBuilder b case "os" -> parseStrings(parser, builder::skipIfOs); case "cluster_features" -> parseStrings(parser, builder::skipIfClusterFeature); case "known_issues" -> parseArray(parser, PrerequisiteSection::parseKnownIssue, builder::skipKnownIssue); + case "capabilities" -> parseArray(parser, PrerequisiteSection::parseCapabilities, builder::skipIfCapabilities); default -> false; }; } @@ -337,12 +367,47 @@ private static KnownIssue parseKnownIssue(XContentParser parser) throws IOExcept if (fields.keySet().equals(KnownIssue.FIELD_NAMES) == false) { throw new ParsingException( parser.getTokenLocation(), - Strings.format("Expected fields %s, but got %s", KnownIssue.FIELD_NAMES, fields.keySet()) + Strings.format("Expected all of %s, but got %s", KnownIssue.FIELD_NAMES, fields.keySet()) ); } return new KnownIssue(fields.get("cluster_feature"), fields.get("fixed_by")); } + private static CapabilitiesCheck parseCapabilities(XContentParser parser) throws IOException { + Map fields = parser.map(); + if (CapabilitiesCheck.FIELD_NAMES.containsAll(fields.keySet()) == false) { + throw new ParsingException( + parser.getTokenLocation(), + Strings.format("Expected some of %s, but got %s", CapabilitiesCheck.FIELD_NAMES, fields.keySet()) + ); + } + Object path = fields.get("path"); + if (path == null) { + throw new ParsingException(parser.getTokenLocation(), "path is required"); + } + + return new CapabilitiesCheck( + ensureString(ensureString(fields.getOrDefault("method", "GET"))), + ensureString(path), + stringArrayAsParamString("parameters", fields), + stringArrayAsParamString("capabilities", fields) + ); + } + + private static String ensureString(Object obj) { + if (obj instanceof String str) return str; + throw new IllegalArgumentException("Expected STRING, but got: " + obj); + } + + private static String stringArrayAsParamString(String name, Map fields) { + Object value = fields.get(name); + if (value == null) return null; + if (value instanceof Collection values) { + return values.stream().map(PrerequisiteSection::ensureString).collect(joining(",")); + } + return ensureString(value); + } + static void parseRequiresSection(XContentParser parser, PrerequisiteSectionBuilder builder) throws IOException { requireStartObject("requires", parser.nextToken()); @@ -361,6 +426,7 @@ static void parseRequiresSection(XContentParser parser, PrerequisiteSectionBuild valid = switch (parser.currentName()) { case "test_runner_features" -> parseStrings(parser, f -> parseFeatureField(f, builder)); case "cluster_features" -> parseStrings(parser, builder::requireClusterFeature); + case "capabilities" -> parseArray(parser, PrerequisiteSection::parseCapabilities, builder::requireCapabilities); default -> false; }; } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/Prerequisites.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/Prerequisites.java index ca10101a4612c..86c035ebad62f 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/Prerequisites.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/Prerequisites.java @@ -10,8 +10,11 @@ import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; +import org.elasticsearch.test.rest.yaml.section.PrerequisiteSection.CapabilitiesCheck; +import org.elasticsearch.test.rest.yaml.section.PrerequisiteSection.KnownIssue; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -45,8 +48,23 @@ static Predicate skipOnClusterFeatures(Set clusterFeatures.stream().anyMatch(context::clusterHasFeature); } - static Predicate skipOnKnownIssue(List knownIssues) { + static Predicate skipOnKnownIssue(List knownIssues) { return context -> knownIssues.stream() .anyMatch(i -> context.clusterHasFeature(i.clusterFeature()) && context.clusterHasFeature(i.fixedBy()) == false); } + + static Predicate requireCapabilities(List checks) { + // requirement not fulfilled if unknown / capabilities API not supported + return context -> checks.stream().allMatch(check -> checkCapabilities(context, check).orElse(false)); + } + + static Predicate skipCapabilities(List checks) { + // skip if unknown / capabilities API not supported + return context -> checks.stream().anyMatch(check -> checkCapabilities(context, check).orElse(true)); + } + + private static Optional checkCapabilities(ClientYamlTestExecutionContext context, CapabilitiesCheck check) { + Optional b = context.clusterHasCapabilities(check.method(), check.path(), check.parameters(), check.capabilities()); + return b; + } } diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSectionTests.java index a77b2cc5b40f1..0bb31ae2c574a 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/PrerequisiteSectionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.core.Strings; import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; +import org.elasticsearch.test.rest.yaml.section.PrerequisiteSection.CapabilitiesCheck; import org.elasticsearch.test.rest.yaml.section.PrerequisiteSection.KnownIssue; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.yaml.YamlXContent; @@ -20,8 +21,11 @@ import java.io.IOException; import java.util.List; +import java.util.Optional; import java.util.Set; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.contains; @@ -36,6 +40,8 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.oneOf; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -357,8 +363,8 @@ public void testParseSkipSectionIncompleteKnownIssues() throws Exception { e.getMessage(), is( oneOf( - ("Expected fields [cluster_feature, fixed_by], but got [cluster_feature]"), - ("Expected fields [fixed_by, cluster_feature], but got [cluster_feature]") + ("Expected all of [cluster_feature, fixed_by], but got [cluster_feature]"), + ("Expected all of [fixed_by, cluster_feature], but got [cluster_feature]") ) ) ); @@ -498,6 +504,42 @@ public void testParseRequireAndSkipSectionsClusterFeatures() throws Exception { assertThat(parser.nextToken(), nullValue()); } + public void testParseRequireAndSkipSectionsCapabilities() throws Exception { + parser = createParser(YamlXContent.yamlXContent, """ + - requires: + capabilities: + - path: /a + - method: POST + path: /b + parameters: [param1, param2] + - method: PUT + path: /c + capabilities: [a, b, c] + reason: required to run test + - skip: + capabilities: + - path: /d + parameters: param1 + capabilities: a + reason: undesired if supported + """); + + var skipSectionBuilder = PrerequisiteSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat( + skipSectionBuilder.requiredCapabilities, + contains( + new CapabilitiesCheck("GET", "/a", null, null), + new CapabilitiesCheck("POST", "/b", "param1,param2", null), + new CapabilitiesCheck("PUT", "/c", null, "a,b,c") + ) + ); + assertThat(skipSectionBuilder.skipCapabilities, contains(new CapabilitiesCheck("GET", "/d", "param1", "a"))); + + assertThat(parser.currentToken(), equalTo(XContentParser.Token.END_ARRAY)); + assertThat(parser.nextToken(), nullValue()); + } + public void testParseRequireAndSkipSectionMultipleClusterFeatures() throws Exception { parser = createParser(YamlXContent.yamlXContent, """ - requires: @@ -659,6 +701,43 @@ public void testSkipKnownIssue() { assertFalse(section.skipCriteriaMet(mockContext)); } + public void testEvaluateCapabilities() { + List skipCapabilities = List.of( + new CapabilitiesCheck("GET", "/s", null, "c1,c2"), + new CapabilitiesCheck("GET", "/s", "p1,p2", "c1") + ); + List requiredCapabilities = List.of( + new CapabilitiesCheck("GET", "/r", null, null), + new CapabilitiesCheck("GET", "/r", "p1", null) + ); + PrerequisiteSection section = new PrerequisiteSection( + List.of(Prerequisites.skipCapabilities(skipCapabilities)), + "skip", + List.of(Prerequisites.requireCapabilities(requiredCapabilities)), + "required", + emptyList() + ); + + var context = mock(ClientYamlTestExecutionContext.class); + + // when the capabilities API is unavailable: + assertTrue(section.skipCriteriaMet(context)); // always skip if unavailable + assertFalse(section.requiresCriteriaMet(context)); // always fail requirements / skip if unavailable + + when(context.clusterHasCapabilities(anyString(), anyString(), any(), any())).thenReturn(Optional.of(FALSE)); + assertFalse(section.skipCriteriaMet(context)); + assertFalse(section.requiresCriteriaMet(context)); + + when(context.clusterHasCapabilities("GET", "/s", null, "c1,c2")).thenReturn(Optional.of(TRUE)); + assertTrue(section.skipCriteriaMet(context)); + + when(context.clusterHasCapabilities("GET", "/r", null, null)).thenReturn(Optional.of(TRUE)); + assertFalse(section.requiresCriteriaMet(context)); + + when(context.clusterHasCapabilities("GET", "/r", "p1", null)).thenReturn(Optional.of(TRUE)); + assertTrue(section.requiresCriteriaMet(context)); + } + public void evaluateEmpty() { var section = new PrerequisiteSection(List.of(), "unsupported", List.of(), "required", List.of()); From 7ed58e75dab2c36c99aabd78157be166c4ec322f Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Fri, 10 May 2024 13:35:00 +0200 Subject: [PATCH 07/11] Do not filter source if exclude contains `*` (#108501) This commit prevents the serialization of source if not needed. --- .../fetch/subphase/FetchSourcePhase.java | 11 +++++++--- .../search/lookup/SourceFilter.java | 4 ++++ .../fetch/subphase/FetchSourcePhaseTests.java | 21 +++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhase.java index 3b8e4e69d9318..68e46186e4505 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhase.java @@ -28,7 +28,7 @@ public FetchSubPhaseProcessor getProcessor(FetchContext fetchContext) { } assert fetchSourceContext.fetchSource(); SourceFilter sourceFilter = fetchSourceContext.filter(); - + final boolean filterExcludesAll = sourceFilter.excludesAll(); return new FetchSubPhaseProcessor() { private int fastPath; @@ -67,8 +67,13 @@ private void hitExecute(FetchSourceContext fetchSourceContext, HitContext hitCon return; } - // Otherwise, filter the source and add it to the hit. - source = source.filter(sourceFilter); + if (filterExcludesAll) { + // we can just add an empty map + source = Source.empty(source.sourceContentType()); + } else { + // Otherwise, filter the source and add it to the hit. + source = source.filter(sourceFilter); + } if (nestedHit) { source = extractNested(source, hitContext.hit().getNestedIdentity()); } diff --git a/server/src/main/java/org/elasticsearch/search/lookup/SourceFilter.java b/server/src/main/java/org/elasticsearch/search/lookup/SourceFilter.java index 3bf32159c1676..ceffb32c08b48 100644 --- a/server/src/main/java/org/elasticsearch/search/lookup/SourceFilter.java +++ b/server/src/main/java/org/elasticsearch/search/lookup/SourceFilter.java @@ -109,4 +109,8 @@ private Function buildBytesFilter() { } }; } + + public boolean excludesAll() { + return Arrays.asList(excludes).contains("*"); + } } diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhaseTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhaseTests.java index 3a4d67ae281f2..2b8bf0dad65fe 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhaseTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhaseTests.java @@ -52,6 +52,27 @@ public void testBasicFiltering() throws IOException { assertEquals(Collections.singletonMap("field1", "value"), hitContext.hit().getSourceAsMap()); } + public void testExcludesAll() throws IOException { + XContentBuilder source = XContentFactory.jsonBuilder().startObject().field("field1", "value").field("field2", "value2").endObject(); + HitContext hitContext = hitExecute(source, false, null, null); + assertNull(hitContext.hit().getSourceAsMap()); + + hitContext = hitExecute(source, true, "field1", "*"); + assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap()); + + hitContext = hitExecute(source, true, null, "*"); + assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap()); + + hitContext = hitExecute(source, true, "*", "*"); + assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap()); + + hitContext = hitExecuteMultiple(source, true, new String[] { "field1", "field2" }, new String[] { "*", "field1" }); + assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap()); + + hitContext = hitExecuteMultiple(source, true, null, new String[] { "field2", "*", "field1" }); + assertEquals(Collections.emptyMap(), hitContext.hit().getSourceAsMap()); + } + public void testMultipleFiltering() throws IOException { XContentBuilder source = XContentFactory.jsonBuilder().startObject().field("field", "value").field("field2", "value2").endObject(); HitContext hitContext = hitExecuteMultiple(source, true, new String[] { "*.notexisting", "field" }, null); From fed808850d708ba4be5190ac2abc3c47d8d2d379 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Fri, 10 May 2024 14:28:19 +0200 Subject: [PATCH 08/11] ES|QL: Add unit tests for now() function (#108498) --- .../functions/date-time-functions.asciidoc | 2 +- .../esql/functions/description/now.asciidoc | 5 ++ .../esql/functions/examples/now.asciidoc | 22 ++++++ .../esql/functions/kibana/definition/now.json | 16 +++++ .../esql/functions/kibana/docs/now.md | 10 +++ .../esql/functions/layout/now.asciidoc | 15 ++++ docs/reference/esql/functions/now.asciidoc | 28 -------- .../esql/functions/parameters/now.asciidoc | 3 + .../esql/functions/signature/now.svg | 1 + .../esql/functions/types/now.asciidoc | 9 +++ .../function/scalar/math/NowTests.java | 68 +++++++++++++++++++ 11 files changed, 150 insertions(+), 29 deletions(-) create mode 100644 docs/reference/esql/functions/description/now.asciidoc create mode 100644 docs/reference/esql/functions/examples/now.asciidoc create mode 100644 docs/reference/esql/functions/kibana/definition/now.json create mode 100644 docs/reference/esql/functions/kibana/docs/now.md create mode 100644 docs/reference/esql/functions/layout/now.asciidoc delete mode 100644 docs/reference/esql/functions/now.asciidoc create mode 100644 docs/reference/esql/functions/parameters/now.asciidoc create mode 100644 docs/reference/esql/functions/signature/now.svg create mode 100644 docs/reference/esql/functions/types/now.asciidoc create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/NowTests.java diff --git a/docs/reference/esql/functions/date-time-functions.asciidoc b/docs/reference/esql/functions/date-time-functions.asciidoc index 8ce26eaabe381..eceb6378426a2 100644 --- a/docs/reference/esql/functions/date-time-functions.asciidoc +++ b/docs/reference/esql/functions/date-time-functions.asciidoc @@ -21,4 +21,4 @@ include::layout/date_extract.asciidoc[] include::layout/date_format.asciidoc[] include::layout/date_parse.asciidoc[] include::layout/date_trunc.asciidoc[] -include::now.asciidoc[] +include::layout/now.asciidoc[] diff --git a/docs/reference/esql/functions/description/now.asciidoc b/docs/reference/esql/functions/description/now.asciidoc new file mode 100644 index 0000000000000..4852c98b4980a --- /dev/null +++ b/docs/reference/esql/functions/description/now.asciidoc @@ -0,0 +1,5 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Description* + +Returns current date and time. diff --git a/docs/reference/esql/functions/examples/now.asciidoc b/docs/reference/esql/functions/examples/now.asciidoc new file mode 100644 index 0000000000000..b8953de93724c --- /dev/null +++ b/docs/reference/esql/functions/examples/now.asciidoc @@ -0,0 +1,22 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Examples* + +[source.merge.styled,esql] +---- +include::{esql-specs}/date.csv-spec[tag=docsNow] +---- +[%header.monospaced.styled,format=dsv,separator=|] +|=== +include::{esql-specs}/date.csv-spec[tag=docsNow-result] +|=== +To retrieve logs from the last hour: +[source.merge.styled,esql] +---- +include::{esql-specs}/date.csv-spec[tag=docsNowWhere] +---- +[%header.monospaced.styled,format=dsv,separator=|] +|=== +include::{esql-specs}/date.csv-spec[tag=docsNowWhere-result] +|=== + diff --git a/docs/reference/esql/functions/kibana/definition/now.json b/docs/reference/esql/functions/kibana/definition/now.json new file mode 100644 index 0000000000000..9cdb4945afa2e --- /dev/null +++ b/docs/reference/esql/functions/kibana/definition/now.json @@ -0,0 +1,16 @@ +{ + "comment" : "This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.", + "type" : "eval", + "name" : "now", + "description" : "Returns current date and time.", + "signatures" : [ + { + "params" : [ ], + "returnType" : "datetime" + } + ], + "examples" : [ + "ROW current_date = NOW()", + "FROM sample_data\n| WHERE @timestamp > NOW() - 1 hour" + ] +} diff --git a/docs/reference/esql/functions/kibana/docs/now.md b/docs/reference/esql/functions/kibana/docs/now.md new file mode 100644 index 0000000000000..5143dc843ebd8 --- /dev/null +++ b/docs/reference/esql/functions/kibana/docs/now.md @@ -0,0 +1,10 @@ + + +### NOW +Returns current date and time. + +``` +ROW current_date = NOW() +``` diff --git a/docs/reference/esql/functions/layout/now.asciidoc b/docs/reference/esql/functions/layout/now.asciidoc new file mode 100644 index 0000000000000..52341c1665619 --- /dev/null +++ b/docs/reference/esql/functions/layout/now.asciidoc @@ -0,0 +1,15 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +[discrete] +[[esql-now]] +=== `NOW` + +*Syntax* + +[.text-center] +image::esql/functions/signature/now.svg[Embedded,opts=inline] + +include::../parameters/now.asciidoc[] +include::../description/now.asciidoc[] +include::../types/now.asciidoc[] +include::../examples/now.asciidoc[] diff --git a/docs/reference/esql/functions/now.asciidoc b/docs/reference/esql/functions/now.asciidoc deleted file mode 100644 index 3c46f557acd1f..0000000000000 --- a/docs/reference/esql/functions/now.asciidoc +++ /dev/null @@ -1,28 +0,0 @@ -[discrete] -[[esql-now]] -=== `NOW` - -*Syntax* - -[source,esql] ----- -NOW() ----- - -*Description* - -Returns current date and time. - -*Example* - -[source,esql] ----- -include::{esql-specs}/date.csv-spec[tag=docsNow] ----- - -To retrieve logs from the last hour: - -[source,esql] ----- -include::{esql-specs}/date.csv-spec[tag=docsNowWhere] ----- \ No newline at end of file diff --git a/docs/reference/esql/functions/parameters/now.asciidoc b/docs/reference/esql/functions/parameters/now.asciidoc new file mode 100644 index 0000000000000..25b3c973f1a26 --- /dev/null +++ b/docs/reference/esql/functions/parameters/now.asciidoc @@ -0,0 +1,3 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Parameters* diff --git a/docs/reference/esql/functions/signature/now.svg b/docs/reference/esql/functions/signature/now.svg new file mode 100644 index 0000000000000..2cd48ac561408 --- /dev/null +++ b/docs/reference/esql/functions/signature/now.svg @@ -0,0 +1 @@ +NOW() \ No newline at end of file diff --git a/docs/reference/esql/functions/types/now.asciidoc b/docs/reference/esql/functions/types/now.asciidoc new file mode 100644 index 0000000000000..5737d98f2f7db --- /dev/null +++ b/docs/reference/esql/functions/types/now.asciidoc @@ -0,0 +1,9 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Supported types* + +[%header.monospaced.styled,format=dsv,separator=|] +|=== +result +datetime +|=== diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/NowTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/NowTests.java new file mode 100644 index 0000000000000..b4f195c5929e3 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/NowTests.java @@ -0,0 +1,68 @@ +/* + * 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.esql.expression.function.scalar.math; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.xpack.esql.EsqlTestUtils; +import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; +import org.elasticsearch.xpack.esql.expression.function.scalar.AbstractConfigurationFunctionTestCase; +import org.elasticsearch.xpack.esql.expression.function.scalar.date.Now; +import org.elasticsearch.xpack.esql.session.EsqlConfiguration; +import org.elasticsearch.xpack.ql.expression.Expression; +import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataTypes; +import org.hamcrest.Matcher; + +import java.util.List; +import java.util.function.Supplier; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.matchesPattern; + +public class NowTests extends AbstractConfigurationFunctionTestCase { + public NowTests(@Name("TestCase") Supplier testCaseSupplier) { + this.testCase = testCaseSupplier.get(); + } + + @ParametersFactory + public static Iterable parameters() { + return parameterSuppliersFromTypedData( + List.of( + new TestCaseSupplier( + "Now Test", + () -> new TestCaseSupplier.TestCase( + List.of(), + matchesPattern("LiteralsEvaluator\\[lit=.*\\]"), + DataTypes.DATETIME, + equalTo(EsqlTestUtils.TEST_CFG.now().toInstant().toEpochMilli()) + ) + ) + ) + ); + } + + @Override + protected Expression buildWithConfiguration(Source source, List args, EsqlConfiguration configuration) { + return new Now(Source.EMPTY, configuration); + } + + @Override + protected void assertSimpleWithNulls(List data, Block value, int nullBlock) { + assertThat(((LongBlock) value).asVector().getLong(0), equalTo(EsqlTestUtils.TEST_CFG.now().toInstant().toEpochMilli())); + } + + @Override + protected Matcher allNullsMatcher() { + return equalTo(EsqlTestUtils.TEST_CFG.now().toInstant().toEpochMilli()); + } + +} From ac102e53f3d5eb318e682101b2060cba7ae90936 Mon Sep 17 00:00:00 2001 From: Dianna Hohensee Date: Fri, 10 May 2024 08:33:28 -0400 Subject: [PATCH 09/11] Improve join NotMasterException response, and add class documentation (#108107) The NotMasterException response to a join request is difficult to use to diagnose a failed join attempt. Enhancing the NotMasterException to include what node is thought to be master and the current term. This additional information will help readers locate the real master, to go look at those logs. The additional class documentation on JoinHelper and ClusterFormationFailureHelper should improve comprehension of the circumstances of error message logs. --- .../ClusterFormationFailureHelper.java | 23 ++++++++++++++- .../cluster/coordination/JoinHelper.java | 29 +++++++++++++++++-- .../coordination/NodeJoinExecutor.java | 10 ++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java index c2cd403836593..e81d8d73af9a2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterFormationFailureHelper.java @@ -43,9 +43,16 @@ import static org.elasticsearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.elasticsearch.monitor.StatusInfo.Status.UNHEALTHY; +/** + * Handles periodic debug logging of information regarding why the cluster has failed to form. + * Periodic logging begins once {@link #start()} is called, and ceases on {@link #stop()}. + */ public class ClusterFormationFailureHelper { private static final Logger logger = LogManager.getLogger(ClusterFormationFailureHelper.class); + /** + * This time period controls how often warning log messages will be written if this node fails to join or form a cluster. + */ public static final Setting DISCOVERY_CLUSTER_FORMATION_WARNING_TIMEOUT_SETTING = Setting.timeSetting( "discovery.cluster_formation_warning_timeout", TimeValue.timeValueMillis(10000), @@ -61,6 +68,16 @@ public class ClusterFormationFailureHelper { @Nullable // if no warning is scheduled private volatile WarningScheduler warningScheduler; + /** + * Works with the {@link JoinHelper} to log the latest node-join attempt failure and cluster state debug information. Must call + * {@link ClusterFormationState#start()} to begin. + * + * @param settings provides the period in which to log cluster formation errors. + * @param clusterFormationStateSupplier information about the current believed cluster state (See {@link ClusterFormationState}) + * @param threadPool the thread pool on which to run debug logging + * @param logLastFailedJoinAttempt invokes an instance of the JoinHelper to log the last encountered join failure + * (See {@link JoinHelper#logLastFailedJoinAttempt()}) + */ public ClusterFormationFailureHelper( Settings settings, Supplier clusterFormationStateSupplier, @@ -78,6 +95,10 @@ public boolean isRunning() { return warningScheduler != null; } + /** + * Schedules a warning debug message to be logged in 'clusterFormationWarningTimeout' time, and periodically thereafter, until + * {@link ClusterFormationState#stop()} has been called. + */ public void start() { assert warningScheduler == null; warningScheduler = new WarningScheduler(); @@ -129,7 +150,7 @@ public String toString() { } /** - * If this node believes that cluster formation has failed, this record provides information that can be used to determine why that is. + * This record provides node state information that can be used to determine why cluster formation has failed. */ public record ClusterFormationState( List initialMasterNodesSetting, diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java b/server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java index b960bb02ceb7f..059400ad81cfb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/JoinHelper.java @@ -194,13 +194,23 @@ private void unregisterAndReleaseConnection(DiscoveryNode destination, Releasabl Releasables.close(connectionReference); } - // package-private for testing + /** + * Saves information about a join failure. The failure information may be logged later via either {@link FailedJoinAttempt#logNow} + * or {@link FailedJoinAttempt#lastFailedJoinAttempt}. + * + * Package-private for testing. + */ static class FailedJoinAttempt { private final DiscoveryNode destination; private final JoinRequest joinRequest; private final ElasticsearchException exception; private final long timestamp; + /** + * @param destination the master node targeted by the join request. + * @param joinRequest the join request that was sent to the perceived master node. + * @param exception the error response received in reply to the join request attempt. + */ FailedJoinAttempt(DiscoveryNode destination, JoinRequest joinRequest, ElasticsearchException exception) { this.destination = destination; this.joinRequest = joinRequest; @@ -208,10 +218,18 @@ static class FailedJoinAttempt { this.timestamp = System.nanoTime(); } + /** + * Logs the failed join attempt exception. + * {@link FailedJoinAttempt#getLogLevel(ElasticsearchException)} determines at what log-level the log is written. + */ void logNow() { logger.log(getLogLevel(exception), () -> format("failed to join %s with %s", destination, joinRequest), exception); } + /** + * Returns the appropriate log level based on the given exception. Every error is at least DEBUG, but unexpected errors are INFO. + * For example, NotMasterException and CircuitBreakingExceptions are DEBUG logs. + */ static Level getLogLevel(ElasticsearchException e) { Throwable cause = e.unwrapCause(); if (cause instanceof CoordinationStateRejectedException @@ -226,6 +244,10 @@ void logWarnWithTimestamp() { logger.warn( () -> format( "last failed join attempt was %s ago, failed to join %s with %s", + // 'timestamp' is when this error exception was received by the local node. If the time that has passed since the error + // was originally received is quite large, it could indicate that this is a stale error exception from some prior + // out-of-order request response (where a later sent request but earlier received response was successful); or + // alternatively an old error could indicate that this node did not retry the join request for a very long time. TimeValue.timeValueMillis(TimeValue.nsecToMSec(System.nanoTime() - timestamp)), destination, joinRequest @@ -235,6 +257,9 @@ void logWarnWithTimestamp() { } } + /** + * Logs a warning message if {@link #lastFailedJoinAttempt} has been set with a failure. + */ void logLastFailedJoinAttempt() { FailedJoinAttempt attempt = lastFailedJoinAttempt.get(); if (attempt != null) { @@ -247,7 +272,7 @@ public void sendJoinRequest(DiscoveryNode destination, long term, Optional assert destination.isMasterNode() : "trying to join master-ineligible " + destination; final StatusInfo statusInfo = nodeHealthService.getHealth(); if (statusInfo.getStatus() == UNHEALTHY) { - logger.debug("dropping join request to [{}]: [{}]", destination, statusInfo.getInfo()); + logger.debug("dropping join request to [{}], unhealthy status: [{}]", destination, statusInfo.getInfo()); return; } final JoinRequest joinRequest = new JoinRequest( diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java b/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java index 2c024063e2399..9223e02fc946c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/NodeJoinExecutor.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.routing.allocation.AllocationService; import org.elasticsearch.cluster.version.CompatibilityVersions; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.Strings; import org.elasticsearch.features.FeatureService; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.IndexVersions; @@ -123,7 +124,14 @@ public ClusterState execute(BatchExecutionContext batchExecutionContex newState = ClusterState.builder(initialState); } else { logger.trace("processing node joins, but we are not the master. current master: {}", currentNodes.getMasterNode()); - throw new NotMasterException("Node [" + currentNodes.getLocalNode() + "] not master for join request"); + throw new NotMasterException( + Strings.format( + "Node [%s] not master for join request. Current known master [%s], current term [%d]", + currentNodes.getLocalNode(), + currentNodes.getMasterNode(), + term + ) + ); } DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(newState.nodes()); From 79032ec77eb5227bdb6eef37df7a4a6d35d98912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Witek?= Date: Fri, 10 May 2024 14:46:33 +0200 Subject: [PATCH 10/11] Do not use global ordinals strategy if the leaf reader context cannot be obtained (#108459) --- docs/changelog/108459.yaml | 6 ++++++ .../FrequentItemSetCollector.java | 12 +++++++----- .../mr/ItemSetMapReduceAggregator.java | 18 ++++++++---------- .../mr/ItemSetMapReduceValueSource.java | 12 +++++++----- 4 files changed, 28 insertions(+), 20 deletions(-) create mode 100644 docs/changelog/108459.yaml diff --git a/docs/changelog/108459.yaml b/docs/changelog/108459.yaml new file mode 100644 index 0000000000000..5e05797f284be --- /dev/null +++ b/docs/changelog/108459.yaml @@ -0,0 +1,6 @@ +pr: 108459 +summary: Do not use global ordinals strategy if the leaf reader context cannot be + obtained +area: Machine Learning +type: bug +issues: [] diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/FrequentItemSetCollector.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/FrequentItemSetCollector.java index 18086748d6fe0..bd80e362f2f71 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/FrequentItemSetCollector.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/FrequentItemSetCollector.java @@ -177,7 +177,8 @@ FrequentItemSet toFrequentItemSet(List fields) throws IOException { int pos = items.nextSetBit(0); while (pos > 0) { Tuple item = transactionStore.getItem(topItemIds.getItemIdAt(pos - 1)); - assert item.v1() < fields.size() : "item id exceed number of given items, did you configure eclat correctly?"; + assert item.v1() < fields.size() + : "eclat error: item id (" + item.v1() + ") exceeds the number of given items (" + fields.size() + ")"; final Field field = fields.get(item.v1()); Object formattedValue = field.formatValue(item.v2()); String fieldName = fields.get(item.v1()).getName(); @@ -252,19 +253,20 @@ public FrequentItemSetCollector(TransactionStore transactionStore, TopItemIds to this.topItemIds = topItemIds; this.size = size; this.min = min; - queue = new FrequentItemSetPriorityQueue(size); - frequentItemsByCount = Maps.newMapWithExpectedSize(size / 10); + this.queue = new FrequentItemSetPriorityQueue(size); + this.frequentItemsByCount = Maps.newMapWithExpectedSize(size / 10); } public FrequentItemSet[] finalizeAndGetResults(List fields) throws IOException { - FrequentItemSet[] topFrequentItems = new FrequentItemSet[size()]; + FrequentItemSet[] topFrequentItems = new FrequentItemSet[queue.size()]; for (int i = topFrequentItems.length - 1; i >= 0; i--) { topFrequentItems[i] = queue.pop().toFrequentItemSet(fields); } return topFrequentItems; } - public int size() { + // Visible for testing + int size() { return queue.size(); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceAggregator.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceAggregator.java index 72bfb6f1f0394..0f9555c77341f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceAggregator.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceAggregator.java @@ -86,17 +86,15 @@ protected ItemSetMapReduceAggregator( boolean rewriteBasedOnOrdinals = false; - if (ctx.isPresent()) { - for (var c : configsAndValueFilters) { - ItemSetMapReduceValueSource e = context.getValuesSourceRegistry() - .getAggregator(registryKey, c.v1()) - .build(c.v1(), id++, c.v2(), ordinalOptimization, ctx.get()); - if (e.getField().getName() != null) { - fields.add(e.getField()); - valueSources.add(e); - } - rewriteBasedOnOrdinals |= e.usesOrdinals(); + for (var c : configsAndValueFilters) { + ItemSetMapReduceValueSource e = context.getValuesSourceRegistry() + .getAggregator(registryKey, c.v1()) + .build(c.v1(), id++, c.v2(), ordinalOptimization, ctx); + if (e.getField().getName() != null) { + fields.add(e.getField()); + valueSources.add(e); } + rewriteBasedOnOrdinals |= e.usesOrdinals(); } this.rewriteBasedOnOrdinals = rewriteBasedOnOrdinals; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceValueSource.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceValueSource.java index c9ec772eb3321..08adecd3fbce5 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceValueSource.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/mr/ItemSetMapReduceValueSource.java @@ -37,6 +37,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Interface to extract values from Lucene in order to feed it into the MapReducer. @@ -53,7 +54,7 @@ ItemSetMapReduceValueSource build( int id, IncludeExclude includeExclude, AbstractItemSetMapReducer.OrdinalOptimization ordinalOptimization, - LeafReaderContext ctx + Optional ctx ) throws IOException; } @@ -345,20 +346,21 @@ public KeywordValueSource( int id, IncludeExclude includeExclude, AbstractItemSetMapReducer.OrdinalOptimization ordinalOptimization, - LeafReaderContext ctx + Optional ctx ) throws IOException { super(config, id, ValueFormatter.BYTES_REF); if (AbstractItemSetMapReducer.OrdinalOptimization.GLOBAL_ORDINALS.equals(ordinalOptimization) && config.getValuesSource() instanceof Bytes.WithOrdinals - && ((Bytes.WithOrdinals) config.getValuesSource()).supportsGlobalOrdinalsMapping()) { + && ((Bytes.WithOrdinals) config.getValuesSource()).supportsGlobalOrdinalsMapping() + && ctx.isPresent()) { logger.debug("Use ordinals for field [{}]", config.fieldContext().field()); this.executionStrategy = new GlobalOrdinalsStrategy( getField(), (Bytes.WithOrdinals) config.getValuesSource(), includeExclude == null ? null : includeExclude.convertToOrdinalsFilter(config.format()), - ctx + ctx.get() ); } else { this.executionStrategy = new MapStrategy( @@ -394,7 +396,7 @@ public NumericValueSource( int id, IncludeExclude includeExclude, AbstractItemSetMapReducer.OrdinalOptimization unusedOrdinalOptimization, - LeafReaderContext unusedCtx + Optional unusedCtx ) { super(config, id, ValueFormatter.LONG); this.source = (Numeric) config.getValuesSource(); From 8d19849dc10b28244b506c131cfe9db6e6c4372d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 May 2024 15:10:07 +0200 Subject: [PATCH 11/11] Fix potential leaks in search execution (#108391) Cleaning up some potentially leaky spots or at the very least making them easier to read. --- .../action/search/TransportSearchAction.java | 58 +++++++++++-------- .../search/internal/SearchContext.java | 1 + 2 files changed, 34 insertions(+), 25 deletions(-) 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 51a8c6ddb3d76..a12d149bbe342 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -1303,8 +1303,8 @@ public SearchPhase newSearchPhase( task, true, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), - listener.delegateFailureAndWrap((l, iters) -> { - SearchPhase action = newSearchPhase( + listener.delegateFailureAndWrap( + (l, iters) -> newSearchPhase( task, searchRequest, executor, @@ -1317,30 +1317,32 @@ public SearchPhase newSearchPhase( false, threadPool, clusters - ); - action.start(); - }) - ); - } else { - // for synchronous CCS minimize_roundtrips=false, use the CCSSingleCoordinatorSearchProgressListener - // (AsyncSearchTask will not return SearchProgressListener.NOOP, since it uses its own progress listener - // which delegates to CCSSingleCoordinatorSearchProgressListener when minimizing roundtrips) - if (clusters.isCcsMinimizeRoundtrips() == false - && clusters.hasRemoteClusters() - && task.getProgressListener() == SearchProgressListener.NOOP) { - task.setProgressListener(new CCSSingleCoordinatorSearchProgressListener()); - } - final SearchPhaseResults queryResultConsumer = searchPhaseController.newSearchPhaseResults( - executor, - circuitBreaker, - task::isCancelled, - task.getProgressListener(), - searchRequest, - shardIterators.size(), - exc -> searchTransportService.cancelSearchTask(task, "failed to merge result [" + exc.getMessage() + "]") + ).start() + ) ); + } + // for synchronous CCS minimize_roundtrips=false, use the CCSSingleCoordinatorSearchProgressListener + // (AsyncSearchTask will not return SearchProgressListener.NOOP, since it uses its own progress listener + // which delegates to CCSSingleCoordinatorSearchProgressListener when minimizing roundtrips) + if (clusters.isCcsMinimizeRoundtrips() == false + && clusters.hasRemoteClusters() + && task.getProgressListener() == SearchProgressListener.NOOP) { + task.setProgressListener(new CCSSingleCoordinatorSearchProgressListener()); + } + final SearchPhaseResults queryResultConsumer = searchPhaseController.newSearchPhaseResults( + executor, + circuitBreaker, + task::isCancelled, + task.getProgressListener(), + searchRequest, + shardIterators.size(), + exc -> searchTransportService.cancelSearchTask(task, "failed to merge result [" + exc.getMessage() + "]") + ); + boolean success = false; + try { + final SearchPhase searchPhase; if (searchRequest.searchType() == DFS_QUERY_THEN_FETCH) { - return new SearchDfsQueryThenFetchAsyncAction( + searchPhase = new SearchDfsQueryThenFetchAsyncAction( logger, namedWriteableRegistry, searchTransportService, @@ -1359,7 +1361,7 @@ public SearchPhase newSearchPhase( ); } else { assert searchRequest.searchType() == QUERY_THEN_FETCH : searchRequest.searchType(); - return new SearchQueryThenFetchAsyncAction( + searchPhase = new SearchQueryThenFetchAsyncAction( logger, namedWriteableRegistry, searchTransportService, @@ -1377,6 +1379,12 @@ public SearchPhase newSearchPhase( clusters ); } + success = true; + return searchPhase; + } finally { + if (success == false) { + queryResultConsumer.close(); + } } } } diff --git a/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java index 232c12e944a96..35f96ee2dc102 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -351,6 +351,7 @@ public Query rewrittenQuery() { * Adds a releasable that will be freed when this context is closed. */ public void addReleasable(Releasable releasable) { // TODO most Releasables are managed by their callers. We probably don't need this. + assert closed.get() == false; releasables.add(releasable); }