diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index b6b730fc3de8b..0c0ede8c3a076 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -78,11 +78,15 @@ if [[ "${USE_SNYK_CREDENTIALS:-}" == "true" ]]; then fi if [[ "${USE_PROD_DOCKER_CREDENTIALS:-}" == "true" ]]; then - DOCKER_REGISTRY_USERNAME="$(vault read -field=username secret/ci/elastic-elasticsearch/migrated/prod_docker_registry_credentials)" - export DOCKER_REGISTRY_USERNAME + if which docker > /dev/null 2>&1; then + DOCKER_REGISTRY_USERNAME="$(vault read -field=username secret/ci/elastic-elasticsearch/migrated/prod_docker_registry_credentials)" + export DOCKER_REGISTRY_USERNAME - DOCKER_REGISTRY_PASSWORD="$(vault read -field=password secret/ci/elastic-elasticsearch/migrated/prod_docker_registry_credentials)" - export DOCKER_REGISTRY_PASSWORD + DOCKER_REGISTRY_PASSWORD="$(vault read -field=password secret/ci/elastic-elasticsearch/migrated/prod_docker_registry_credentials)" + export DOCKER_REGISTRY_PASSWORD + + docker login --username "$DOCKER_REGISTRY_USERNAME" --password "$DOCKER_REGISTRY_PASSWORD" docker.elastic.co + fi fi if [[ "$BUILDKITE_AGENT_META_DATA_PROVIDER" != *"k8s"* ]]; then diff --git a/.buildkite/pipelines/periodic-packaging.yml b/.buildkite/pipelines/periodic-packaging.yml index 8ef8f5954887e..76cc543a6898e 100644 --- a/.buildkite/pipelines/periodic-packaging.yml +++ b/.buildkite/pipelines/periodic-packaging.yml @@ -30,7 +30,8 @@ steps: image: family/elasticsearch-{{matrix.image}} diskSizeGb: 350 machineType: n1-standard-8 - env: {} + env: + USE_PROD_DOCKER_CREDENTIALS: "true" - group: packaging-tests-upgrade steps: - label: "{{matrix.image}} / 8.0.1 / packaging-tests-upgrade" diff --git a/.ci/scripts/packaging-test.sh b/.ci/scripts/packaging-test.sh index 6b9938dabffa8..bb7547933b213 100755 --- a/.ci/scripts/packaging-test.sh +++ b/.ci/scripts/packaging-test.sh @@ -77,5 +77,6 @@ sudo -E env \ --unset=ES_JAVA_HOME \ --unset=JAVA_HOME \ SYSTEM_JAVA_HOME=`readlink -f -n $BUILD_JAVA_HOME` \ + DOCKER_CONFIG="${HOME}/.docker" \ ./gradlew -g $HOME/.gradle --scan --parallel --build-cache -Dorg.elasticsearch.build.cache.url=https://gradle-enterprise.elastic.co/cache/ --continue $@ diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/SortBench.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/SortBench.java index 423db48337586..4bec6a183fe94 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/SortBench.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/SortBench.java @@ -21,10 +21,12 @@ package org.elasticsearch.benchmark.tdigest; +import org.elasticsearch.common.breaker.NoopCircuitBreaker; +import org.elasticsearch.search.aggregations.metrics.MemoryTrackingTDigestArrays; import org.elasticsearch.tdigest.Sort; +import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; import org.elasticsearch.tdigest.arrays.TDigestIntArray; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -51,7 +53,8 @@ @State(Scope.Thread) public class SortBench { private final int size = 100000; - private final TDigestDoubleArray values = WrapperTDigestArrays.INSTANCE.newDoubleArray(size); + private final TDigestArrays arrays = new MemoryTrackingTDigestArrays(new NoopCircuitBreaker("default-wrapper-tdigest-arrays")); + private final TDigestDoubleArray values = arrays.newDoubleArray(size); @Param({ "0", "1", "-1" }) public int sortDirection; @@ -72,7 +75,7 @@ public void setup() { @Benchmark public void stableSort() { - TDigestIntArray order = WrapperTDigestArrays.INSTANCE.newIntArray(size); + TDigestIntArray order = arrays.newIntArray(size); for (int i = 0; i < size; i++) { order.set(i, i); } diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/TDigestBench.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/TDigestBench.java index 58bb5b07d22cd..36ffc34c482d7 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/TDigestBench.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/tdigest/TDigestBench.java @@ -21,9 +21,11 @@ package org.elasticsearch.benchmark.tdigest; +import org.elasticsearch.common.breaker.NoopCircuitBreaker; +import org.elasticsearch.search.aggregations.metrics.MemoryTrackingTDigestArrays; import org.elasticsearch.tdigest.MergingDigest; import org.elasticsearch.tdigest.TDigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; +import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -56,24 +58,25 @@ @Threads(1) @State(Scope.Thread) public class TDigestBench { + private static final TDigestArrays arrays = new MemoryTrackingTDigestArrays(new NoopCircuitBreaker("default-wrapper-tdigest-arrays")); public enum TDigestFactory { MERGE { @Override TDigest create(double compression) { - return new MergingDigest(WrapperTDigestArrays.INSTANCE, compression, (int) (10 * compression)); + return new MergingDigest(arrays, compression, (int) (10 * compression)); } }, AVL_TREE { @Override TDigest create(double compression) { - return TDigest.createAvlTreeDigest(WrapperTDigestArrays.INSTANCE, compression); + return TDigest.createAvlTreeDigest(arrays, compression); } }, HYBRID { @Override TDigest create(double compression) { - return TDigest.createHybridDigest(WrapperTDigestArrays.INSTANCE, compression); + return TDigest.createHybridDigest(arrays, compression); } }; diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/docker/DockerBuildTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/docker/DockerBuildTask.java index 8971f27838578..9b28401994ee2 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/docker/DockerBuildTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/docker/DockerBuildTask.java @@ -30,6 +30,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; import org.gradle.process.ExecOperations; +import org.gradle.process.ExecSpec; import org.gradle.workers.WorkAction; import org.gradle.workers.WorkParameters; import org.gradle.workers.WorkerExecutor; @@ -166,6 +167,7 @@ private void pullBaseImage(String baseImage) { for (int attempt = 1; attempt <= maxAttempts; attempt++) { try { LoggedExec.exec(execOperations, spec -> { + maybeConfigureDockerConfig(spec); spec.executable("docker"); spec.args("pull"); spec.args(baseImage); @@ -181,6 +183,13 @@ private void pullBaseImage(String baseImage) { throw new GradleException("Failed to pull Docker base image [" + baseImage + "], all attempts failed"); } + private void maybeConfigureDockerConfig(ExecSpec spec) { + String dockerConfig = System.getenv("DOCKER_CONFIG"); + if (dockerConfig != null) { + spec.environment("DOCKER_CONFIG", dockerConfig); + } + } + @Override public void execute() { final Parameters parameters = getParameters(); @@ -193,6 +202,8 @@ public void execute() { final boolean isCrossPlatform = isCrossPlatform(); LoggedExec.exec(execOperations, spec -> { + maybeConfigureDockerConfig(spec); + spec.executable("docker"); if (isCrossPlatform) { diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 47f79749cbefa..fd2516f2fdc9a 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -163,9 +163,16 @@ RUN <%= retry.loop(package_manager, " ${package_manager} update && \n" + " ${package_manager} upgrade && \n" + " ${package_manager} add --no-cache \n" + - " bash ca-certificates curl libsystemd netcat-openbsd p11-kit p11-kit-trust shadow tini unzip zip zstd && \n" + + " bash java-cacerts curl libstdc++ libsystemd netcat-openbsd p11-kit p11-kit-trust posix-libc-utils shadow tini unzip zip zstd && \n" + " rm -rf /var/cache/apk/* " ) %> + +# Set Bash as the default shell for future commands +SHELL ["/bin/bash", "-c"] + +# Optionally set Bash as the default shell in the container at runtime +CMD ["/bin/bash"] + <% } else if (docker_base == "default" || docker_base == "cloud") { %> # Change default shell to bash, then install required packages with retries. @@ -224,7 +231,7 @@ COPY --from=builder --chown=0:0 /opt /opt <% } %> ENV PATH /usr/share/elasticsearch/bin:\$PATH - +ENV SHELL /bin/bash COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh # 1. Sync the user and group permissions of /etc/passwd @@ -249,6 +256,8 @@ RUN chmod g=u /etc/passwd && \\ # stays up-to-date with changes to Ubuntu's store) COPY bin/docker-openjdk /etc/ca-certificates/update.d/docker-openjdk RUN /etc/ca-certificates/update.d/docker-openjdk +<% } else if (docker_base == 'wolfi') { %> +RUN ln -sf /etc/ssl/certs/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts <% } else { %> RUN ln -sf /etc/pki/ca-trust/extracted/java/cacerts /usr/share/elasticsearch/jdk/lib/security/cacerts <% } %> diff --git a/docs/changelog/112092.yaml b/docs/changelog/112092.yaml new file mode 100644 index 0000000000000..35c731074d760 --- /dev/null +++ b/docs/changelog/112092.yaml @@ -0,0 +1,5 @@ +pr: 112092 +summary: "Apply auto-flattening to `subobjects: auto`" +area: Mapping +type: enhancement +issues: [] diff --git a/docs/changelog/112761.yaml b/docs/changelog/112761.yaml new file mode 100644 index 0000000000000..fe63f38f365a4 --- /dev/null +++ b/docs/changelog/112761.yaml @@ -0,0 +1,6 @@ +pr: 112761 +summary: Fix collapse interaction with stored fields +area: Search +type: bug +issues: + - 112646 diff --git a/docs/reference/inference/delete-inference.asciidoc b/docs/reference/inference/delete-inference.asciidoc index 4df72ba672092..bee39bf9b9851 100644 --- a/docs/reference/inference/delete-inference.asciidoc +++ b/docs/reference/inference/delete-inference.asciidoc @@ -49,13 +49,12 @@ The type of {infer} task that the model performs. `dry_run`:: (Optional, Boolean) -When `true`, checks the {infer} processors that reference the endpoint and -returns them in a list, but does not delete the endpoint. Defaults to `false`. +When `true`, checks the `semantic_text` fields and {infer} processors that reference the endpoint and returns them in a list, but does not delete the endpoint. +Defaults to `false`. `force`:: (Optional, Boolean) -Deletes the endpoint regardless if it's used in an {infer} pipeline or in a -`semantic_text` field. +Deletes the endpoint regardless if it's used in a `semantic_text` field or in an {infer} pipeline. [discrete] diff --git a/docs/reference/ingest/processors/inference.asciidoc b/docs/reference/ingest/processors/inference.asciidoc index c942959d34e53..fa4f246cdd7c8 100644 --- a/docs/reference/ingest/processors/inference.asciidoc +++ b/docs/reference/ingest/processors/inference.asciidoc @@ -455,6 +455,29 @@ include::{es-ref-dir}/ml/ml-shared.asciidoc[tag=inference-config-nlp-tokenizatio ======= ===== +[discrete] +[[inference-processor-text-similarity-opt]] +==== Text similarity configuration options + +`text_similarity`::: +(Object, optional) +include::{es-ref-dir}/ml/ml-shared.asciidoc[tag=inference-config-text-similarity] ++ +.Properties of text_similarity inference +[%collapsible%open] +===== +`span_score_combination_function`:::: +(Optional, string) +include::{es-ref-dir}/ml/ml-shared.asciidoc[tag=inference-config-text-similarity-span-score-func] + +`tokenization`:::: +(Optional, object) +include::{es-ref-dir}/ml/ml-shared.asciidoc[tag=inference-config-nlp-tokenization] ++ +Refer to <> to review the properties of the +`tokenization` object. +===== + [discrete] [[inference-processor-zero-shot-opt]] diff --git a/docs/reference/mapping/params/subobjects.asciidoc b/docs/reference/mapping/params/subobjects.asciidoc index b0a5d3817c332..63e8e3c2db3fe 100644 --- a/docs/reference/mapping/params/subobjects.asciidoc +++ b/docs/reference/mapping/params/subobjects.asciidoc @@ -10,7 +10,7 @@ where for instance a field `metrics.time` holds a value too, which is common whe A document holding a value for both `metrics.time.max` and `metrics.time` gets rejected given that `time` would need to be a leaf field to hold a value as well as an object to hold the `max` sub-field. -The `subobjects` setting, which can be applied only to the top-level mapping definition and +The `subobjects: false` setting, which can be applied only to the top-level mapping definition and to <> fields, disables the ability for an object to hold further subobjects and makes it possible to store documents where field names contain dots and share common prefixes. From the example above, if the object container `metrics` has `subobjects` set to `false`, it can hold values for both `time` and `time.max` directly @@ -109,26 +109,138 @@ PUT my-index-000001/_doc/metric_1 <1> The entire mapping is configured to not support objects. <2> The document does not support objects +Setting `subobjects: false` disallows the definition of <> and <> sub-fields, which +can be too restrictive in cases where it's desirable to have <> objects or sub-objects with specific +behavior (e.g. with `enabled:false`). In this case, it's possible to set `subobjects: auto`, which +<> whenever possible and falls back to creating an object mapper otherwise (instead of +rejecting the mapping as `subobjects: false` does). For instance: + +[source,console] +-------------------------------------------------- +PUT my-index-000002 +{ + "mappings": { + "properties": { + "metrics": { + "type": "object", + "subobjects": "auto", <1> + "properties": { + "inner": { + "type": "object", + "enabled": false + }, + "nested": { + "type": "nested" + } + } + } + } + } +} + +PUT my-index-000002/_doc/metric_1 +{ + "metrics.time" : 100, <2> + "metrics.time.min" : 10, + "metrics.time.max" : 900 +} + +PUT my-index-000002/_doc/metric_2 +{ + "metrics" : { <3> + "time" : 100, + "time.min" : 10, + "time.max" : 900, + "inner": { + "foo": "bar", + "path.to.some.field": "baz" + }, + "nested": [ + { "id": 10 }, + { "id": 1 } + ] + } +} + +GET my-index-000002/_mapping +-------------------------------------------------- + +[source,console-result] +-------------------------------------------------- +{ + "my-index-000002" : { + "mappings" : { + "properties" : { + "metrics" : { + "subobjects" : auto, + "properties" : { + "inner": { <4> + "type": "object", + "enabled": false + }, + "nested": { + "type": "nested", + "properties" : { + "id" : { + "type" : "long" + } + } + }, + "time" : { + "type" : "long" + }, + "time.min" : { + "type" : "long" + }, + "time.max" : { + "type" : "long" + } + } + } + } + } + } +} +-------------------------------------------------- + +<1> The `metrics` field can only hold statically defined objects, namely `inner` and `nested`. +<2> Sample document holding flat paths +<3> Sample document holding an object (configured with sub-objects) and its leaf sub-fields +<4> The resulting mapping where dots in field names (`time.min`, `time_max`), as well as the +statically-defined sub-objects `inner` and `nested`, were preserved + The `subobjects` setting for existing fields and the top-level mapping definition cannot be updated. +[[auto-flattening]] ==== Auto-flattening object mappings -It is generally recommended to define the properties of an object that is configured with `subobjects: false` with dotted field names -(as shown in the first example). -However, it is also possible to define these properties as sub-objects in the mappings. -In that case, the mapping will be automatically flattened before it is stored. -This makes it easier to re-use existing mappings without having to re-write them. +It is generally recommended to define the properties of an object that is configured with `subobjects: false` or +`subobjects: auto` with dotted field names (as shown in the first example). However, it is also possible to define +these properties as sub-objects in the mappings. In that case, the mapping will be automatically flattened before +it is stored. This makes it easier to re-use existing mappings without having to re-write them. + +Note that auto-flattening does not apply if any of the following <> are set +on object mappings that are defined under an object configured with `subobjects: false` or `subobjects: auto`: -Note that auto-flattening will not work when certain <> are set -on object mappings that are defined under an object configured with `subobjects: false`: +* The <> mapping parameter is `false`. +* The <> mapping parameter contradicts the implicit or explicit value of the parent. +For example, when `dynamic` is set to `false` in the root of the mapping, object mappers that set `dynamic` to `true` +can't be auto-flattened. +* The <> mapping parameter is set to `auto` or `true` explicitly. -* The <> mapping parameter must not be `false`. -* The <> mapping parameter must not contradict the implicit or explicit value of the parent. For example, when `dynamic` is set to `false` in the root of the mapping, object mappers that set `dynamic` to `true` can't be auto-flattened. -* The <> mapping parameter must not be set to `true` explicitly. +If such a sub-object is detected, the behavior depends on the `subobjects` value: + +* `subobjects: false` is not compatible, so a mapping error is returned during mapping construction. +* `subobjects: auto` reverts to adding the object to the mapping, bypassing auto-flattening for it. Still, any +intermediate objects will be auto-flattened if applicable (i.e. the object name gets directly attached under the parent +object with `subobjects: auto`). Auto-flattening can be applied within sub-objects, if they are configured with +`subobjects: auto` too. + +Auto-flattening example with `subobjects: false`: [source,console] -------------------------------------------------- -PUT my-index-000002 +PUT my-index-000003 { "mappings": { "properties": { @@ -147,13 +259,13 @@ PUT my-index-000002 } } } -GET my-index-000002/_mapping +GET my-index-000003/_mapping -------------------------------------------------- [source,console-result] -------------------------------------------------- { - "my-index-000002" : { + "my-index-000003" : { "mappings" : { "properties" : { "metrics" : { @@ -175,5 +287,85 @@ GET my-index-000002/_mapping <1> The metrics object can contain further object mappings that will be auto-flattened. Object mappings at this level must not set certain mapping parameters as explained above. -<2> This field will be auto-flattened to `"time.min"` before the mapping is stored. -<3> The auto-flattened `"time.min"` field can be inspected by looking at the index mapping. +<2> This field will be auto-flattened to `time.min` before the mapping is stored. +<3> The auto-flattened `time.min` field can be inspected by looking at the index mapping. + +Auto-flattening example with `subobjects: auto`: + +[source,console] +-------------------------------------------------- +PUT my-index-000004 +{ + "mappings": { + "properties": { + "metrics": { + "subobjects": "auto", + "properties": { + "time": { + "type": "object", <1> + "properties": { + "min": { "type": "long" } <2> + } + }, + "to": { + "type": "object", + "properties": { + "inner_metrics": { <3> + "type": "object", + "subobjects": "auto", + "properties": { + "time": { + "type": "object", + "properties": { + "max": { "type": "long" } <4> + } + } + } + } + } + } + } + } + } + } +} +GET my-index-000004/_mapping +-------------------------------------------------- + +[source,console-result] +-------------------------------------------------- +{ + "my-index-000004" : { + "mappings" : { + "properties" : { + "metrics" : { + "subobjects" : "auto", + "properties" : { + "time.min" : { <5> + "type" : "long" + }, + "to.inner_metrics" : { <6> + "subobjects" : "auto", + "properties" : { + "time.max" : { <7> + "type" : "long" + } + } + } + } + } + } + } + } +} +-------------------------------------------------- + +<1> The metrics object can contain further object mappings that may be auto-flattened, depending on their mapping +parameters as explained above. +<2> This field will be auto-flattened to `time.min` before the mapping is stored. +<3> This object has param `subobjects: auto` so it can't be auto-flattened. Its parent does qualify for auto-flattening, +so it becomes `to.inner_metrics` before the mapping is stored. +<4> This field will be auto-flattened to `time.max` before the mapping is stored. +<5> The auto-flattened `time.min` field can be inspected by looking at the index mapping. +<6> The inner object `to.inner_metrics` can be inspected by looking at the index mapping. +<7> The auto-flattened `time.max` field can be inspected by looking at the index mapping. diff --git a/docs/reference/mapping/types/semantic-text.asciidoc b/docs/reference/mapping/types/semantic-text.asciidoc index a006f288dc66d..d0fdf0145aa58 100644 --- a/docs/reference/mapping/types/semantic-text.asciidoc +++ b/docs/reference/mapping/types/semantic-text.asciidoc @@ -14,9 +14,8 @@ The `semantic_text` field type specifies an inference endpoint identifier that w You can create the inference endpoint by using the <>. This field type and the <> type make it simpler to perform semantic search on your data. -Using `semantic_text`, you won't need to specify how to generate embeddings for -your data, or how to index it. The inference endpoint automatically determines -the embedding generation, indexing, and query to use. +Using `semantic_text`, you won't need to specify how to generate embeddings for your data, or how to index it. +The {infer} endpoint automatically determines the embedding generation, indexing, and query to use. [source,console] ------------------------------------------------------------ @@ -32,7 +31,29 @@ PUT my-index-000001 } } ------------------------------------------------------------ -// TEST[skip:TBD] +// TEST[skip:Requires inference endpoint] + + +The recommended way to use semantic_text is by having dedicated {infer} endpoints for ingestion and search. +This ensures that search speed remains unaffected by ingestion workloads, and vice versa. +After creating dedicated {infer} endpoints for both, you can reference them using the `inference_id` and `search_inference_id` parameters when setting up the index mapping for an index that uses the `semantic_text` field. + +[source,console] +------------------------------------------------------------ +PUT my-index-000002 +{ + "mappings": { + "properties": { + "inference_field": { + "type": "semantic_text", + "inference_id": "my-elser-endpoint-for-ingest", + "search_inference_id": "my-elser-endpoint-for-search" + } + } + } +} +------------------------------------------------------------ +// TEST[skip:Requires inference endpoint] [discrete] @@ -41,9 +62,15 @@ PUT my-index-000001 `inference_id`:: (Required, string) -Inference endpoint that will be used to generate the embeddings for the field. +{infer-cap} endpoint that will be used to generate the embeddings for the field. Use the <> to create the endpoint. +If `search_inference_id` is specified, the {infer} endpoint defined by `inference_id` will only be used at index time. +`search_inference_id`:: +(Optional, string) +{infer-cap} endpoint that will be used to generate embeddings at query time. +Use the <> to create the endpoint. +If not specified, the {infer} endpoint defined by `inference_id` will be used at both index and query time. [discrete] [[infer-endpoint-validation]] @@ -55,6 +82,7 @@ When the first document is indexed, the `inference_id` will be used to generate WARNING: Removing an {infer} endpoint will cause ingestion of documents and semantic queries to fail on indices that define `semantic_text` fields with that {infer} endpoint as their `inference_id`. Trying to <> that is used on a `semantic_text` field will result in an error. + [discrete] [[auto-text-chunking]] ==== Automatic text chunking @@ -183,6 +211,7 @@ PUT test-index/_bulk Notice that both the `semantic_text` field and the source field are updated in the bulk request. + [discrete] [[limitations]] ==== Limitations diff --git a/libs/tdigest/build.gradle b/libs/tdigest/build.gradle index 771df2e83d85d..df60862b27386 100644 --- a/libs/tdigest/build.gradle +++ b/libs/tdigest/build.gradle @@ -22,6 +22,8 @@ apply plugin: 'elasticsearch.build' apply plugin: 'elasticsearch.publish' dependencies { + api project(':libs:elasticsearch-core') + testImplementation(project(":test:framework")) { exclude group: 'org.elasticsearch', module: 'elasticsearch-tdigest' } diff --git a/libs/tdigest/src/main/java/module-info.java b/libs/tdigest/src/main/java/module-info.java index 8edaff3f31d8c..cc7ff1810905f 100644 --- a/libs/tdigest/src/main/java/module-info.java +++ b/libs/tdigest/src/main/java/module-info.java @@ -18,6 +18,8 @@ */ module org.elasticsearch.tdigest { + requires org.elasticsearch.base; + exports org.elasticsearch.tdigest; exports org.elasticsearch.tdigest.arrays; } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLGroupTree.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLGroupTree.java index 8528db2128729..a1a65e1e71cde 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLGroupTree.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLGroupTree.java @@ -21,6 +21,8 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; import org.elasticsearch.tdigest.arrays.TDigestLongArray; @@ -31,7 +33,7 @@ /** * A tree of t-digest centroids. */ -final class AVLGroupTree extends AbstractCollection { +final class AVLGroupTree extends AbstractCollection implements Releasable { /* For insertions into the tree */ private double centroid; private long count; @@ -267,4 +269,8 @@ private void checkAggregates(int node) { } } + @Override + public void close() { + Releasables.close(centroids, counts, aggregatedCounts, tree); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLTreeDigest.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLTreeDigest.java index c28f86b9b8edc..f6b027edb1e9c 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLTreeDigest.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/AVLTreeDigest.java @@ -21,6 +21,7 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import java.util.Collection; @@ -153,26 +154,27 @@ public void compress() { } needsCompression = false; - AVLGroupTree centroids = summary; - this.summary = new AVLGroupTree(arrays); + try (AVLGroupTree centroids = summary) { + this.summary = new AVLGroupTree(arrays); - final int[] nodes = new int[centroids.size()]; - nodes[0] = centroids.first(); - for (int i = 1; i < nodes.length; ++i) { - nodes[i] = centroids.next(nodes[i - 1]); - assert nodes[i] != IntAVLTree.NIL; - } - assert centroids.next(nodes[nodes.length - 1]) == IntAVLTree.NIL; + final int[] nodes = new int[centroids.size()]; + nodes[0] = centroids.first(); + for (int i = 1; i < nodes.length; ++i) { + nodes[i] = centroids.next(nodes[i - 1]); + assert nodes[i] != IntAVLTree.NIL; + } + assert centroids.next(nodes[nodes.length - 1]) == IntAVLTree.NIL; - for (int i = centroids.size() - 1; i > 0; --i) { - final int other = gen.nextInt(i + 1); - final int tmp = nodes[other]; - nodes[other] = nodes[i]; - nodes[i] = tmp; - } + for (int i = centroids.size() - 1; i > 0; --i) { + final int other = gen.nextInt(i + 1); + final int tmp = nodes[other]; + nodes[other] = nodes[i]; + nodes[i] = tmp; + } - for (int node : nodes) { - add(centroids.mean(node), centroids.count(node)); + for (int node : nodes) { + add(centroids.mean(node), centroids.count(node)); + } } } @@ -356,4 +358,9 @@ public int byteSize() { compress(); return 64 + summary.size() * 13; } + + @Override + public void close() { + Releasables.close(summary); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/HybridDigest.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/HybridDigest.java index c28a99fbd6d44..8d03ce4e303a6 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/HybridDigest.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/HybridDigest.java @@ -19,6 +19,7 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import java.util.Collection; @@ -110,6 +111,7 @@ public void reserve(long size) { } mergingDigest.reserve(size); // Release the allocated SortingDigest. + sortingDigest.close(); sortingDigest = null; } else { sortingDigest.reserve(size); @@ -196,4 +198,9 @@ public int byteSize() { } return sortingDigest.byteSize(); } + + @Override + public void close() { + Releasables.close(sortingDigest, mergingDigest); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/IntAVLTree.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/IntAVLTree.java index cda8aecdb2ccc..b4a82257693d8 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/IntAVLTree.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/IntAVLTree.java @@ -21,6 +21,8 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.elasticsearch.tdigest.arrays.TDigestByteArray; import org.elasticsearch.tdigest.arrays.TDigestIntArray; @@ -33,7 +35,7 @@ * want to add data to the nodes, typically by using arrays and node * identifiers as indices. */ -abstract class IntAVLTree { +abstract class IntAVLTree implements Releasable { /** * We use 0 instead of -1 so that left(NIL) works without * condition. @@ -586,4 +588,8 @@ int size() { } + @Override + public void close() { + Releasables.close(parent, left, right, depth); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/MergingDigest.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/MergingDigest.java index 1649af041ee19..f2ccfc33aa2a9 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/MergingDigest.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/MergingDigest.java @@ -21,6 +21,7 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; import org.elasticsearch.tdigest.arrays.TDigestIntArray; @@ -66,8 +67,6 @@ * what the AVLTreeDigest uses and no dynamic allocation is required at all. */ public class MergingDigest extends AbstractTDigest { - private final TDigestArrays arrays; - private int mergeCount = 0; private final double publicCompression; @@ -138,8 +137,6 @@ public MergingDigest(TDigestArrays arrays, double compression, int bufferSize) { * @param size Size of main buffer */ public MergingDigest(TDigestArrays arrays, double compression, int bufferSize, int size) { - this.arrays = arrays; - // ensure compression >= 10 // default size = 2 * ceil(compression) // default bufferSize = 5 * size @@ -274,9 +271,6 @@ private void merge( incomingWeight.set(incomingCount, weight, 0, lastUsedCell); incomingCount += lastUsedCell; - if (incomingOrder == null) { - incomingOrder = arrays.newIntArray(incomingCount); - } Sort.stableSort(incomingOrder, incomingMean, incomingCount); totalWeight += unmergedWeight; @@ -581,4 +575,9 @@ public String toString() { + "-" + (useTwoLevelCompression ? "twoLevel" : "oneLevel"); } + + @Override + public void close() { + Releasables.close(weight, mean, tempWeight, tempMean, order); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/SortingDigest.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/SortingDigest.java index 94b5c667e0672..f063ca9a511c6 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/SortingDigest.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/SortingDigest.java @@ -19,6 +19,7 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; @@ -137,4 +138,9 @@ public void reserve(long size) { public int byteSize() { return values.size() * 8; } + + @Override + public void close() { + Releasables.close(values); + } } diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/TDigest.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/TDigest.java index 4e79f9e68cd02..e578a688738cb 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/TDigest.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/TDigest.java @@ -21,6 +21,7 @@ package org.elasticsearch.tdigest; +import org.elasticsearch.core.Releasable; import org.elasticsearch.tdigest.arrays.TDigestArrays; import java.util.Collection; @@ -37,7 +38,7 @@ * - test coverage roughly at 90% * - easy to adapt for use with map-reduce */ -public abstract class TDigest { +public abstract class TDigest implements Releasable { protected ScaleFunction scale = ScaleFunction.K_2; double min = Double.POSITIVE_INFINITY; double max = Double.NEGATIVE_INFINITY; diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestByteArray.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestByteArray.java index 481dde9784008..ae8e84800b433 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestByteArray.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestByteArray.java @@ -21,10 +21,12 @@ package org.elasticsearch.tdigest.arrays; +import org.elasticsearch.core.Releasable; + /** * Minimal interface for ByteArray-like classes used within TDigest. */ -public interface TDigestByteArray { +public interface TDigestByteArray extends Releasable { int size(); byte get(int index); diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestDoubleArray.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestDoubleArray.java index 92530db5e7dc4..1699dbd9beaf1 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestDoubleArray.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestDoubleArray.java @@ -21,10 +21,12 @@ package org.elasticsearch.tdigest.arrays; +import org.elasticsearch.core.Releasable; + /** * Minimal interface for DoubleArray-like classes used within TDigest. */ -public interface TDigestDoubleArray { +public interface TDigestDoubleArray extends Releasable { int size(); double get(int index); diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestIntArray.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestIntArray.java index c944a4f8faf07..44e366aacd173 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestIntArray.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestIntArray.java @@ -21,10 +21,12 @@ package org.elasticsearch.tdigest.arrays; +import org.elasticsearch.core.Releasable; + /** * Minimal interface for IntArray-like classes used within TDigest. */ -public interface TDigestIntArray { +public interface TDigestIntArray extends Releasable { int size(); int get(int index); diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestLongArray.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestLongArray.java index 7e75dd512e86d..5deea6b28b1ed 100644 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestLongArray.java +++ b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/TDigestLongArray.java @@ -21,10 +21,12 @@ package org.elasticsearch.tdigest.arrays; +import org.elasticsearch.core.Releasable; + /** * Minimal interface for LongArray-like classes used within TDigest. */ -public interface TDigestLongArray { +public interface TDigestLongArray extends Releasable { int size(); long get(int index); diff --git a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/WrapperTDigestArrays.java b/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/WrapperTDigestArrays.java deleted file mode 100644 index ce2dd4f8d8e1d..0000000000000 --- a/libs/tdigest/src/main/java/org/elasticsearch/tdigest/arrays/WrapperTDigestArrays.java +++ /dev/null @@ -1,258 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * - * This project is based on a modification of https://github.com/tdunning/t-digest which is licensed under the Apache 2.0 License. - */ - -package org.elasticsearch.tdigest.arrays; - -import java.util.Arrays; - -/** - * Temporal TDigestArrays with raw arrays. - * - *

- * Delete after the right implementation for BigArrays is made. - *

- */ -public class WrapperTDigestArrays implements TDigestArrays { - - public static final WrapperTDigestArrays INSTANCE = new WrapperTDigestArrays(); - - private WrapperTDigestArrays() {} - - @Override - public WrapperTDigestDoubleArray newDoubleArray(int initialCapacity) { - return new WrapperTDigestDoubleArray(initialCapacity); - } - - @Override - public WrapperTDigestIntArray newIntArray(int initialSize) { - return new WrapperTDigestIntArray(initialSize); - } - - @Override - public TDigestLongArray newLongArray(int initialSize) { - return new WrapperTDigestLongArray(initialSize); - } - - @Override - public TDigestByteArray newByteArray(int initialSize) { - return new WrapperTDigestByteArray(initialSize); - } - - public WrapperTDigestDoubleArray newDoubleArray(double[] array) { - return new WrapperTDigestDoubleArray(array); - } - - public WrapperTDigestIntArray newIntArray(int[] array) { - return new WrapperTDigestIntArray(array); - } - - public static class WrapperTDigestDoubleArray implements TDigestDoubleArray { - private double[] array; - private int size; - - public WrapperTDigestDoubleArray(int initialSize) { - this(new double[initialSize]); - } - - public WrapperTDigestDoubleArray(double[] array) { - this.array = array; - this.size = array.length; - } - - @Override - public int size() { - return size; - } - - @Override - public double get(int index) { - assert index >= 0 && index < size; - return array[index]; - } - - @Override - public void set(int index, double value) { - assert index >= 0 && index < size; - array[index] = value; - } - - @Override - public void add(double value) { - ensureCapacity(size + 1); - array[size++] = value; - } - - @Override - public void sort() { - Arrays.sort(array, 0, size); - } - - @Override - public void ensureCapacity(int requiredCapacity) { - if (requiredCapacity > array.length) { - int newSize = array.length + (array.length >> 1); - if (newSize < requiredCapacity) { - newSize = requiredCapacity; - } - double[] newArray = new double[newSize]; - System.arraycopy(array, 0, newArray, 0, size); - array = newArray; - } - } - - @Override - public void resize(int newSize) { - if (newSize > array.length) { - array = Arrays.copyOf(array, newSize); - } - if (newSize > size) { - Arrays.fill(array, size, newSize, 0); - } - size = newSize; - } - } - - public static class WrapperTDigestIntArray implements TDigestIntArray { - private int[] array; - private int size; - - public WrapperTDigestIntArray(int initialSize) { - this(new int[initialSize]); - } - - public WrapperTDigestIntArray(int[] array) { - this.array = array; - this.size = array.length; - } - - @Override - public int size() { - return size; - } - - @Override - public int get(int index) { - assert index >= 0 && index < size; - return array[index]; - } - - @Override - public void set(int index, int value) { - assert index >= 0 && index < size; - array[index] = value; - } - - @Override - public void resize(int newSize) { - if (newSize > array.length) { - array = Arrays.copyOf(array, newSize); - } - if (newSize > size) { - Arrays.fill(array, size, newSize, 0); - } - size = newSize; - } - } - - public static class WrapperTDigestLongArray implements TDigestLongArray { - private long[] array; - private int size; - - public WrapperTDigestLongArray(int initialSize) { - this(new long[initialSize]); - } - - public WrapperTDigestLongArray(long[] array) { - this.array = array; - this.size = array.length; - } - - @Override - public int size() { - return size; - } - - @Override - public long get(int index) { - assert index >= 0 && index < size; - return array[index]; - } - - @Override - public void set(int index, long value) { - assert index >= 0 && index < size; - array[index] = value; - } - - @Override - public void resize(int newSize) { - if (newSize > array.length) { - array = Arrays.copyOf(array, newSize); - } - if (newSize > size) { - Arrays.fill(array, size, newSize, 0); - } - size = newSize; - } - } - - public static class WrapperTDigestByteArray implements TDigestByteArray { - private byte[] array; - private int size; - - public WrapperTDigestByteArray(int initialSize) { - this(new byte[initialSize]); - } - - public WrapperTDigestByteArray(byte[] array) { - this.array = array; - this.size = array.length; - } - - @Override - public int size() { - return size; - } - - @Override - public byte get(int index) { - assert index >= 0 && index < size; - return array[index]; - } - - @Override - public void set(int index, byte value) { - assert index >= 0 && index < size; - array[index] = value; - } - - @Override - public void resize(int newSize) { - if (newSize > array.length) { - array = Arrays.copyOf(array, newSize); - } - if (newSize > size) { - Arrays.fill(array, size, newSize, (byte) 0); - } - size = newSize; - } - } -} diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLGroupTreeTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLGroupTreeTests.java index 71be849f401f4..7ac55afd87808 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLGroupTreeTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLGroupTreeTests.java @@ -21,13 +21,10 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; - -public class AVLGroupTreeTests extends ESTestCase { +public class AVLGroupTreeTests extends TDigestTestCase { public void testSimpleAdds() { - AVLGroupTree x = new AVLGroupTree(WrapperTDigestArrays.INSTANCE); + AVLGroupTree x = new AVLGroupTree(arrays()); assertEquals(IntAVLTree.NIL, x.floor(34)); assertEquals(IntAVLTree.NIL, x.first()); assertEquals(IntAVLTree.NIL, x.last()); @@ -46,7 +43,7 @@ public void testSimpleAdds() { } public void testBalancing() { - AVLGroupTree x = new AVLGroupTree(WrapperTDigestArrays.INSTANCE); + AVLGroupTree x = new AVLGroupTree(arrays()); for (int i = 0; i < 101; i++) { x.add(new Centroid(i)); } @@ -60,7 +57,7 @@ public void testBalancing() { public void testFloor() { // mostly tested in other tests - AVLGroupTree x = new AVLGroupTree(WrapperTDigestArrays.INSTANCE); + AVLGroupTree x = new AVLGroupTree(arrays()); for (int i = 0; i < 101; i++) { x.add(new Centroid(i / 2)); } @@ -73,7 +70,7 @@ public void testFloor() { } public void testHeadSum() { - AVLGroupTree x = new AVLGroupTree(WrapperTDigestArrays.INSTANCE); + AVLGroupTree x = new AVLGroupTree(arrays()); for (int i = 0; i < 1000; ++i) { x.add(randomDouble(), randomIntBetween(1, 10)); } @@ -88,7 +85,7 @@ public void testHeadSum() { } public void testFloorSum() { - AVLGroupTree x = new AVLGroupTree(WrapperTDigestArrays.INSTANCE); + AVLGroupTree x = new AVLGroupTree(arrays()); int total = 0; for (int i = 0; i < 1000; ++i) { int count = randomIntBetween(1, 10); diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLTreeDigestTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLTreeDigestTests.java index 3cd89de4746f1..f6dde4e168291 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLTreeDigestTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AVLTreeDigestTests.java @@ -21,13 +21,11 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; - public class AVLTreeDigestTests extends TDigestTests { protected DigestFactory factory(final double compression) { return () -> { - AVLTreeDigest digest = new AVLTreeDigest(WrapperTDigestArrays.INSTANCE, compression); + AVLTreeDigest digest = new AVLTreeDigest(arrays(), compression); digest.setRandomSeed(randomLong()); return digest; }; diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AlternativeMergeTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AlternativeMergeTests.java index 4b95e9c0ee695..0d095ec37fa45 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AlternativeMergeTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/AlternativeMergeTests.java @@ -21,15 +21,12 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; - import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Random; -public class AlternativeMergeTests extends ESTestCase { +public class AlternativeMergeTests extends TDigestTestCase { /** * Computes size using the alternative scaling limit for both an idealized merge and for * a MergingDigest. @@ -37,8 +34,8 @@ public class AlternativeMergeTests extends ESTestCase { public void testMerges() { for (int n : new int[] { 100, 1000, 10000, 100000 }) { for (double compression : new double[] { 50, 100, 200, 400 }) { - MergingDigest mergingDigest = new MergingDigest(WrapperTDigestArrays.INSTANCE, compression); - AVLTreeDigest treeDigest = new AVLTreeDigest(WrapperTDigestArrays.INSTANCE, compression); + MergingDigest mergingDigest = new MergingDigest(arrays(), compression); + AVLTreeDigest treeDigest = new AVLTreeDigest(arrays(), compression); List data = new ArrayList<>(); Random gen = random(); for (int i = 0; i < n; i++) { diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTests.java index 68b07f1096eea..7520d76172ef9 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTests.java @@ -21,9 +21,7 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.test.ESTestCase; - -public abstract class BigCountTests extends ESTestCase { +public abstract class BigCountTests extends TDigestTestCase { public void testBigMerge() { TDigest digest = createDigest(); diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsMergingDigestTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsMergingDigestTests.java index 25cd1af05a0ba..ab28628200cce 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsMergingDigestTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsMergingDigestTests.java @@ -21,11 +21,9 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; - public class BigCountTestsMergingDigestTests extends BigCountTests { @Override public TDigest createDigest() { - return new MergingDigest(WrapperTDigestArrays.INSTANCE, 100); + return new MergingDigest(arrays(), 100); } } diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsTreeDigestTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsTreeDigestTests.java index a2cdf49d8f8ad..a9af82164c2ba 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsTreeDigestTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/BigCountTestsTreeDigestTests.java @@ -21,11 +21,9 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; - public class BigCountTestsTreeDigestTests extends BigCountTests { @Override public TDigest createDigest() { - return new AVLTreeDigest(WrapperTDigestArrays.INSTANCE, 100); + return new AVLTreeDigest(arrays(), 100); } } diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/ComparisonTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/ComparisonTests.java index f5df0c2f86ea1..82620459891ec 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/ComparisonTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/ComparisonTests.java @@ -21,13 +21,10 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; - import java.util.Arrays; import java.util.function.Supplier; -public class ComparisonTests extends ESTestCase { +public class ComparisonTests extends TDigestTestCase { private static final int SAMPLE_COUNT = 1_000_000; @@ -40,10 +37,10 @@ public class ComparisonTests extends ESTestCase { private void loadData(Supplier sampleGenerator) { final int COMPRESSION = 100; - avlTreeDigest = TDigest.createAvlTreeDigest(WrapperTDigestArrays.INSTANCE, COMPRESSION); - mergingDigest = TDigest.createMergingDigest(WrapperTDigestArrays.INSTANCE, COMPRESSION); - sortingDigest = TDigest.createSortingDigest(WrapperTDigestArrays.INSTANCE); - hybridDigest = TDigest.createHybridDigest(WrapperTDigestArrays.INSTANCE, COMPRESSION); + avlTreeDigest = TDigest.createAvlTreeDigest(arrays(), COMPRESSION); + mergingDigest = TDigest.createMergingDigest(arrays(), COMPRESSION); + sortingDigest = TDigest.createSortingDigest(arrays()); + hybridDigest = TDigest.createHybridDigest(arrays(), COMPRESSION); samples = new double[SAMPLE_COUNT]; for (int i = 0; i < SAMPLE_COUNT; i++) { diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/IntAVLTreeTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/IntAVLTreeTests.java index 58c91ae6e03e6..5178701e96c2c 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/IntAVLTreeTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/IntAVLTreeTests.java @@ -21,8 +21,7 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.tdigest.arrays.TDigestArrays; import java.util.Arrays; import java.util.Iterator; @@ -30,7 +29,7 @@ import java.util.Random; import java.util.TreeMap; -public class IntAVLTreeTests extends ESTestCase { +public class IntAVLTreeTests extends TDigestTestCase { static class IntegerBag extends IntAVLTree { @@ -38,8 +37,8 @@ static class IntegerBag extends IntAVLTree { int[] values; int[] counts; - IntegerBag() { - super(WrapperTDigestArrays.INSTANCE); + IntegerBag(TDigestArrays arrays) { + super(arrays); values = new int[capacity()]; counts = new int[capacity()]; } @@ -89,7 +88,7 @@ protected void merge(int node) { public void testDualAdd() { Random r = random(); TreeMap map = new TreeMap<>(); - IntegerBag bag = new IntegerBag(); + IntegerBag bag = new IntegerBag(arrays()); for (int i = 0; i < 100000; ++i) { final int v = r.nextInt(100000); if (map.containsKey(v)) { @@ -112,7 +111,7 @@ public void testDualAdd() { public void testDualAddRemove() { Random r = random(); TreeMap map = new TreeMap<>(); - IntegerBag bag = new IntegerBag(); + IntegerBag bag = new IntegerBag(arrays()); for (int i = 0; i < 100000; ++i) { final int v = r.nextInt(1000); if (r.nextBoolean()) { diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/MedianTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/MedianTests.java index dd455b307344e..c8acec935c040 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/MedianTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/MedianTests.java @@ -21,14 +21,11 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; - -public class MedianTests extends ESTestCase { +public class MedianTests extends TDigestTestCase { public void testAVL() { double[] data = new double[] { 7, 15, 36, 39, 40, 41 }; - TDigest digest = new AVLTreeDigest(WrapperTDigestArrays.INSTANCE, 100); + TDigest digest = new AVLTreeDigest(arrays(), 100); for (double value : data) { digest.add(value); } @@ -39,7 +36,7 @@ public void testAVL() { public void testMergingDigest() { double[] data = new double[] { 7, 15, 36, 39, 40, 41 }; - TDigest digest = new MergingDigest(WrapperTDigestArrays.INSTANCE, 100); + TDigest digest = new MergingDigest(arrays(), 100); for (double value : data) { digest.add(value); } @@ -50,7 +47,7 @@ public void testMergingDigest() { public void testSortingDigest() { double[] data = new double[] { 7, 15, 36, 39, 40, 41 }; - TDigest digest = new SortingDigest(WrapperTDigestArrays.INSTANCE); + TDigest digest = new SortingDigest(arrays()); for (double value : data) { digest.add(value); } @@ -61,7 +58,7 @@ public void testSortingDigest() { public void testHybridDigest() { double[] data = new double[] { 7, 15, 36, 39, 40, 41 }; - TDigest digest = new HybridDigest(WrapperTDigestArrays.INSTANCE, 100); + TDigest digest = new HybridDigest(arrays(), 100); for (double value : data) { digest.add(value); } diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/SortTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/SortTests.java index 7327dfb5aac3c..425e4d1497eda 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/SortTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/SortTests.java @@ -22,22 +22,20 @@ package org.elasticsearch.tdigest; import org.elasticsearch.tdigest.arrays.TDigestIntArray; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; import java.util.HashMap; import java.util.Map; import java.util.Random; -public class SortTests extends ESTestCase { +public class SortTests extends TDigestTestCase { public void testReverse() { - TDigestIntArray x = WrapperTDigestArrays.INSTANCE.newIntArray(0); + TDigestIntArray x = arrays().newIntArray(0); // don't crash with no input Sort.reverse(x, 0, x.size()); // reverse stuff! - x = WrapperTDigestArrays.INSTANCE.newIntArray(new int[] { 1, 2, 3, 4, 5 }); + x = arrays().newIntArray(new int[] { 1, 2, 3, 4, 5 }); Sort.reverse(x, 0, x.size()); for (int i = 0; i < 5; i++) { assertEquals(5 - i, x.get(i)); @@ -59,7 +57,7 @@ public void testReverse() { assertEquals(4, x.get(3)); assertEquals(1, x.get(4)); - x = WrapperTDigestArrays.INSTANCE.newIntArray(new int[] { 1, 2, 3, 4, 5, 6 }); + x = arrays().newIntArray(new int[] { 1, 2, 3, 4, 5, 6 }); Sort.reverse(x, 0, x.size()); for (int i = 0; i < 6; i++) { assertEquals(6 - i, x.get(i)); @@ -229,8 +227,8 @@ private void checkOrder(int[] order, double[] values) { } private void sort(int[] order, double[] values, int n) { - var wrappedOrder = WrapperTDigestArrays.INSTANCE.newIntArray(order); - var wrappedValues = WrapperTDigestArrays.INSTANCE.newDoubleArray(values); + var wrappedOrder = arrays().newIntArray(order); + var wrappedValues = arrays().newDoubleArray(values); Sort.stableSort(wrappedOrder, wrappedValues, n); } diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTestCase.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTestCase.java new file mode 100644 index 0000000000000..76db01d5dd0bf --- /dev/null +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTestCase.java @@ -0,0 +1,109 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * This project is based on a modification of https://github.com/tdunning/t-digest which is licensed under the Apache 2.0 License. + */ + +package org.elasticsearch.tdigest; + +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.search.aggregations.metrics.MemoryTrackingTDigestArrays; +import org.elasticsearch.tdigest.arrays.TDigestArrays; +import org.elasticsearch.tdigest.arrays.TDigestByteArray; +import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; +import org.elasticsearch.tdigest.arrays.TDigestIntArray; +import org.elasticsearch.tdigest.arrays.TDigestLongArray; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; + +import java.util.Collection; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Base class for TDigest tests that require {@link TDigestArrays} instances. + *

+ * This class provides arrays that will be automatically closed after the test. + * It will also test that all memory have been freed, as the arrays use a counting CircuitBreaker. + *

+ */ +public abstract class TDigestTestCase extends ESTestCase { + private final Collection trackedArrays = ConcurrentHashMap.newKeySet(); + + /** + * Create a new TDigestArrays instance with a limited breaker. This method may be called multiple times. + * + *

+ * The arrays created by this method will be automatically released after the test. + *

+ */ + protected DelegatingTDigestArrays arrays() { + return new DelegatingTDigestArrays(); + } + + /** + * Release all arrays before {@link ESTestCase} checks for unreleased bytes. + */ + @After + public void releaseArrays() { + Releasables.close(trackedArrays); + trackedArrays.clear(); + } + + private T register(T releasable) { + trackedArrays.add(releasable); + return releasable; + } + + protected final class DelegatingTDigestArrays implements TDigestArrays { + private final MemoryTrackingTDigestArrays delegate; + + DelegatingTDigestArrays() { + this.delegate = new MemoryTrackingTDigestArrays(newLimitedBreaker(ByteSizeValue.ofMb(100))); + } + + public TDigestDoubleArray newDoubleArray(double[] data) { + return register(delegate.newDoubleArray(data)); + } + + @Override + public TDigestDoubleArray newDoubleArray(int size) { + return register(delegate.newDoubleArray(size)); + } + + public TDigestIntArray newIntArray(int[] data) { + return register(delegate.newIntArray(data)); + } + + @Override + public TDigestIntArray newIntArray(int size) { + return register(delegate.newIntArray(size)); + } + + @Override + public TDigestLongArray newLongArray(int size) { + return register(delegate.newLongArray(size)); + } + + @Override + public TDigestByteArray newByteArray(int initialSize) { + return register(delegate.newByteArray(initialSize)); + } + } +} diff --git a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTests.java b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTests.java index 43f1e36afb314..89a0c037dc864 100644 --- a/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTests.java +++ b/libs/tdigest/src/test/java/org/elasticsearch/tdigest/TDigestTests.java @@ -21,10 +21,6 @@ package org.elasticsearch.tdigest; -import org.elasticsearch.tdigest.arrays.TDigestArrays; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; -import org.elasticsearch.test.ESTestCase; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -34,7 +30,7 @@ /** * Base test case for TDigests, just extend this class and implement the abstract methods. */ -public abstract class TDigestTests extends ESTestCase { +public abstract class TDigestTests extends TDigestTestCase { public interface DigestFactory { TDigest create(); @@ -544,8 +540,4 @@ public void testMonotonicity() { lastQuantile = q; } } - - protected static TDigestArrays arrays() { - return WrapperTDigestArrays.INSTANCE; - } } diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeDisabledRestTestIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeDisabledRestTestIT.java index c9818a34169de..123ca3b806153 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeDisabledRestTestIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeDisabledRestTestIT.java @@ -11,6 +11,7 @@ import org.elasticsearch.client.RestClient; import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.hamcrest.Matchers; @@ -23,6 +24,22 @@ public class LogsIndexModeDisabledRestTestIT extends LogsIndexModeRestTestIT { + private static final String MAPPINGS = """ + { + "template": { + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "message": { + "type": "text" + } + } + } + } + }"""; + @ClassRule() public static ElasticsearchCluster cluster = ElasticsearchCluster.local() .distribution(DistributionType.DEFAULT) @@ -50,8 +67,59 @@ public void setup() throws Exception { public void testLogsSettingsIndexModeDisabled() throws IOException { assertOK(createDataStream(client, "logs-custom-dev")); - final String indexMode = (String) getSetting(client, getDataStreamBackingIndex(client, "logs-custom-dev", 0), "index.mode"); + final String indexMode = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); assertThat(indexMode, Matchers.not(equalTo(IndexMode.LOGSDB.getName()))); } + public void testTogglingLogsdb() throws IOException { + putComponentTemplate(client, "logs@settings", MAPPINGS); + assertOK(createDataStream(client, "logs-custom-dev")); + final String indexModeBefore = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); + assertThat(indexModeBefore, Matchers.not(equalTo(IndexMode.LOGSDB.getName()))); + assertOK(putClusterSetting(client, "cluster.logsdb.enabled", "true")); + final String indexModeAfter = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); + assertThat(indexModeAfter, Matchers.not(equalTo(IndexMode.LOGSDB.getName()))); + assertOK(rolloverDataStream(client, "logs-custom-dev")); + final String indexModeLater = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 1), + IndexSettings.MODE.getKey() + ); + assertThat(indexModeLater, equalTo(IndexMode.LOGSDB.getName())); + assertOK(putClusterSetting(client, "cluster.logsdb.enabled", "false")); + assertOK(rolloverDataStream(client, "logs-custom-dev")); + final String indexModeFinal = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 2), + IndexSettings.MODE.getKey() + ); + assertThat(indexModeFinal, Matchers.not(equalTo(IndexMode.LOGSDB.getName()))); + + } + + public void testEnablingLogsdb() throws IOException { + putComponentTemplate(client, "logs@settings", MAPPINGS); + assertOK(putClusterSetting(client, "cluster.logsdb.enabled", true)); + assertOK(createDataStream(client, "logs-custom-dev")); + final String indexMode = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); + assertThat(indexMode, equalTo(IndexMode.LOGSDB.getName())); + assertOK(putClusterSetting(client, "cluster.logsdb.enabled", false)); + } + } diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeEnabledRestTestIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeEnabledRestTestIT.java index d7bdf54007d69..a024a2c0f303c 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeEnabledRestTestIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeEnabledRestTestIT.java @@ -10,8 +10,10 @@ package org.elasticsearch.datastreams.logsdb; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.hamcrest.Matchers; @@ -179,7 +181,11 @@ public void setup() throws Exception { public void testCreateDataStream() throws IOException { assertOK(putComponentTemplate(client, "logs@custom", MAPPINGS)); assertOK(createDataStream(client, "logs-custom-dev")); - final String indexMode = (String) getSetting(client, getDataStreamBackingIndex(client, "logs-custom-dev", 0), "index.mode"); + final String indexMode = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); assertThat(indexMode, equalTo(IndexMode.LOGSDB.getName())); } @@ -224,4 +230,83 @@ public void testRolloverDataStream() throws IOException { assertThat(firstBackingIndex, Matchers.not(equalTo(secondBackingIndex))); assertThat(getDataStreamBackingIndices(client, "logs-custom-dev").size(), equalTo(2)); } + + public void testLogsAtSettingWithStandardOverride() throws IOException { + assertOK(putComponentTemplate(client, "logs@custom", """ + { + "template": { + "settings": { + "index": { + "mode": "standard" + } + } + } + } + """)); + assertOK(createDataStream(client, "logs-custom-dev")); + final String indexMode = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); + assertThat(indexMode, equalTo(IndexMode.STANDARD.getName())); + } + + public void testLogsAtSettingWithTimeSeriesOverride() throws IOException { + assertOK(putComponentTemplate(client, "logs@custom", """ + { + "template": { + "settings": { + "index": { + "routing_path": [ "hostname" ], + "mode": "time_series", + "sort.field": [], + "sort.order": [] + } + }, + "mappings": { + "properties": { + "hostname": { + "type": "keyword", + "time_series_dimension": true + } + } + } + } + } + """)); + assertOK(createDataStream(client, "logs-custom-dev")); + final String indexMode = (String) getSetting( + client, + getDataStreamBackingIndex(client, "logs-custom-dev", 0), + IndexSettings.MODE.getKey() + ); + assertThat(indexMode, equalTo(IndexMode.TIME_SERIES.getName())); + } + + public void testLogsAtSettingWithTimeSeriesOverrideFailure() { + // NOTE: apm@settings defines sorting on @timestamp and template composition results in index.mode "time_series" + // with a non-allowed index.sort.field '@timestamp'. This fails at template composition stage before the index is even created. + final ResponseException ex = assertThrows(ResponseException.class, () -> putComponentTemplate(client, "logs@custom", """ + { + "template": { + "settings": { + "index": { + "routing_path": [ "hostname" ], + "mode": "time_series" + } + }, + "mappings": { + "properties": { + "hostname": { + "type": "keyword", + "time_series_dimension": true + } + } + } + } + } + """)); + assertTrue(ex.getMessage().contains("[index.mode=time_series] is incompatible with [index.sort.field]")); + } } diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeRestTestIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeRestTestIT.java index 7d65207794598..22ac2b6d7d239 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeRestTestIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeRestTestIT.java @@ -33,10 +33,16 @@ protected static void waitForLogs(RestClient client) throws Exception { }); } - protected static Response putComponentTemplate(final RestClient client, final String templateName, final String mappings) + protected static Response putComponentTemplate(final RestClient client, final String componentTemplate, final String contends) throws IOException { - final Request request = new Request("PUT", "/_component_template/" + templateName); - request.setJsonEntity(mappings); + final Request request = new Request("PUT", "/_component_template/" + componentTemplate); + request.setJsonEntity(contends); + return client.performRequest(request); + } + + protected static Response putTemplate(final RestClient client, final String template, final String contents) throws IOException { + final Request request = new Request("PUT", "/_index_template/" + template); + request.setJsonEntity(contents); return client.performRequest(request); } @@ -87,4 +93,11 @@ protected static Response bulkIndex(final RestClient client, final String dataSt bulkRequest.addParameter("refresh", "true"); return client.performRequest(bulkRequest); } + + protected static Response putClusterSetting(final RestClient client, final String settingName, final Object settingValue) + throws IOException { + final Request request = new Request("PUT", "/_cluster/settings"); + request.setJsonEntity("{ \"transient\": { \"" + settingName + "\": " + settingValue + " } }"); + return client.performRequest(request); + } } diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/qa/DataGenerationHelper.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/qa/DataGenerationHelper.java index 7fd1ccde10053..ce0820b940bf8 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/qa/DataGenerationHelper.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/qa/DataGenerationHelper.java @@ -35,12 +35,7 @@ class DataGenerationHelper { private final DataGenerator dataGenerator; DataGenerationHelper() { - // TODO enable subobjects: auto - // It is disabled because it currently does not have auto flattening and that results in asserts being triggered when using copy_to. - this.subobjects = ESTestCase.randomValueOtherThan( - ObjectMapper.Subobjects.AUTO, - () -> ESTestCase.randomFrom(ObjectMapper.Subobjects.values()) - ); + this.subobjects = ESTestCase.randomFrom(ObjectMapper.Subobjects.values()); this.keepArraySource = ESTestCase.randomBoolean(); var specificationBuilder = DataGeneratorSpecification.builder().withFullyDynamicMapping(ESTestCase.randomBoolean()); diff --git a/muted-tests.yml b/muted-tests.yml index 602d790246648..5d7474af06d86 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -251,9 +251,6 @@ tests: - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=transform/transforms_force_delete/Test force deleting a running transform} issue: https://github.com/elastic/elasticsearch/issues/113327 -- class: org.elasticsearch.xpack.security.support.SecurityIndexManagerIntegTests - method: testOnIndexAvailableForSearchIndexAlreadyAvailable - issue: https://github.com/elastic/elasticsearch/issues/113336 - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=analytics/top_metrics/sort by scaled float field} issue: https://github.com/elastic/elasticsearch/issues/113340 @@ -293,12 +290,6 @@ tests: - class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT method: test {p0=search/180_locale_dependent_mapping/Test Index and Search locale dependent mappings / dates} issue: https://github.com/elastic/elasticsearch/issues/113537 -- class: org.elasticsearch.xpack.esql.qa.mixed.EsqlClientYamlIT - method: test {p0=esql/70_locale/Date format with default locale} - issue: https://github.com/elastic/elasticsearch/issues/113539 -- class: org.elasticsearch.xpack.esql.qa.mixed.EsqlClientYamlIT - method: test {p0=esql/70_locale/Date format with Italian locale} - issue: https://github.com/elastic/elasticsearch/issues/113540 - class: org.elasticsearch.xpack.inference.TextEmbeddingCrudIT method: testPutE5WithTrainedModelAndInference issue: https://github.com/elastic/elasticsearch/issues/113565 @@ -308,6 +299,12 @@ tests: - class: org.elasticsearch.xpack.ml.integration.MlJobIT method: testCantCreateJobWithSameID issue: https://github.com/elastic/elasticsearch/issues/113581 +- class: org.elasticsearch.integration.KibanaUserRoleIntegTests + method: testFieldMappings + issue: https://github.com/elastic/elasticsearch/issues/113592 +- class: org.elasticsearch.integration.KibanaUserRoleIntegTests + method: testSearchAndMSearch + issue: https://github.com/elastic/elasticsearch/issues/113593 # Examples: # diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index a9402c324f7fc..f588b78c78cc8 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -386,6 +386,9 @@ public void test040JavaUsesTheOsProvidedKeystore() { if (distribution.packaging == Packaging.DOCKER_UBI || distribution.packaging == Packaging.DOCKER_IRON_BANK) { // In these images, the `cacerts` file ought to be a symlink here assertThat(path, equalTo("/etc/pki/ca-trust/extracted/java/cacerts")); + } else if (distribution.packaging == Packaging.DOCKER_WOLFI) { + // In these images, the `cacerts` file ought to be a symlink here + assertThat(path, equalTo("/etc/ssl/certs/java/cacerts")); } else { // Whereas on other images, it's a real file so the real path is the same assertThat(path, equalTo("/usr/share/elasticsearch/jdk/lib/security/cacerts")); diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index 5b86796aa80ca..a988a446f561f 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -436,7 +436,10 @@ private void verifyKeystorePermissions() { switch (distribution.packaging) { case TAR, ZIP -> assertThat(keystore, file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); case DEB, RPM -> assertThat(keystore, file(File, "root", "elasticsearch", p660)); - case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS -> assertThat(keystore, DockerFileMatcher.file(p660)); + case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS, DOCKER_WOLFI -> assertThat( + keystore, + DockerFileMatcher.file(p660) + ); default -> throw new IllegalStateException("Unknown Elasticsearch packaging type."); } } diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index a1a9af3b6e307..644990105f60f 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -245,7 +245,7 @@ protected static void install() throws Exception { installation = Packages.installPackage(sh, distribution); Packages.verifyPackageInstallation(installation, distribution, sh); } - case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS -> { + case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS, DOCKER_WOLFI -> { installation = Docker.runContainer(distribution); Docker.verifyContainerInstallation(installation); } @@ -337,6 +337,7 @@ public Shell.Result runElasticsearchStartCommand(String password, boolean daemon case DOCKER_IRON_BANK: case DOCKER_CLOUD: case DOCKER_CLOUD_ESS: + case DOCKER_WOLFI: // nothing, "installing" docker image is running it return Shell.NO_OP; default: @@ -359,6 +360,7 @@ public void stopElasticsearch() throws Exception { case DOCKER_IRON_BANK: case DOCKER_CLOUD: case DOCKER_CLOUD_ESS: + case DOCKER_WOLFI: // nothing, "installing" docker image is running it break; default: @@ -371,7 +373,7 @@ public void awaitElasticsearchStartup(Shell.Result result) throws Exception { switch (distribution.packaging) { case TAR, ZIP -> Archives.assertElasticsearchStarted(installation); case DEB, RPM -> Packages.assertElasticsearchStarted(sh, installation); - case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS -> Docker.waitForElasticsearchToStart(); + case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS, DOCKER_WOLFI -> Docker.waitForElasticsearchToStart(); default -> throw new IllegalStateException("Unknown Elasticsearch packaging type."); } } diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/Distribution.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/Distribution.java index b3ea54425af8e..05cef4a0818ba 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/Distribution.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/Distribution.java @@ -37,6 +37,8 @@ public Distribution(Path path) { this.packaging = Packaging.DOCKER_CLOUD; } else if (filename.endsWith(".cloud-ess.tar")) { this.packaging = Packaging.DOCKER_CLOUD_ESS; + } else if (filename.endsWith(".wolfi.tar")) { + this.packaging = Packaging.DOCKER_WOLFI; } else { int lastDot = filename.lastIndexOf('.'); this.packaging = Packaging.valueOf(filename.substring(lastDot + 1).toUpperCase(Locale.ROOT)); @@ -61,7 +63,7 @@ public boolean isPackage() { */ public boolean isDocker() { return switch (packaging) { - case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS -> true; + case DOCKER, DOCKER_UBI, DOCKER_IRON_BANK, DOCKER_CLOUD, DOCKER_CLOUD_ESS, DOCKER_WOLFI -> true; default -> false; }; } @@ -76,7 +78,8 @@ public enum Packaging { DOCKER_UBI(".ubi.tar", Platforms.isDocker()), DOCKER_IRON_BANK(".ironbank.tar", Platforms.isDocker()), DOCKER_CLOUD(".cloud.tar", Platforms.isDocker()), - DOCKER_CLOUD_ESS(".cloud-ess.tar", Platforms.isDocker()); + DOCKER_CLOUD_ESS(".cloud-ess.tar", Platforms.isDocker()), + DOCKER_WOLFI(".wolfi.tar", Platforms.isDocker()); /** The extension of this distribution's file */ public final String extension; diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/Docker.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/Docker.java index cb8a955a5972c..c38eaa58f0552 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/Docker.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/Docker.java @@ -486,9 +486,9 @@ public static void verifyContainerInstallation(Installation es) { // Ensure the `elasticsearch` user and group exist. // These lines will both throw an exception if the command fails dockerShell.run("id elasticsearch"); - dockerShell.run("getent group elasticsearch"); + dockerShell.run("grep -E '^elasticsearch:' /etc/group"); - final Shell.Result passwdResult = dockerShell.run("getent passwd elasticsearch"); + final Shell.Result passwdResult = dockerShell.run("grep -E '^elasticsearch:' /etc/passwd"); final String homeDir = passwdResult.stdout().trim().split(":")[5]; assertThat("elasticsearch user's home directory is incorrect", homeDir, equalTo("/usr/share/elasticsearch")); diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java index 6c58bcba09879..2b3eb7ff7a617 100644 --- a/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java +++ b/qa/packaging/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java @@ -167,6 +167,7 @@ public static String getImageName(Distribution distribution) { case DOCKER_IRON_BANK -> "-ironbank"; case DOCKER_CLOUD -> "-cloud"; case DOCKER_CLOUD_ESS -> "-cloud-ess"; + case DOCKER_WOLFI -> "-wolfi"; default -> throw new IllegalStateException("Unexpected distribution packaging type: " + distribution.packaging); }; diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 45c1b65d19600..a742e83255bbb 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -54,9 +54,7 @@ tasks.named("precommit").configure { dependsOn 'enforceYamlTestConvention' } -tasks.named("yamlRestCompatTestTransform").configure({ task -> - task.skipTest("tsdb/140_routing_path/multi-value routing path field", "Multi-value routing paths are allowed now. See #112645") - task.skipTest("indices.sort/10_basic/Index Sort", "warning does not exist for compatibility") - task.skipTest("search/330_fetch_fields/Test search rewrite", "warning does not exist for compatibility") - task.skipTest("search/540_ignore_above_synthetic_source/ignore_above mapping level setting on arrays", "Temporary mute while backporting to 8.x") +tasks.named("yamlRestCompatTestTransform").configure({task -> + task.skipTest("indices.sort/10_basic/Index Sort", "warning does not exist for compatibility") + task.skipTest("search/330_fetch_fields/Test search rewrite", "warning does not exist for compatibility") }) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/index/92_metrics_auto_subobjects.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/index/92_metrics_auto_subobjects.yml index 414c24cfffd7d..603cc4fc2e304 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/index/92_metrics_auto_subobjects.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/index/92_metrics_auto_subobjects.yml @@ -2,7 +2,7 @@ "Metrics object indexing": - requires: test_runner_features: [ "allowed_warnings", "allowed_warnings_regex" ] - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: requires supporting subobjects auto setting - do: @@ -69,7 +69,7 @@ "Root with metrics": - requires: test_runner_features: [ "allowed_warnings", "allowed_warnings_regex" ] - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: requires supporting subobjects auto setting - do: @@ -131,7 +131,7 @@ "Metrics object indexing with synthetic source": - requires: test_runner_features: [ "allowed_warnings", "allowed_warnings_regex" ] - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: added in 8.4.0 - do: @@ -201,7 +201,7 @@ "Root without subobjects with synthetic source": - requires: test_runner_features: [ "allowed_warnings", "allowed_warnings_regex" ] - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: added in 8.4.0 - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml index b5a9146bc54a6..41d9fcc30a880 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml @@ -887,7 +887,7 @@ doubly nested object: --- subobjects auto: - requires: - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: requires tracking ignored source and supporting subobjects auto setting - do: @@ -924,9 +924,21 @@ subobjects auto: type: keyword nested: type: nested - auto_obj: - type: object - subobjects: auto + path: + properties: + to: + properties: + auto_obj: + type: object + subobjects: auto + properties: + inner: + properties: + id: + type: keyword + id: + type: + integer - do: bulk: @@ -934,13 +946,13 @@ subobjects auto: refresh: true body: - '{ "create": { } }' - - '{ "id": 1, "foo": 10, "foo.bar": 100, "regular": [ { "trace": { "id": "a" }, "span": { "id": "1" } }, { "trace": { "id": "b" }, "span": { "id": "1" } } ] }' + - '{ "id": 1, "foo": 10, "foo.bar": 100, "regular.trace.id": ["b", "a", "b"], "regular.span.id": "1" }' - '{ "create": { } }' - '{ "id": 2, "foo": 20, "foo.bar": 200, "stored": [ { "trace": { "id": "a" }, "span": { "id": "1" } }, { "trace": { "id": "b" }, "span": { "id": "1" } } ] }' - '{ "create": { } }' - '{ "id": 3, "foo": 30, "foo.bar": 300, "nested": [ { "a": 10, "b": 20 }, { "a": 100, "b": 200 } ] }' - '{ "create": { } }' - - '{ "id": 4, "auto_obj": { "foo": 40, "foo.bar": 400 } }' + - '{ "id": 4, "path.to.auto_obj": { "foo": 40, "foo.bar": 400, "inner.id": "baz" }, "path.to.id": 4000 }' - match: { errors: false } @@ -952,8 +964,8 @@ subobjects auto: - match: { hits.hits.0._source.id: 1 } - match: { hits.hits.0._source.foo: 10 } - match: { hits.hits.0._source.foo\.bar: 100 } - - match: { hits.hits.0._source.regular.span.id: "1" } - - match: { hits.hits.0._source.regular.trace.id: [ "a", "b" ] } + - match: { hits.hits.0._source.regular\.span\.id: "1" } + - match: { hits.hits.0._source.regular\.trace\.id: [ "a", "b" ] } - match: { hits.hits.1._source.id: 2 } - match: { hits.hits.1._source.foo: 20 } - match: { hits.hits.1._source.foo\.bar: 200 } @@ -969,8 +981,110 @@ subobjects auto: - match: { hits.hits.2._source.nested.1.a: 100 } - match: { hits.hits.2._source.nested.1.b: 200 } - match: { hits.hits.3._source.id: 4 } - - match: { hits.hits.3._source.auto_obj.foo: 40 } - - match: { hits.hits.3._source.auto_obj.foo\.bar: 400 } + - match: { hits.hits.3._source.path\.to\.auto_obj.foo: 40 } + - match: { hits.hits.3._source.path\.to\.auto_obj.foo\.bar: 400 } + - match: { hits.hits.3._source.path\.to\.auto_obj.inner\.id: baz } + - match: { hits.hits.3._source.path\.to\.id: 4000 } + + +--- +subobjects auto with path flattening: + - requires: + cluster_features: ["mapper.subobjects_auto_fixes"] + reason: requires tracking ignored source and supporting subobjects auto setting + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: auto + properties: + id: + type: integer + attributes: + type: object + subobjects: auto + + - do: + bulk: + index: test + refresh: true + body: + - '{ "create": { } }' + - '{ "id": 1, "attributes": { "foo": { "bar": 10 } } }' + - '{ "create": { } }' + - '{ "id": 2, "attributes": { "foo": { "bar": 20 } } }' + - '{ "create": { } }' + - '{ "id": 3, "attributes": { "foo": { "bar": 30 } } }' + - '{ "create": { } }' + - '{ "id": 4, "attributes": { "foo": { "bar": 40 } } }' + + - match: { errors: false } + + - do: + search: + index: test + sort: id + + - match: { hits.hits.0._source.id: 1 } + - match: { hits.hits.0._source.attributes.foo\.bar: 10 } + - match: { hits.hits.1._source.id: 2 } + - match: { hits.hits.1._source.attributes.foo\.bar: 20 } + - match: { hits.hits.2._source.id: 3 } + - match: { hits.hits.2._source.attributes.foo\.bar: 30 } + - match: { hits.hits.3._source.id: 4 } + - match: { hits.hits.3._source.attributes.foo\.bar: 40 } + + +--- +subobjects auto with dynamic template: + - requires: + cluster_features: ["mapper.subobjects_auto_fixes"] + reason: requires tracking ignored source and supporting subobjects auto setting + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: auto + dynamic_templates: + - attributes_tmpl: + match: attributes + mapping: + type: object + enabled: false + subobjects: auto + properties: + id: + type: integer + + - do: + bulk: + index: test + refresh: true + body: + - '{ "create": { } }' + - '{ "id": 1, "attributes": { "foo": 10, "path.to.bar": "val1" }, "a": 100, "a.b": 1000 }' + + - match: { errors: false } + + - do: + search: + index: test + sort: id + + - match: { hits.hits.0._source.id: 1 } + - match: { hits.hits.0._source.attributes.foo: 10 } + - match: { hits.hits.0._source.attributes.path\.to\.bar: val1 } + - match: { hits.hits.0._source.a: 100 } + - match: { hits.hits.0._source.a\.b: 1000 } + --- synthetic_source with copy_to: @@ -1755,7 +1869,7 @@ synthetic_source with copy_to pointing to ambiguous field and subobjects false: --- synthetic_source with copy_to pointing to ambiguous field and subobjects auto: - requires: - cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: requires copy_to support in synthetic source - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_index_template/15_composition.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_index_template/15_composition.yml index 3d82539944a97..912f4e9f93df9 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_index_template/15_composition.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_index_template/15_composition.yml @@ -453,7 +453,7 @@ --- "Composable index templates that include subobjects: auto at root": - requires: - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: "https://github.com/elastic/elasticsearch/issues/96768 fixed at 8.11.0" test_runner_features: "allowed_warnings" @@ -504,7 +504,7 @@ --- "Composable index templates that include subobjects: auto on arbitrary field": - requires: - cluster_features: ["mapper.subobjects_auto"] + cluster_features: ["mapper.subobjects_auto_fixes"] reason: "https://github.com/elastic/elasticsearch/issues/96768 fixed at 8.11.0" test_runner_features: "allowed_warnings" diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml index 8a8dffda69e20..2b77b5558b3d3 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml @@ -1129,7 +1129,7 @@ fetch geo_point: --- "Test with subobjects: auto": - requires: - cluster_features: "mapper.subobjects_auto" + cluster_features: "mapper.subobjects_auto_fixes" reason: requires support for subobjects auto setting - do: diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java index 48dda7fd30068..89474a0181597 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/CollapseSearchResultsIT.java @@ -14,6 +14,7 @@ import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.search.collapse.CollapseBuilder; import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.xcontent.XContentType; import java.util.Map; import java.util.Set; @@ -85,4 +86,33 @@ public void testCollapseWithFields() { } ); } + + public void testCollapseWithStoredFields() { + final String indexName = "test_collapse"; + createIndex(indexName); + final String collapseField = "collapse_field"; + assertAcked(indicesAdmin().preparePutMapping(indexName).setSource(""" + { + "dynamic": "strict", + "properties": { + "collapse_field": { "type": "keyword", "store": true }, + "ts": { "type": "date", "store": true } + } + } + """, XContentType.JSON)); + index(indexName, "id_1_0", Map.of(collapseField, "value1", "ts", 0)); + index(indexName, "id_1_1", Map.of(collapseField, "value1", "ts", 1)); + index(indexName, "id_2_0", Map.of(collapseField, "value2", "ts", 2)); + refresh(indexName); + + assertNoFailuresAndResponse( + prepareSearch(indexName).setQuery(new MatchAllQueryBuilder()) + .setFetchSource(false) + .storedFields("*") + .setCollapse(new CollapseBuilder(collapseField)), + searchResponse -> { + assertEquals(collapseField, searchResponse.getHits().getCollapseField()); + } + ); + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index ebe9f27f461cf..7f9b59d427656 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -389,6 +389,14 @@ static Mapping createDynamicUpdate(DocumentParserContext context) { rootBuilder.addRuntimeField(runtimeField); } RootObjectMapper root = rootBuilder.build(MapperBuilderContext.root(context.mappingLookup().isSourceSynthetic(), false)); + + // Repeat the check, in case the dynamic mappers don't produce a mapping update. + // For instance, the parsed source may contain intermediate objects that get flattened, + // leading to an empty dynamic update. + if (root.mappers.isEmpty() && root.runtimeFields().isEmpty()) { + return null; + } + return context.mappingLookup().getMapping().mappingUpdate(root); } @@ -638,7 +646,7 @@ private static void parseObject(final DocumentParserContext context, String curr private static void doParseObject(DocumentParserContext context, String currentFieldName, Mapper objectMapper) throws IOException { context.path().add(currentFieldName); boolean withinLeafObject = context.path().isWithinLeafObject(); - if (objectMapper instanceof ObjectMapper objMapper && objMapper.subobjects() != ObjectMapper.Subobjects.ENABLED) { + if (objectMapper instanceof ObjectMapper objMapper && objMapper.subobjects() == ObjectMapper.Subobjects.DISABLED) { context.path().setWithinLeafObject(true); } parseObjectOrField(context, objectMapper); @@ -1012,11 +1020,15 @@ private static Mapper getLeafMapper(final DocumentParserContext context, String // don't create a dynamic mapping for it and don't index it. String fieldPath = context.path().pathAsText(fieldName); MappedFieldType fieldType = context.mappingLookup().getFieldType(fieldPath); - if (fieldType != null) { - // we haven't found a mapper with this name above, which means if a field type is found it is for sure a runtime field. - assert fieldType.hasDocValues() == false && fieldType.isAggregatable() && fieldType.isSearchable(); + + if (fieldType != null && fieldType.hasDocValues() == false && fieldType.isAggregatable() && fieldType.isSearchable()) { + // We haven't found a mapper with this name above, which means it is a runtime field. return noopFieldMapper(fieldPath); } + // No match or the matching field type corresponds to a mapper with flattened name (containing dots), + // e.g. for field 'foo.bar' under root there is no 'bar' mapper in object 'bar'. + // Returning null leads to creating a dynamic mapper. In the case of a mapper with flattened name, + // the dynamic mapper later gets deduplicated when building the dynamic update for the doc at hand. return null; } @@ -1160,11 +1172,10 @@ private static class RootDocumentParserContext extends DocumentParserContext { mappingLookup.getMapping().getRoot(), ObjectMapper.Dynamic.getRootDynamic(mappingLookup) ); - if (mappingLookup.getMapping().getRoot().subobjects() == ObjectMapper.Subobjects.ENABLED) { - this.parser = DotExpandingXContentParser.expandDots(parser, this.path); - } else { - this.parser = parser; - } + // If root supports no subobjects, there's no point in expanding dots in names to subobjects. + this.parser = (mappingLookup.getMapping().getRoot().subobjects() == ObjectMapper.Subobjects.DISABLED) + ? parser + : DotExpandingXContentParser.expandDots(parser, this.path, this); this.document = new LuceneDocument(); this.documents.add(document); this.maxAllowedNumNestedDocs = indexSettings().getMappingNestedDocsLimit(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java index c2970d8716147..b8acdb716b467 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java @@ -123,6 +123,7 @@ public int get() { private Field version; private final SeqNoFieldMapper.SequenceIDFields seqID; private final Set fieldsAppliedFromTemplates; + private final boolean supportsObjectAutoFlattening; /** * Fields that are copied from values of other fields via copy_to. @@ -177,6 +178,7 @@ private DocumentParserContext( this.copyToFields = copyToFields; this.dynamicMappersSize = dynamicMapperSize; this.recordedSource = recordedSource; + this.supportsObjectAutoFlattening = checkForAutoFlatteningSupport(); } private DocumentParserContext(ObjectMapper parent, ObjectMapper.Dynamic dynamic, DocumentParserContext in) { @@ -204,6 +206,43 @@ private DocumentParserContext(ObjectMapper parent, ObjectMapper.Dynamic dynamic, ); } + private boolean checkForAutoFlatteningSupport() { + if (root().subobjects() != ObjectMapper.Subobjects.ENABLED) { + return true; + } + for (ObjectMapper objectMapper : mappingLookup.objectMappers().values()) { + if (objectMapper.subobjects() != ObjectMapper.Subobjects.ENABLED) { + return true; + } + } + if (root().dynamicTemplates() != null) { + for (DynamicTemplate dynamicTemplate : root().dynamicTemplates()) { + if (findSubobjects(dynamicTemplate.getMapping())) { + return true; + } + } + } + for (ObjectMapper objectMapper : dynamicObjectMappers.values()) { + if (objectMapper.subobjects() != ObjectMapper.Subobjects.ENABLED) { + return true; + } + } + return false; + } + + @SuppressWarnings("unchecked") + private static boolean findSubobjects(Map mapping) { + for (var entry : mapping.entrySet()) { + if (entry.getKey().equals("subobjects") && (entry.getValue() instanceof Boolean || entry.getValue() instanceof String)) { + return true; + } + if (entry.getValue() instanceof Map && findSubobjects((Map) entry.getValue())) { + return true; + } + } + return false; + } + protected DocumentParserContext( MappingLookup mappingLookup, MappingParserContext mappingParserContext, @@ -464,6 +503,10 @@ public Set getCopyToFields() { return copyToFields; } + boolean supportsObjectAutoFlattening() { + return supportsObjectAutoFlattening; + } + /** * Add a new mapper dynamically created while parsing. * @@ -599,6 +642,25 @@ final ObjectMapper getDynamicObjectMapper(String name) { return dynamicObjectMappers.get(name); } + ObjectMapper findObject(String fullName) { + // does the object mapper already exist? if so, use that + ObjectMapper objectMapper = mappingLookup().objectMappers().get(fullName); + if (objectMapper != null) { + return objectMapper; + } + // has the object mapper been added as a dynamic update already? + return getDynamicObjectMapper(fullName); + } + + ObjectMapper.Builder findObjectBuilder(String fullName) { + // does the object mapper already exist? if so, use that + ObjectMapper objectMapper = findObject(fullName); + if (objectMapper != null) { + return objectMapper.newBuilder(indexSettings().getIndexVersionCreated()); + } + return null; + } + /** * Add a new runtime field dynamically created while parsing. * We use the same set for both new indexed and new runtime fields, @@ -698,7 +760,7 @@ public LuceneDocument doc() { */ public final DocumentParserContext createCopyToContext(String copyToField, LuceneDocument doc) throws IOException { ContentPath path = new ContentPath(); - XContentParser parser = DotExpandingXContentParser.expandDots(new CopyToParser(copyToField, parser()), path); + XContentParser parser = DotExpandingXContentParser.expandDots(new CopyToParser(copyToField, parser()), path, this); return new Wrapper(root(), this) { @Override public ContentPath path() { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DotExpandingXContentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DotExpandingXContentParser.java index fc003e709cbca..728c7ac6f25ac 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DotExpandingXContentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DotExpandingXContentParser.java @@ -18,6 +18,8 @@ import java.io.IOException; import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Deque; import java.util.List; import java.util.Map; @@ -38,9 +40,13 @@ private static final class WrappingParser extends FilterXContentParser { private final ContentPath contentPath; final Deque parsers = new ArrayDeque<>(); + final DocumentParserContext context; + boolean supportsObjectAutoFlattening; - WrappingParser(XContentParser in, ContentPath contentPath) throws IOException { + WrappingParser(XContentParser in, ContentPath contentPath, DocumentParserContext context) throws IOException { this.contentPath = contentPath; + this.context = context; + this.supportsObjectAutoFlattening = (context != null && context.supportsObjectAutoFlattening()); parsers.push(in); if (in.currentToken() == Token.FIELD_NAME) { expandDots(in); @@ -107,7 +113,7 @@ private void doExpandDots(XContentParser delegate, String field, int dotCount) t if (resultSize == 0) { throw new IllegalArgumentException("field name cannot contain only dots"); } - final String[] subpaths; + String[] subpaths; if (resultSize == list.length) { for (String part : list) { // check if the field name contains only whitespace @@ -126,6 +132,9 @@ private void doExpandDots(XContentParser delegate, String field, int dotCount) t } subpaths = extractAndValidateResults(field, list, resultSize); } + if (supportsObjectAutoFlattening && subpaths.length > 1) { + subpaths = maybeFlattenPaths(Arrays.asList(subpaths), context, contentPath).toArray(String[]::new); + } pushSubParser(delegate, subpaths); } @@ -235,11 +244,13 @@ public List listOrderedMap() throws IOException { /** * Wraps an XContentParser such that it re-interprets dots in field names as an object structure - * @param in the parser to wrap - * @return the wrapped XContentParser + * @param in the parser to wrap + * @param contentPath the starting path to expand, can be empty + * @param context provides mapping context to check for objects supporting sub-object auto-flattening + * @return the wrapped XContentParser */ - static XContentParser expandDots(XContentParser in, ContentPath contentPath) throws IOException { - return new WrappingParser(in, contentPath); + static XContentParser expandDots(XContentParser in, ContentPath contentPath, DocumentParserContext context) throws IOException { + return new WrappingParser(in, contentPath, context); } private enum State { @@ -410,4 +421,49 @@ public Token nextToken() throws IOException { return null; } } + + static List maybeFlattenPaths(List subpaths, DocumentParserContext context, ContentPath contentPath) { + String prefixWithDots = contentPath.pathAsText(""); + ObjectMapper parent = contentPath.length() == 0 + ? context.root() + : context.findObject(prefixWithDots.substring(0, prefixWithDots.length() - 1)); + List result = new ArrayList<>(subpaths.size()); + for (int i = 0; i < subpaths.size(); i++) { + String fullPath = prefixWithDots + String.join(".", subpaths.subList(0, i)); + if (i > 0) { + parent = context.findObject(fullPath); + } + boolean match = false; + StringBuilder path = new StringBuilder(subpaths.get(i)); + if (parent == null) { + // We get here for dynamic objects, which always get parsed with subobjects and may get flattened later. + match = true; + } else if (parent.subobjects() == ObjectMapper.Subobjects.ENABLED) { + match = true; + } else if (parent.subobjects() == ObjectMapper.Subobjects.AUTO) { + // Check if there's any subobject in the remaining path. + for (int j = i; j < subpaths.size() - 1; j++) { + if (j > i) { + path.append(".").append(subpaths.get(j)); + } + Mapper mapper = parent.mappers.get(path.toString()); + if (mapper instanceof ObjectMapper objectMapper + && (ObjectMapper.isFlatteningCandidate(objectMapper.subobjects, objectMapper) + || objectMapper.checkFlattenable(null).isPresent())) { + i = j; + match = true; + break; + } + } + } + if (match) { + result.add(path.toString()); + } else { + // We only get here if parent has subobjects set to false, or set to auto with no non-flattenable object in the sub-path. + result.add(String.join(".", subpaths.subList(i, subpaths.size()))); + return result; + } + } + return result; + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java index 4b6419b85e155..cf810e278782a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.time.DateTimeException; import java.util.Map; +import java.util.Optional; /** * Encapsulates the logic for dynamically creating fields as part of document parsing. @@ -162,7 +163,9 @@ static Mapper createDynamicObjectMapper(DocumentParserContext context, String na Mapper mapper = createObjectMapperFromTemplate(context, name); return mapper != null ? mapper - : new ObjectMapper.Builder(name, context.parent().subobjects).enabled(ObjectMapper.Defaults.ENABLED) + // Dynamic objects are configured with subobject support, otherwise they can't get auto-flattened + // even if they otherwise qualify. + : new ObjectMapper.Builder(name, Optional.empty()).enabled(ObjectMapper.Defaults.ENABLED) .build(context.createDynamicMapperBuilderContext()); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java index 2f665fd5d1e6a..31df558492b35 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java @@ -36,6 +36,7 @@ public Set getFeatures() { NodeMappingStats.SEGMENT_LEVEL_FIELDS_STATS, BooleanFieldMapper.BOOLEAN_DIMENSION, ObjectMapper.SUBOBJECTS_AUTO, + ObjectMapper.SUBOBJECTS_AUTO_FIXES, KeywordFieldMapper.KEYWORD_NORMALIZER_SYNTHETIC_SOURCE, SourceFieldMapper.SYNTHETIC_SOURCE_STORED_FIELDS_ADVANCE_FIX, Mapper.SYNTHETIC_SOURCE_KEEP_FEATURE, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index f9c854749e885..b9b611d8c62f9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -45,6 +45,7 @@ public class ObjectMapper extends Mapper { public static final String CONTENT_TYPE = "object"; static final String STORE_ARRAY_SOURCE_PARAM = "store_array_source"; static final NodeFeature SUBOBJECTS_AUTO = new NodeFeature("mapper.subobjects_auto"); + static final NodeFeature SUBOBJECTS_AUTO_FIXES = new NodeFeature("mapper.subobjects_auto_fixes"); /** * Enhances the previously boolean option for subobjects support with an intermediate mode `auto` that uses @@ -176,42 +177,84 @@ public final void addDynamic(String name, String prefix, Mapper mapper, Document // If the mapper to add has no dots, or the current object mapper has subobjects set to false, // we just add it as it is for sure a leaf mapper if (name.contains(".") == false || (subobjects.isPresent() && (subobjects.get() == Subobjects.DISABLED))) { - add(name, mapper); - } else { - // We strip off the first object path of the mapper name, load or create - // the relevant object mapper, and then recurse down into it, passing the remainder - // of the mapper name. So for a mapper 'foo.bar.baz', we locate 'foo' and then - // call addDynamic on it with the name 'bar.baz', and next call addDynamic on 'bar' with the name 'baz'. - int firstDotIndex = name.indexOf('.'); - String immediateChild = name.substring(0, firstDotIndex); - String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild; - Builder parentBuilder = findObjectBuilder(immediateChildFullName, context); - if (parentBuilder != null) { - parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context); - add(parentBuilder); - } else if (subobjects.isPresent() && subobjects.get() == Subobjects.AUTO) { - // No matching parent object was found, the mapper is added as a leaf - similar to subobjects false. - add(name, mapper); - } else { - // Expected to find a matching parent object but got null. - throw new IllegalStateException("Missing intermediate object " + immediateChildFullName); + if (mapper instanceof ObjectMapper objectMapper + && isFlatteningCandidate(subobjects, objectMapper) + && objectMapper.checkFlattenable(null).isEmpty()) { + // Subobjects auto and false don't allow adding subobjects dynamically. + return; } + add(name, mapper); + return; } - } + if (subobjects.isPresent() && subobjects.get() == Subobjects.AUTO) { + // Check if there's an existing field with the sanme, to avoid no-op dynamic updates. + ObjectMapper objectMapper = (prefix == null) ? context.root() : context.mappingLookup().objectMappers().get(prefix); + if (objectMapper != null && objectMapper.mappers.containsKey(name)) { + return; + } + + // Check for parent objects. Due to auto-flattening, names with dots are allowed so we need to check for all possible + // object names. For instance, for mapper 'foo.bar.baz.bad', we have the following options: + // -> object 'foo' found => call addDynamic on 'bar.baz.bad' + // ---> object 'bar' found => call addDynamic on 'baz.bad' + // -----> object 'baz' found => add field 'bad' to it + // -----> no match found => add field 'baz.bad' to 'bar' + // ---> object 'bar.baz' found => add field 'bad' to it + // ---> no match found => add field 'bar.baz.bad' to 'foo' + // -> object 'foo.bar' found => call addDynamic on 'baz.bad' + // ---> object 'baz' found => add field 'bad' to it + // ---> no match found=> add field 'baz.bad' to 'foo.bar' + // -> object 'foo.bar.baz' found => add field 'bad' to it + // -> no match found => add field 'foo.bar.baz.bad' to parent + String fullPathToMapper = name.substring(0, name.lastIndexOf(mapper.leafName())); + String[] fullPathTokens = fullPathToMapper.split("\\."); + StringBuilder candidateObject = new StringBuilder(); + String candidateObjectPrefix = prefix == null ? "" : prefix + "."; + for (int i = 0; i < fullPathTokens.length; i++) { + if (candidateObject.isEmpty() == false) { + candidateObject.append("."); + } + candidateObject.append(fullPathTokens[i]); + String candidateFullObject = candidateObjectPrefix.isEmpty() + ? candidateObject.toString() + : candidateObjectPrefix + candidateObject.toString(); + ObjectMapper parent = context.findObject(candidateFullObject); + if (parent != null) { + var parentBuilder = parent.newBuilder(context.indexSettings().getIndexVersionCreated()); + parentBuilder.addDynamic(name.substring(candidateObject.length() + 1), candidateFullObject, mapper, context); + if (parentBuilder.mappersBuilders.isEmpty() == false) { + add(parentBuilder); + } + return; + } + } - private static Builder findObjectBuilder(String fullName, DocumentParserContext context) { - // does the object mapper already exist? if so, use that - ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName); - if (objectMapper != null) { - return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated()); + // No matching parent object was found, the mapper is added as a leaf - similar to subobjects false. + // This only applies to field mappers, as subobjects get auto-flattened. + if (mapper instanceof FieldMapper fieldMapper) { + FieldMapper.Builder fieldBuilder = fieldMapper.getMergeBuilder(); + fieldBuilder.setLeafName(name); // Update to reflect the current, possibly flattened name. + add(fieldBuilder); + } + return; } - // has the object mapper been added as a dynamic update already? - objectMapper = context.getDynamicObjectMapper(fullName); - if (objectMapper != null) { - return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated()); + + // We strip off the first object path of the mapper name, load or create + // the relevant object mapper, and then recurse down into it, passing the remainder + // of the mapper name. So for a mapper 'foo.bar.baz', we locate 'foo' and then + // call addDynamic on it with the name 'bar.baz', and next call addDynamic on 'bar' with the name 'baz'. + int firstDotIndex = name.indexOf('.'); + String immediateChild = name.substring(0, firstDotIndex); + String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild; + Builder parentBuilder = context.findObjectBuilder(immediateChildFullName); + if (parentBuilder != null) { + parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context); + add(parentBuilder); + } else { + // Expected to find a matching parent object but got null. + throw new IllegalStateException("Missing intermediate object " + immediateChildFullName); } - // no object mapper found - return null; + } protected final Map buildMappers(MapperBuilderContext mapperBuilderContext) { @@ -227,9 +270,10 @@ protected final Map buildMappers(MapperBuilderContext mapperBuil // mix of object notation and dot notation. mapper = existing.merge(mapper, MapperMergeContext.from(mapperBuilderContext, Long.MAX_VALUE)); } - if (subobjects.isPresent() && subobjects.get() == Subobjects.DISABLED && mapper instanceof ObjectMapper objectMapper) { - // We're parsing a mapping that has set `subobjects: false` but has defined sub-objects - objectMapper.asFlattenedFieldMappers(mapperBuilderContext).forEach(m -> mappers.put(m.leafName(), m)); + if (mapper instanceof ObjectMapper objectMapper && isFlatteningCandidate(subobjects, objectMapper)) { + // We're parsing a mapping that has defined sub-objects, may need to flatten them. + objectMapper.asFlattenedFieldMappers(mapperBuilderContext, throwOnFlattenableError(subobjects)) + .forEach(m -> mappers.put(m.leafName(), m)); } else { mappers.put(mapper.leafName(), mapper); } @@ -624,12 +668,11 @@ private static Map buildMergedMappers( Optional subobjects ) { Map mergedMappers = new HashMap<>(); + var context = objectMergeContext.getMapperBuilderContext(); for (Mapper childOfExistingMapper : existing.mappers.values()) { - if (subobjects.isPresent() - && subobjects.get() == Subobjects.DISABLED - && childOfExistingMapper instanceof ObjectMapper objectMapper) { - // An existing mapping with sub-objects is merged with a mapping that has set `subobjects: false` - objectMapper.asFlattenedFieldMappers(objectMergeContext.getMapperBuilderContext()) + if (childOfExistingMapper instanceof ObjectMapper objectMapper && isFlatteningCandidate(subobjects, objectMapper)) { + // An existing mapping with sub-objects is merged with a mapping that has `subobjects` set to false or auto. + objectMapper.asFlattenedFieldMappers(context, throwOnFlattenableError(subobjects)) .forEach(m -> mergedMappers.put(m.leafName(), m)); } else { putMergedMapper(mergedMappers, childOfExistingMapper); @@ -638,11 +681,9 @@ private static Map buildMergedMappers( for (Mapper mergeWithMapper : mergeWithObject) { Mapper mergeIntoMapper = mergedMappers.get(mergeWithMapper.leafName()); if (mergeIntoMapper == null) { - if (subobjects.isPresent() - && subobjects.get() == Subobjects.DISABLED - && mergeWithMapper instanceof ObjectMapper objectMapper) { - // An existing mapping that has set `subobjects: false` is merged with a mapping with sub-objects - objectMapper.asFlattenedFieldMappers(objectMergeContext.getMapperBuilderContext()) + if (mergeWithMapper instanceof ObjectMapper objectMapper && isFlatteningCandidate(subobjects, objectMapper)) { + // An existing mapping with `subobjects` set to false or auto is merged with a mapping with sub-objects + objectMapper.asFlattenedFieldMappers(context, throwOnFlattenableError(subobjects)) .stream() .filter(m -> objectMergeContext.decrementFieldBudgetIfPossible(m.getTotalFieldsCount())) .forEach(m -> putMergedMapper(mergedMappers, m)); @@ -699,57 +740,83 @@ private static ObjectMapper truncateObjectMapper(MapperMergeContext context, Obj * * @throws IllegalArgumentException if the mapper cannot be flattened */ - List asFlattenedFieldMappers(MapperBuilderContext context) { - List flattenedMappers = new ArrayList<>(); + List asFlattenedFieldMappers(MapperBuilderContext context, boolean throwOnFlattenableError) { + List flattenedMappers = new ArrayList<>(); ContentPath path = new ContentPath(); - asFlattenedFieldMappers(context, flattenedMappers, path); + asFlattenedFieldMappers(context, flattenedMappers, path, throwOnFlattenableError); return flattenedMappers; } - private void asFlattenedFieldMappers(MapperBuilderContext context, List flattenedMappers, ContentPath path) { - ensureFlattenable(context, path); + static boolean isFlatteningCandidate(Optional subobjects, ObjectMapper mapper) { + return subobjects.isPresent() && subobjects.get() != Subobjects.ENABLED && mapper instanceof NestedObjectMapper == false; + } + + private static boolean throwOnFlattenableError(Optional subobjects) { + return subobjects.isPresent() && subobjects.get() == Subobjects.DISABLED; + } + + private void asFlattenedFieldMappers( + MapperBuilderContext context, + List flattenedMappers, + ContentPath path, + boolean throwOnFlattenableError + ) { + var error = checkFlattenable(context); + if (error.isPresent()) { + if (throwOnFlattenableError) { + throw new IllegalArgumentException( + "Object mapper [" + + path.pathAsText(leafName()) + + "] was found in a context where subobjects is set to false. " + + "Auto-flattening [" + + path.pathAsText(leafName()) + + "] failed because " + + error.get() + ); + } + // The object can't be auto-flattened under the parent object, so it gets added at the current level. + // [subobjects=auto] applies auto-flattening to names, so the leaf name may need to change. + // Since mapper objects are immutable, we create a clone of the current one with the updated leaf name. + flattenedMappers.add( + path.pathAsText("").isEmpty() + ? this + : new ObjectMapper(path.pathAsText(leafName()), fullPath, enabled, subobjects, storeArraySource, dynamic, mappers) + ); + return; + } path.add(leafName()); for (Mapper mapper : mappers.values()) { if (mapper instanceof FieldMapper fieldMapper) { FieldMapper.Builder fieldBuilder = fieldMapper.getMergeBuilder(); fieldBuilder.setLeafName(path.pathAsText(mapper.leafName())); flattenedMappers.add(fieldBuilder.build(context)); - } else if (mapper instanceof ObjectMapper objectMapper) { - objectMapper.asFlattenedFieldMappers(context, flattenedMappers, path); + } else if (mapper instanceof ObjectMapper objectMapper && mapper instanceof NestedObjectMapper == false) { + objectMapper.asFlattenedFieldMappers(context, flattenedMappers, path, throwOnFlattenableError); } } path.remove(); } - private void ensureFlattenable(MapperBuilderContext context, ContentPath path) { - if (dynamic != null && context.getDynamic() != dynamic) { - throwAutoFlatteningException( - path, + Optional checkFlattenable(MapperBuilderContext context) { + if (dynamic != null && (context == null || context.getDynamic() != dynamic)) { + return Optional.of( "the value of [dynamic] (" + dynamic + ") is not compatible with the value from its parent context (" - + context.getDynamic() + + (context != null ? context.getDynamic() : "") + ")" ); } + if (storeArraySource()) { + return Optional.of("the value of [store_array_source] is [true]"); + } if (isEnabled() == false) { - throwAutoFlatteningException(path, "the value of [enabled] is [false]"); + return Optional.of("the value of [enabled] is [false]"); } - if (subobjects.isPresent() && subobjects.get() == Subobjects.ENABLED) { - throwAutoFlatteningException(path, "the value of [subobjects] is [true]"); + if (subobjects.isPresent() && subobjects.get() != Subobjects.DISABLED) { + return Optional.of("the value of [subobjects] is [" + subobjects().printedValue + "]"); } - } - - private void throwAutoFlatteningException(ContentPath path, String reason) { - throw new IllegalArgumentException( - "Object mapper [" - + path.pathAsText(leafName()) - + "] was found in a context where subobjects is set to false. " - + "Auto-flattening [" - + path.pathAsText(leafName()) - + "] failed because " - + reason - ); + return Optional.empty(); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/EmptyTDigestState.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/EmptyTDigestState.java index 6ae9c655df3e8..56ac38a70cf07 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/EmptyTDigestState.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/EmptyTDigestState.java @@ -9,12 +9,10 @@ package org.elasticsearch.search.aggregations.metrics; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; - public final class EmptyTDigestState extends TDigestState { public EmptyTDigestState() { // Use the sorting implementation to minimize memory allocation. - super(WrapperTDigestArrays.INSTANCE, Type.SORTING, 1.0D); + super(MemoryTrackingTDigestArrays.INSTANCE, Type.SORTING, 1.0D); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArrays.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArrays.java new file mode 100644 index 0000000000000..e99bfbfe534c8 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArrays.java @@ -0,0 +1,401 @@ +/* + * 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.search.aggregations.metrics; + +import org.apache.lucene.util.Accountable; +import org.apache.lucene.util.ArrayUtil; +import org.apache.lucene.util.RamUsageEstimator; +import org.elasticsearch.common.breaker.CircuitBreaker; +import org.elasticsearch.common.breaker.NoopCircuitBreaker; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.tdigest.arrays.TDigestArrays; +import org.elasticsearch.tdigest.arrays.TDigestByteArray; +import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; +import org.elasticsearch.tdigest.arrays.TDigestIntArray; +import org.elasticsearch.tdigest.arrays.TDigestLongArray; + +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * TDigestArrays with raw arrays and circuit breaking. + */ +public class MemoryTrackingTDigestArrays implements TDigestArrays { + + /** + * Default no-op CB instance of the wrapper. + * + * @deprecated This instance shouldn't be used, and will be removed after all usages are replaced. + */ + @Deprecated + public static final MemoryTrackingTDigestArrays INSTANCE = new MemoryTrackingTDigestArrays( + new NoopCircuitBreaker("default-wrapper-tdigest-arrays") + ); + + private final CircuitBreaker breaker; + + public MemoryTrackingTDigestArrays(CircuitBreaker breaker) { + this.breaker = breaker; + } + + @Override + public MemoryTrackingTDigestDoubleArray newDoubleArray(int initialSize) { + breaker.addEstimateBytesAndMaybeBreak( + MemoryTrackingTDigestDoubleArray.estimatedRamBytesUsed(initialSize), + "tdigest-new-double-array" + ); + return new MemoryTrackingTDigestDoubleArray(breaker, initialSize); + } + + @Override + public MemoryTrackingTDigestIntArray newIntArray(int initialSize) { + breaker.addEstimateBytesAndMaybeBreak(MemoryTrackingTDigestIntArray.estimatedRamBytesUsed(initialSize), "tdigest-new-int-array"); + return new MemoryTrackingTDigestIntArray(breaker, initialSize); + } + + @Override + public TDigestLongArray newLongArray(int initialSize) { + breaker.addEstimateBytesAndMaybeBreak(MemoryTrackingTDigestLongArray.estimatedRamBytesUsed(initialSize), "tdigest-new-long-array"); + return new MemoryTrackingTDigestLongArray(breaker, initialSize); + } + + @Override + public TDigestByteArray newByteArray(int initialSize) { + breaker.addEstimateBytesAndMaybeBreak(MemoryTrackingTDigestByteArray.estimatedRamBytesUsed(initialSize), "tdigest-new-byte-array"); + return new MemoryTrackingTDigestByteArray(breaker, initialSize); + } + + public MemoryTrackingTDigestDoubleArray newDoubleArray(double[] array) { + breaker.addEstimateBytesAndMaybeBreak( + MemoryTrackingTDigestDoubleArray.estimatedRamBytesUsed(array.length), + "tdigest-new-double-array" + ); + return new MemoryTrackingTDigestDoubleArray(breaker, array); + } + + public MemoryTrackingTDigestIntArray newIntArray(int[] array) { + breaker.addEstimateBytesAndMaybeBreak(MemoryTrackingTDigestIntArray.estimatedRamBytesUsed(array.length), "tdigest-new-int-array"); + return new MemoryTrackingTDigestIntArray(breaker, array); + } + + private static long estimatedArraySize(long arrayLength, long bytesPerElement) { + return RamUsageEstimator.alignObjectSize(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + arrayLength * bytesPerElement); + } + + private abstract static class AbstractMemoryTrackingArray implements Releasable, Accountable { + protected final CircuitBreaker breaker; + private final AtomicBoolean closed = new AtomicBoolean(false); + + AbstractMemoryTrackingArray(CircuitBreaker breaker) { + this.breaker = breaker; + } + + @Override + public final void close() { + if (closed.compareAndSet(false, true)) { + breaker.addWithoutBreaking(-ramBytesUsed()); + } + } + } + + public static class MemoryTrackingTDigestDoubleArray extends AbstractMemoryTrackingArray implements TDigestDoubleArray { + static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(MemoryTrackingTDigestDoubleArray.class); + + private double[] array; + private int size; + + public MemoryTrackingTDigestDoubleArray(CircuitBreaker breaker, int initialSize) { + this(breaker, new double[initialSize]); + } + + public MemoryTrackingTDigestDoubleArray(CircuitBreaker breaker, double[] array) { + super(breaker); + this.array = array; + this.size = array.length; + } + + public static long estimatedRamBytesUsed(int size) { + return SHALLOW_SIZE + estimatedArraySize(size, Double.BYTES); + } + + @Override + public long ramBytesUsed() { + return estimatedRamBytesUsed(array.length); + } + + @Override + public int size() { + return size; + } + + @Override + public double get(int index) { + assert index >= 0 && index < size; + return array[index]; + } + + @Override + public void set(int index, double value) { + assert index >= 0 && index < size; + array[index] = value; + } + + @Override + public void add(double value) { + ensureCapacity(size + 1); + array[size++] = value; + } + + @Override + public void sort() { + Arrays.sort(array, 0, size); + } + + @Override + public void resize(int newSize) { + ensureCapacity(newSize); + + if (newSize > size) { + Arrays.fill(array, size, newSize, 0); + } + + size = newSize; + } + + @Override + public void ensureCapacity(int requiredCapacity) { + if (requiredCapacity > array.length) { + double[] oldArray = array; + // Used for used bytes assertion + long oldRamBytesUsed = ramBytesUsed(); + long oldArraySize = RamUsageEstimator.sizeOf(oldArray); + + int newSize = ArrayUtil.oversize(requiredCapacity, Double.BYTES); + long newArraySize = estimatedArraySize(newSize, Double.BYTES); + breaker.addEstimateBytesAndMaybeBreak(newArraySize, "tdigest-new-capacity-double-array"); + array = Arrays.copyOf(array, newSize); + breaker.addWithoutBreaking(-RamUsageEstimator.sizeOf(oldArray)); + + assert ramBytesUsed() - oldRamBytesUsed == newArraySize - oldArraySize + : "ramBytesUsed() should be aligned with manual array calculations"; + } + } + } + + public static class MemoryTrackingTDigestIntArray extends AbstractMemoryTrackingArray implements TDigestIntArray { + static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(MemoryTrackingTDigestIntArray.class); + + private int[] array; + private int size; + + public MemoryTrackingTDigestIntArray(CircuitBreaker breaker, int initialSize) { + this(breaker, new int[initialSize]); + } + + public MemoryTrackingTDigestIntArray(CircuitBreaker breaker, int[] array) { + super(breaker); + this.array = array; + this.size = array.length; + } + + public static long estimatedRamBytesUsed(int size) { + return SHALLOW_SIZE + estimatedArraySize(size, Integer.BYTES); + } + + @Override + public long ramBytesUsed() { + return estimatedRamBytesUsed(array.length); + } + + @Override + public int size() { + return size; + } + + @Override + public int get(int index) { + assert index >= 0 && index < size; + return array[index]; + } + + @Override + public void set(int index, int value) { + assert index >= 0 && index < size; + array[index] = value; + } + + @Override + public void resize(int newSize) { + ensureCapacity(newSize); + if (newSize > size) { + Arrays.fill(array, size, newSize, 0); + } + size = newSize; + } + + private void ensureCapacity(int requiredCapacity) { + if (requiredCapacity > array.length) { + int[] oldArray = array; + // Used for used bytes assertion + long oldRamBytesUsed = ramBytesUsed(); + long oldArraySize = RamUsageEstimator.sizeOf(oldArray); + + int newSize = ArrayUtil.oversize(requiredCapacity, Integer.BYTES); + long newArraySize = estimatedArraySize(newSize, Integer.BYTES); + breaker.addEstimateBytesAndMaybeBreak(newArraySize, "tdigest-new-capacity-int-array"); + array = Arrays.copyOf(array, newSize); + breaker.addWithoutBreaking(-RamUsageEstimator.sizeOf(oldArray)); + + assert ramBytesUsed() - oldRamBytesUsed == newArraySize - oldArraySize + : "ramBytesUsed() should be aligned with manual array calculations"; + } + } + } + + public static class MemoryTrackingTDigestLongArray extends AbstractMemoryTrackingArray implements TDigestLongArray { + static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(MemoryTrackingTDigestLongArray.class); + + private long[] array; + private int size; + + public MemoryTrackingTDigestLongArray(CircuitBreaker breaker, int initialSize) { + this(breaker, new long[initialSize]); + } + + public MemoryTrackingTDigestLongArray(CircuitBreaker breaker, long[] array) { + super(breaker); + this.array = array; + this.size = array.length; + } + + public static long estimatedRamBytesUsed(int size) { + return SHALLOW_SIZE + estimatedArraySize(size, Long.BYTES); + } + + @Override + public long ramBytesUsed() { + return estimatedRamBytesUsed(array.length); + } + + @Override + public int size() { + return size; + } + + @Override + public long get(int index) { + assert index >= 0 && index < size; + return array[index]; + } + + @Override + public void set(int index, long value) { + assert index >= 0 && index < size; + array[index] = value; + } + + @Override + public void resize(int newSize) { + ensureCapacity(newSize); + if (newSize > size) { + Arrays.fill(array, size, newSize, 0); + } + size = newSize; + } + + private void ensureCapacity(int requiredCapacity) { + if (requiredCapacity > array.length) { + long[] oldArray = array; + // Used for used bytes assertion + long oldRamBytesUsed = ramBytesUsed(); + long oldArraySize = RamUsageEstimator.sizeOf(oldArray); + + int newSize = ArrayUtil.oversize(requiredCapacity, Long.BYTES); + long newArraySize = estimatedArraySize(newSize, Long.BYTES); + breaker.addEstimateBytesAndMaybeBreak(newArraySize, "tdigest-new-capacity-long-array"); + array = Arrays.copyOf(array, newSize); + breaker.addWithoutBreaking(-RamUsageEstimator.sizeOf(oldArray)); + + assert ramBytesUsed() - oldRamBytesUsed == newArraySize - oldArraySize + : "ramBytesUsed() should be aligned with manual array calculations"; + } + } + } + + public static class MemoryTrackingTDigestByteArray extends AbstractMemoryTrackingArray implements TDigestByteArray { + static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(MemoryTrackingTDigestByteArray.class); + + private byte[] array; + private int size; + + public MemoryTrackingTDigestByteArray(CircuitBreaker breaker, int initialSize) { + this(breaker, new byte[initialSize]); + } + + public MemoryTrackingTDigestByteArray(CircuitBreaker breaker, byte[] array) { + super(breaker); + this.array = array; + this.size = array.length; + } + + public static long estimatedRamBytesUsed(int size) { + return SHALLOW_SIZE + estimatedArraySize(size, Byte.BYTES); + } + + @Override + public long ramBytesUsed() { + return estimatedRamBytesUsed(array.length); + } + + @Override + public int size() { + return size; + } + + @Override + public byte get(int index) { + assert index >= 0 && index < size; + return array[index]; + } + + @Override + public void set(int index, byte value) { + assert index >= 0 && index < size; + array[index] = value; + } + + @Override + public void resize(int newSize) { + ensureCapacity(newSize); + if (newSize > size) { + Arrays.fill(array, size, newSize, (byte) 0); + } + size = newSize; + } + + private void ensureCapacity(int requiredCapacity) { + if (requiredCapacity > array.length) { + byte[] oldArray = array; + // Used for used bytes assertion + long oldRamBytesUsed = ramBytesUsed(); + long oldArraySize = RamUsageEstimator.sizeOf(oldArray); + + int newSize = ArrayUtil.oversize(requiredCapacity, Byte.BYTES); + long newArraySize = estimatedArraySize(newSize, Byte.BYTES); + breaker.addEstimateBytesAndMaybeBreak(newArraySize, "tdigest-new-capacity-byte-array"); + array = Arrays.copyOf(array, newSize); + breaker.addWithoutBreaking(-RamUsageEstimator.sizeOf(oldArray)); + + assert ramBytesUsed() - oldRamBytesUsed == newArraySize - oldArraySize + : "ramBytesUsed() should be aligned with manual array calculations"; + } + } + } +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestState.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestState.java index 48bdb59e430a5..78ef81684a256 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestState.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestState.java @@ -11,10 +11,11 @@ import org.elasticsearch.TransportVersions; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.Centroid; import org.elasticsearch.tdigest.TDigest; import org.elasticsearch.tdigest.arrays.TDigestArrays; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; import java.io.IOException; import java.util.Collection; @@ -25,7 +26,7 @@ * through factory method params, providing one optimized for performance (e.g. MergingDigest or HybridDigest) by default, or optionally one * that produces highly accurate results regardless of input size but its construction over the sample population takes 2x-10x longer. */ -public class TDigestState { +public class TDigestState implements Releasable { private final double compression; @@ -54,7 +55,7 @@ static Type valueForHighAccuracy() { */ @Deprecated public static TDigestState create(double compression) { - return create(WrapperTDigestArrays.INSTANCE, compression); + return create(MemoryTrackingTDigestArrays.INSTANCE, compression); } /** @@ -81,7 +82,7 @@ public static TDigestState createOptimizedForAccuracy(TDigestArrays arrays, doub */ @Deprecated public static TDigestState create(double compression, TDigestExecutionHint executionHint) { - return create(WrapperTDigestArrays.INSTANCE, compression, executionHint); + return create(MemoryTrackingTDigestArrays.INSTANCE, compression, executionHint); } /** @@ -106,7 +107,7 @@ public static TDigestState create(TDigestArrays arrays, double compression, TDig * @return a TDigestState object */ public static TDigestState createUsingParamsFrom(TDigestState state) { - return new TDigestState(WrapperTDigestArrays.INSTANCE, state.type, state.compression); + return new TDigestState(MemoryTrackingTDigestArrays.INSTANCE, state.type, state.compression); } protected TDigestState(TDigestArrays arrays, Type type, double compression) { @@ -143,7 +144,7 @@ public static void write(TDigestState state, StreamOutput out) throws IOExceptio */ @Deprecated public static TDigestState read(StreamInput in) throws IOException { - return read(WrapperTDigestArrays.INSTANCE, in); + return read(MemoryTrackingTDigestArrays.INSTANCE, in); } public static TDigestState read(TDigestArrays arrays, StreamInput in) throws IOException { @@ -267,4 +268,9 @@ public final double getMin() { public final double getMax() { return tdigest.getMax(); } + + @Override + public void close() { + Releasables.close(tdigest); + } } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/StoredFieldsPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/StoredFieldsPhase.java index b3211f0b1e31c..17b57645d7d5f 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/StoredFieldsPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/StoredFieldsPhase.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /** * Process stored fields loaded from a HitContext into DocumentFields @@ -42,7 +43,8 @@ List process(Map> loadedFields) { if (inputs == null) { return List.of(); } - return inputs.stream().map(ft::valueForDisplay).toList(); + // This is eventually provided to DocumentField, which needs this collection to be mutable + return inputs.stream().map(ft::valueForDisplay).collect(Collectors.toList()); } boolean hasValue(Map> loadedFields) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index 71b52dc41705b..93f546eb288b9 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -2307,6 +2307,60 @@ public void testSubobjectsFalseFlattened() throws Exception { assertNotNull(doc.rootDoc().getField("attributes.simple.attribute")); } + public void testSubobjectsAutoFlattened() throws Exception { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("attributes"); + { + b.field("dynamic", false); + b.field("subobjects", "auto"); + b.startObject("properties"); + { + b.startObject("simple.attribute").field("type", "keyword").endObject(); + b.startObject("complex.attribute").field("type", "flattened").endObject(); + b.startObject("path").field("type", "object"); + { + b.field("store_array_source", "true").field("subobjects", "auto"); + b.startObject("properties"); + { + b.startObject("nested.attribute").field("type", "keyword").endObject(); + } + b.endObject(); + } + b.endObject(); + b.startObject("flattened_object").field("type", "object"); + { + b.startObject("properties"); + { + b.startObject("nested.attribute").field("type", "keyword").endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + })); + ParsedDocument doc = mapper.parse(source(""" + { + "attributes": { + "complex.attribute": { + "foo" : "bar" + }, + "simple.attribute": "sa", + "path": { + "nested.attribute": "na" + }, + "flattened_object.nested.attribute": "fna" + } + } + """)); + assertNotNull(doc.rootDoc().getField("attributes.complex.attribute")); + assertNotNull(doc.rootDoc().getField("attributes.simple.attribute")); + assertNotNull(doc.rootDoc().getField("attributes.path.nested.attribute")); + assertNotNull(doc.rootDoc().getField("attributes.flattened_object.nested.attribute")); + } + public void testWriteToFieldAlias() throws Exception { DocumentMapper mapper = createDocumentMapper(mapping(b -> { b.startObject("alias-field"); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DotExpandingXContentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DotExpandingXContentParserTests.java index b38c65c1710d6..c4e223a4d1b77 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DotExpandingXContentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DotExpandingXContentParserTests.java @@ -13,9 +13,12 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.json.JsonXContent; +import org.hamcrest.Matchers; import java.io.IOException; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -26,7 +29,7 @@ private void assertXContentMatches(String dotsExpanded, String withDots) throws final ContentPath contentPath = new ContentPath(); try ( XContentParser inputParser = createParser(JsonXContent.jsonXContent, withDots); - XContentParser expandedParser = DotExpandingXContentParser.expandDots(inputParser, contentPath) + XContentParser expandedParser = DotExpandingXContentParser.expandDots(inputParser, contentPath, null) ) { expandedParser.allowDuplicateKeys(true); @@ -37,7 +40,7 @@ private void assertXContentMatches(String dotsExpanded, String withDots) throws expectedParser.allowDuplicateKeys(true); try ( var p = createParser(JsonXContent.jsonXContent, withDots); - XContentParser actualParser = DotExpandingXContentParser.expandDots(p, contentPath) + XContentParser actualParser = DotExpandingXContentParser.expandDots(p, contentPath, null) ) { XContentParser.Token currentToken; while ((currentToken = actualParser.nextToken()) != null) { @@ -127,7 +130,7 @@ public void testDuplicateKeys() throws IOException { public void testDotsCollapsingFlatPaths() throws IOException { ContentPath contentPath = new ContentPath(); XContentParser parser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, """ - {"metrics.service.time": 10, "metrics.service.time.max": 500, "metrics.foo": "value"}"""), contentPath); + {"metrics.service.time": 10, "metrics.service.time.max": 500, "metrics.foo": "value"}"""), contentPath, null); parser.nextToken(); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("metrics", parser.currentName()); @@ -197,7 +200,7 @@ public void testDotsCollapsingStructuredPath() throws IOException { }, "foo" : "value" } - }"""), contentPath); + }"""), contentPath, null); parser.nextToken(); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("metrics", parser.currentName()); @@ -235,7 +238,7 @@ public void testDotsCollapsingStructuredPath() throws IOException { public void testSkipChildren() throws IOException { XContentParser parser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, """ - { "test.with.dots" : "value", "nodots" : "value2" }"""), new ContentPath()); + { "test.with.dots" : "value", "nodots" : "value2" }"""), new ContentPath(), null); parser.nextToken(); // start object assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("test", parser.currentName()); @@ -258,7 +261,7 @@ public void testSkipChildren() throws IOException { public void testSkipChildrenWithinInnerObject() throws IOException { XContentParser parser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, """ - { "test.with.dots" : {"obj" : {"field":"value"}}, "nodots" : "value2" }"""), new ContentPath()); + { "test.with.dots" : {"obj" : {"field":"value"}}, "nodots" : "value2" }"""), new ContentPath(), null); parser.nextToken(); // start object assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); @@ -306,7 +309,8 @@ public void testGetTokenLocation() throws IOException { XContentParser expectedParser = createParser(JsonXContent.jsonXContent, jsonInput); XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, jsonInput), - new ContentPath() + new ContentPath(), + null ); assertEquals(expectedParser.getTokenLocation(), dotExpandedParser.getTokenLocation()); @@ -364,7 +368,8 @@ public void testGetTokenLocation() throws IOException { public void testParseMapUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, dotExpandedParser::map); } @@ -372,7 +377,8 @@ public void testParseMapUOE() throws Exception { public void testParseMapOrderedUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, dotExpandedParser::mapOrdered); } @@ -380,7 +386,8 @@ public void testParseMapOrderedUOE() throws Exception { public void testParseMapStringsUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, dotExpandedParser::mapStrings); } @@ -388,7 +395,8 @@ public void testParseMapStringsUOE() throws Exception { public void testParseMapSupplierUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, () -> dotExpandedParser.map(HashMap::new, XContentParser::text)); } @@ -403,7 +411,8 @@ public void testParseMap() throws Exception { contentPath.setWithinLeafObject(true); XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, jsonInput), - contentPath + contentPath, + null ); assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); @@ -418,7 +427,8 @@ public void testParseMap() throws Exception { public void testParseListUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, dotExpandedParser::list); } @@ -426,7 +436,8 @@ public void testParseListUOE() throws Exception { public void testParseListOrderedUOE() throws Exception { XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, ""), - new ContentPath() + new ContentPath(), + null ); expectThrows(UnsupportedOperationException.class, dotExpandedParser::listOrderedMap); } @@ -440,7 +451,8 @@ public void testParseList() throws Exception { contentPath.setWithinLeafObject(true); XContentParser dotExpandedParser = DotExpandingXContentParser.expandDots( createParser(JsonXContent.jsonXContent, jsonInput), - contentPath + contentPath, + null ); assertEquals(XContentParser.Token.START_OBJECT, dotExpandedParser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, dotExpandedParser.nextToken()); @@ -450,4 +462,104 @@ public void testParseList() throws Exception { assertEquals("one", list.get(0)); assertEquals("two", list.get(1)); } + + private static DocumentParserContext createContext(XContentBuilder builder) throws IOException { + var documentMapper = new MapperServiceTestCase() { + }.createDocumentMapper(builder); + return new TestDocumentParserContext(documentMapper.mappers(), null); + } + + private static List getSubPaths(XContentBuilder builder, String... path) throws IOException { + DocumentParserContext context = createContext(builder); + return DotExpandingXContentParser.maybeFlattenPaths(Arrays.stream(path).toList(), context, new ContentPath()); + } + + private static List getSubPaths(XContentBuilder builder, List contentPath, List path) throws IOException { + DocumentParserContext context = createContext(builder); + ContentPath content = new ContentPath(); + for (String c : contentPath) { + content.add(c); + } + return DotExpandingXContentParser.maybeFlattenPaths(path, context, content); + } + + public void testAutoFlattening() throws Exception { + var b = XContentBuilder.builder(XContentType.JSON.xContent()); + b.startObject().startObject("_doc"); + { + b.field("subobjects", "auto"); + b.startObject("properties"); + { + b.startObject("path").startObject("properties"); + { + b.startObject("to").startObject("properties"); + { + b.startObject("field").field("type", "integer").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + b.startObject("path.auto").field("subobjects", "auto").startObject("properties"); + { + b.startObject("to").startObject("properties"); + { + b.startObject("some.field").field("type", "integer").endObject(); + } + b.endObject().endObject(); + b.startObject("inner.enabled").field("dynamic", "false").startObject("properties"); + { + b.startObject("field").field("type", "integer").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + b.startObject("path.disabled").field("subobjects", "false").startObject("properties"); + { + b.startObject("to").startObject("properties"); + { + b.startObject("some.field").field("type", "integer").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + } + b.endObject(); + } + b.endObject().endObject(); + + // inner [subobjects:enabled] gets flattened + assertThat(getSubPaths(b, "field"), Matchers.contains("field")); + assertThat(getSubPaths(b, "path", "field"), Matchers.contains("path.field")); + assertThat(getSubPaths(b, "path", "to", "field"), Matchers.contains("path.to.field")); + assertThat(getSubPaths(b, "path", "to", "any"), Matchers.contains("path.to.any")); + + // inner [subobjects:auto] does not get flattened + assertThat(getSubPaths(b, "path", "auto", "field"), Matchers.contains("path.auto", "field")); + assertThat(getSubPaths(b, "path", "auto", "some", "field"), Matchers.contains("path.auto", "some.field")); + assertThat(getSubPaths(b, "path", "auto", "to", "some", "field"), Matchers.contains("path.auto", "to.some.field")); + assertThat(getSubPaths(b, "path", "auto", "to", "some", "other"), Matchers.contains("path.auto", "to.some.other")); + assertThat(getSubPaths(b, "path", "auto", "inner", "enabled", "field"), Matchers.contains("path.auto", "inner.enabled", "field")); + assertThat( + getSubPaths(b, "path", "auto", "inner", "enabled", "to", "some", "field"), + Matchers.contains("path.auto", "inner.enabled", "to", "some", "field") + ); + + // inner [subobjects:disabled] gets flattened + assertThat(getSubPaths(b, "path", "disabled", "field"), Matchers.contains("path.disabled.field")); + assertThat(getSubPaths(b, "path", "disabled", "some", "field"), Matchers.contains("path.disabled.some.field")); + assertThat(getSubPaths(b, "path", "disabled", "to", "some", "field"), Matchers.contains("path.disabled.to.some.field")); + assertThat(getSubPaths(b, "path", "disabled", "to", "some", "other"), Matchers.contains("path.disabled.to.some.other")); + + // Non-empty content path. + assertThat(getSubPaths(b, List.of("path"), List.of("field")), Matchers.contains("field")); + assertThat(getSubPaths(b, List.of("path"), List.of("to", "field")), Matchers.contains("to", "field")); + assertThat(getSubPaths(b, List.of("path", "to"), List.of("field")), Matchers.contains("field")); + assertThat(getSubPaths(b, List.of("path"), List.of("auto", "field")), Matchers.contains("auto", "field")); + assertThat(getSubPaths(b, List.of("path", "auto"), List.of("to", "some", "field")), Matchers.contains("to.some.field")); + assertThat( + getSubPaths(b, List.of("path", "auto"), List.of("inner", "enabled", "to", "some", "field")), + Matchers.contains("inner.enabled", "to", "some", "field") + ); + assertThat(getSubPaths(b, List.of("path", "disabled"), List.of("to", "some", "field")), Matchers.contains("to", "some", "field")); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index 7f430cf676809..43ee47245f492 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -1619,10 +1619,9 @@ public void testSubobjectsAutoWithInnerNestedFromDynamicTemplate() throws IOExce assertNotNull(doc.rootDoc().get("metrics.time.max")); assertNotNull(doc.docs().get(0).get("metrics.time.foo")); - assertThat( - ((ObjectMapper) doc.dynamicMappingsUpdate().getRoot().getMapper("metrics")).getMapper("time"), - instanceOf(NestedObjectMapper.class) - ); + var metrics = ((ObjectMapper) doc.dynamicMappingsUpdate().getRoot().getMapper("metrics")); + assertThat(metrics.getMapper("time"), instanceOf(NestedObjectMapper.class)); + assertThat(metrics.getMapper("time.max"), instanceOf(NumberFieldMapper.class)); } public void testDynamicSubobject() throws IOException { @@ -2057,7 +2056,7 @@ public void testSubobjectsAutoFlattened() throws IOException { "dynamic_templates": [ { "test": { - "path_match": "attributes.resource.*", + "path_match": "attributes.*", "match_mapping_type": "object", "mapping": { "type": "flattened" @@ -2070,7 +2069,7 @@ public void testSubobjectsAutoFlattened() throws IOException { """; String docJson = """ { - "attributes.resource": { + "attributes": { "complex.attribute": { "a": "b" }, @@ -2083,14 +2082,67 @@ public void testSubobjectsAutoFlattened() throws IOException { ParsedDocument parsedDoc = mapperService.documentMapper().parse(source(docJson)); merge(mapperService, dynamicMapping(parsedDoc.dynamicMappingsUpdate())); - Mapper fooBarMapper = mapperService.documentMapper().mappers().getMapper("attributes.resource.foo.bar"); + Mapper fooBarMapper = mapperService.documentMapper().mappers().getMapper("attributes.foo.bar"); assertNotNull(fooBarMapper); assertEquals("text", fooBarMapper.typeName()); - Mapper fooStructuredMapper = mapperService.documentMapper().mappers().getMapper("attributes.resource.complex.attribute"); + Mapper fooStructuredMapper = mapperService.documentMapper().mappers().getMapper("attributes.complex.attribute"); assertNotNull(fooStructuredMapper); assertEquals("flattened", fooStructuredMapper.typeName()); } + public void testSubobjectsAutoWithObjectInDynamicTemplate() throws IOException { + String mapping = """ + { + "_doc": { + "properties": { + "attributes": { + "type": "object", + "subobjects": "auto" + } + }, + "dynamic_templates": [ + { + "test": { + "path_match": "attributes.*", + "match_mapping_type": "object", + "mapping": { + "type": "object", + "dynamic": "false", + "properties": { + "id": { + "type": "integer" + } + } + } + } + } + ] + } + } + """; + String docJson = """ + { + "attributes": { + "to": { + "id": 10 + }, + "foo.bar": "baz" + } + } + """; + + MapperService mapperService = createMapperService(mapping); + ParsedDocument parsedDoc = mapperService.documentMapper().parse(source(docJson)); + merge(mapperService, dynamicMapping(parsedDoc.dynamicMappingsUpdate())); + + Mapper fooBarMapper = mapperService.documentMapper().mappers().getMapper("attributes.foo.bar"); + assertNotNull(fooBarMapper); + assertEquals("text", fooBarMapper.typeName()); + Mapper innerObject = mapperService.documentMapper().mappers().objectMappers().get("attributes.to"); + assertNotNull(innerObject); + assertEquals("integer", mapperService.documentMapper().mappers().getMapper("attributes.to.id").typeName()); + } + public void testMatchWithArrayOfFieldNames() throws IOException { String mapping = """ { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java index eaa7bf6528203..5d5273f0fc788 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java @@ -1549,6 +1549,66 @@ public void testCopyToLogicInsideObject() throws IOException { assertEquals("{\"path\":{\"at\":\"A\"}}", syntheticSource); } + public void testCopyToRootWithSubobjectFlattening() throws IOException { + DocumentMapper documentMapper = createMapperService(topMapping(b -> { + b.startObject("_source").field("mode", "synthetic").endObject(); + b.field("subobjects", randomFrom("false", "auto")); + b.startObject("properties"); + { + b.startObject("k").field("type", "keyword").field("copy_to", "a.b.c").endObject(); + b.startObject("a").startObject("properties"); + { + b.startObject("b").startObject("properties"); + { + b.startObject("c").field("type", "keyword").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + } + b.endObject(); + })).documentMapper(); + + CheckedConsumer document = b -> b.field("k", "hey"); + + var doc = documentMapper.parse(source(document)); + assertNotNull(doc.docs().get(0).getField("a.b.c")); + + var syntheticSource = syntheticSource(documentMapper, document); + assertEquals("{\"k\":\"hey\"}", syntheticSource); + } + + public void testCopyToObjectWithSubobjectFlattening() throws IOException { + DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> { + b.startObject("path").field("subobjects", randomFrom("false", "auto")).startObject("properties"); + { + b.startObject("k").field("type", "keyword").field("copy_to", "path.a.b.c").endObject(); + b.startObject("a").startObject("properties"); + { + b.startObject("b").startObject("properties"); + { + b.startObject("c").field("type", "keyword").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + })).documentMapper(); + + CheckedConsumer document = b -> { + b.startObject("path"); + b.field("k", "hey"); + b.endObject(); + }; + + var doc = documentMapper.parse(source(document)); + assertNotNull(doc.docs().get(0).getField("path.a.b.c")); + + var syntheticSource = syntheticSource(documentMapper, document); + assertEquals("{\"path\":{\"k\":\"hey\"}}", syntheticSource); + } + protected void validateRoundTripReader(String syntheticSource, DirectoryReader reader, DirectoryReader roundTripReader) throws IOException { // We exclude ignored source field since in some cases it contains an exact copy of a part of document source. diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java index 3312c94e8a0e1..4bc91b793d049 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java @@ -354,12 +354,8 @@ public void testSubobjectsFalse() throws Exception { b.field("subobjects", false); b.startObject("properties"); { - b.startObject("time"); - b.field("type", "long"); - b.endObject(); - b.startObject("time.max"); - b.field("type", "long"); - b.endObject(); + b.startObject("time").field("type", "long").endObject(); + b.startObject("time.max").field("type", "long").endObject(); } b.endObject(); } @@ -380,9 +376,7 @@ public void testSubobjectsFalseWithInnerObject() throws IOException { { b.startObject("properties"); { - b.startObject("max"); - b.field("type", "long"); - b.endObject(); + b.startObject("max").field("type", "long").endObject(); } b.endObject(); } @@ -403,9 +397,7 @@ public void testSubobjectsFalseWithInnerNested() { b.field("subobjects", false); b.startObject("properties"); { - b.startObject("time"); - b.field("type", "nested"); - b.endObject(); + b.startObject("time").field("type", "nested").endObject(); } b.endObject(); } @@ -419,12 +411,8 @@ public void testSubobjectsFalseWithInnerNested() { public void testSubobjectsFalseRoot() throws Exception { MapperService mapperService = createMapperService(mappingNoSubobjects(b -> { - b.startObject("metrics.service.time"); - b.field("type", "long"); - b.endObject(); - b.startObject("metrics.service.time.max"); - b.field("type", "long"); - b.endObject(); + b.startObject("metrics.service.time").field("type", "long").endObject(); + b.startObject("metrics.service.time.max").field("type", "long").endObject(); })); assertNotNull(mapperService.fieldType("metrics.service.time")); assertNotNull(mapperService.fieldType("metrics.service.time.max")); @@ -441,9 +429,7 @@ public void testSubobjectsFalseRootWithInnerObject() throws IOException { { b.startObject("properties"); { - b.startObject("max"); - b.field("type", "long"); - b.endObject(); + b.startObject("max").field("type", "long").endObject(); } b.endObject(); } @@ -455,9 +441,7 @@ public void testSubobjectsFalseRootWithInnerObject() throws IOException { public void testSubobjectsFalseRootWithInnerNested() { MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createMapperService(mappingNoSubobjects(b -> { - b.startObject("metrics.service"); - b.field("type", "nested"); - b.endObject(); + b.startObject("metrics.service").field("type", "nested").endObject(); }))); assertEquals( "Failed to parse mapping: Tried to add nested object [metrics.service] to object [_doc] which does not support subobjects", @@ -473,8 +457,7 @@ public void testSubobjectsCannotBeUpdated() throws IOException { "_doc", MergeReason.MAPPING_UPDATE, new CompressedXContent(BytesReference.bytes(fieldMapping(b -> { - b.field("type", "object"); - b.field("subobjects", "false"); + b.field("type", "object").field("subobjects", "false"); }))) ); MapperException exception = expectThrows( @@ -509,12 +492,8 @@ public void testSubobjectsAuto() throws Exception { b.field("subobjects", "auto"); b.startObject("properties"); { - b.startObject("time"); - b.field("type", "long"); - b.endObject(); - b.startObject("time.max"); - b.field("type", "long"); - b.endObject(); + b.startObject("time").field("type", "long").endObject(); + b.startObject("time.max").field("type", "long").endObject(); b.startObject("attributes"); { b.field("type", "object"); @@ -531,7 +510,7 @@ public void testSubobjectsAuto() throws Exception { assertNotNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.attributes")); } - public void testSubobjectsAutoWithInnerObject() throws IOException { + public void testSubobjectsAutoWithInnerFlattenableObject() throws IOException { MapperService mapperService = createMapperService(mapping(b -> { b.startObject("metrics.service"); { @@ -542,16 +521,12 @@ public void testSubobjectsAutoWithInnerObject() throws IOException { { b.startObject("properties"); { - b.startObject("max"); - b.field("type", "long"); - b.endObject(); + b.startObject("max").field("type", "long").endObject(); } b.endObject(); } b.endObject(); - b.startObject("foo"); - b.field("type", "keyword"); - b.endObject(); + b.startObject("foo").field("type", "keyword").endObject(); } b.endObject(); } @@ -560,11 +535,11 @@ public void testSubobjectsAutoWithInnerObject() throws IOException { assertNull(mapperService.fieldType("metrics.service.time")); assertNotNull(mapperService.fieldType("metrics.service.time.max")); assertNotNull(mapperService.fieldType("metrics.service.foo")); - assertNotNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time")); + assertNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time")); // Gets flattened. assertNotNull(mapperService.documentMapper().mappers().getMapper("metrics.service.foo")); } - public void testSubobjectsAutoWithInnerNested() throws IOException { + public void testSubobjectsAutoWithInnerNonFlattenableObject() throws IOException { MapperService mapperService = createMapperService(mapping(b -> { b.startObject("metrics.service"); { @@ -572,8 +547,36 @@ public void testSubobjectsAutoWithInnerNested() throws IOException { b.startObject("properties"); { b.startObject("time"); - b.field("type", "nested"); + { + b.field(ObjectMapper.STORE_ARRAY_SOURCE_PARAM, true); + b.startObject("properties"); + { + b.startObject("max").field("type", "long").endObject(); + } + b.endObject(); + } b.endObject(); + b.startObject("foo").field("type", "keyword").endObject(); + } + b.endObject(); + } + b.endObject(); + })); + assertNull(mapperService.fieldType("metrics.service.time")); + assertNotNull(mapperService.fieldType("metrics.service.time.max")); + assertNotNull(mapperService.fieldType("metrics.service.foo")); + assertNotNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time")); // Not flattened. + assertNotNull(mapperService.documentMapper().mappers().getMapper("metrics.service.foo")); + } + + public void testSubobjectsAutoWithInnerNested() throws IOException { + MapperService mapperService = createMapperService(mapping(b -> { + b.startObject("metrics.service"); + { + b.field("subobjects", "auto"); + b.startObject("properties"); + { + b.startObject("time").field("type", "nested").endObject(); } b.endObject(); } @@ -587,12 +590,8 @@ public void testSubobjectsAutoWithInnerNested() throws IOException { public void testSubobjectsAutoRoot() throws Exception { MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { - b.startObject("metrics.service.time"); - b.field("type", "long"); - b.endObject(); - b.startObject("metrics.service.time.max"); - b.field("type", "long"); - b.endObject(); + b.startObject("metrics.service.time").field("type", "long").endObject(); + b.startObject("metrics.service.time.max").field("type", "long").endObject(); b.startObject("metrics.attributes"); { b.field("type", "object"); @@ -605,15 +604,13 @@ public void testSubobjectsAutoRoot() throws Exception { assertNotNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.attributes")); } - public void testSubobjectsAutoRootWithInnerObject() throws IOException { + public void testSubobjectsAutoRootWithInnerFlattenableObject() throws IOException { MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { b.startObject("metrics.service.time"); { b.startObject("properties"); { - b.startObject("max"); - b.field("type", "long"); - b.endObject(); + b.startObject("max").field("type", "long").endObject(); } b.endObject(); } @@ -621,8 +618,48 @@ public void testSubobjectsAutoRootWithInnerObject() throws IOException { }, "auto")); assertNull(mapperService.fieldType("metrics.service.time")); assertNotNull(mapperService.fieldType("metrics.service.time.max")); - assertNotNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time")); - assertNotNull(mapperService.documentMapper().mappers().getMapper("metrics.service.time.max")); + assertNull(mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time")); // Gets flattened. + + Mapper innerField = mapperService.documentMapper().mappers().getMapper("metrics.service.time.max"); + assertNotNull(innerField); + assertEquals("metrics.service.time.max", innerField.leafName()); + } + + public void testSubobjectsAutoRootWithInnerNonFlattenableObject() throws IOException { + MapperService mapperService = createMapperService(mappingWithSubobjects(b -> { + b.startObject("metrics").startObject("properties"); + { + b.startObject("service.time"); + { + b.field("subobjects", "auto"); + b.startObject("properties"); + { + b.startObject("path").startObject("properties"); + { + b.startObject("to").startObject("properties"); + { + b.startObject("max").field("type", "long").endObject(); + } + b.endObject().endObject(); + } + b.endObject().endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject().endObject(); + }, "auto")); + assertNull(mapperService.fieldType("metrics.service.time")); + assertNotNull(mapperService.fieldType("metrics.service.time.path.to.max")); + + ObjectMapper innerObject = mapperService.documentMapper().mappers().objectMappers().get("metrics.service.time"); // Not flattened. + assertNotNull(innerObject); + assertEquals("metrics.service.time", innerObject.leafName()); + + Mapper innerField = mapperService.documentMapper().mappers().getMapper("metrics.service.time.path.to.max"); + assertNotNull(innerField); + assertEquals("path.to.max", innerField.leafName()); } public void testSubobjectsAutoRootWithInnerNested() throws IOException { @@ -742,16 +779,7 @@ public void testFlatten() { ObjectMapper objectMapper = new ObjectMapper.Builder("parent", Optional.empty()).add( new ObjectMapper.Builder("child", Optional.empty()).add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())) ).add(new KeywordFieldMapper.Builder("keyword1", IndexVersion.current())).build(rootContext); - List fields = objectMapper.asFlattenedFieldMappers(rootContext).stream().map(FieldMapper::fullPath).toList(); - assertThat(fields, containsInAnyOrder("parent.keyword1", "parent.child.keyword2")); - } - - public void testFlattenSubobjectsAuto() { - MapperBuilderContext rootContext = MapperBuilderContext.root(false, false); - ObjectMapper objectMapper = new ObjectMapper.Builder("parent", Optional.of(ObjectMapper.Subobjects.AUTO)).add( - new ObjectMapper.Builder("child", Optional.empty()).add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())) - ).add(new KeywordFieldMapper.Builder("keyword1", IndexVersion.current())).build(rootContext); - List fields = objectMapper.asFlattenedFieldMappers(rootContext).stream().map(FieldMapper::fullPath).toList(); + List fields = objectMapper.asFlattenedFieldMappers(rootContext, true).stream().map(Mapper::fullPath).toList(); assertThat(fields, containsInAnyOrder("parent.keyword1", "parent.child.keyword2")); } @@ -760,7 +788,7 @@ public void testFlattenSubobjectsFalse() { ObjectMapper objectMapper = new ObjectMapper.Builder("parent", Optional.of(ObjectMapper.Subobjects.DISABLED)).add( new ObjectMapper.Builder("child", Optional.empty()).add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())) ).add(new KeywordFieldMapper.Builder("keyword1", IndexVersion.current())).build(rootContext); - List fields = objectMapper.asFlattenedFieldMappers(rootContext).stream().map(FieldMapper::fullPath).toList(); + List fields = objectMapper.asFlattenedFieldMappers(rootContext, true).stream().map(Mapper::fullPath).toList(); assertThat(fields, containsInAnyOrder("parent.keyword1", "parent.child.keyword2")); } @@ -772,7 +800,7 @@ public void testFlattenDynamicIncompatible() { IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> objectMapper.asFlattenedFieldMappers(rootContext) + () -> objectMapper.asFlattenedFieldMappers(rootContext, true) ); assertEquals( "Object mapper [parent.child] was found in a context where subobjects is set to false. " @@ -788,7 +816,7 @@ public void testFlattenEnabledFalse() { IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> objectMapper.asFlattenedFieldMappers(rootContext) + () -> objectMapper.asFlattenedFieldMappers(rootContext, true) ); assertEquals( "Object mapper [parent] was found in a context where subobjects is set to false. " @@ -797,13 +825,30 @@ public void testFlattenEnabledFalse() { ); } + public void testFlattenSubobjectsAuto() { + MapperBuilderContext rootContext = MapperBuilderContext.root(false, false); + ObjectMapper objectMapper = new ObjectMapper.Builder("parent", Optional.of(ObjectMapper.Subobjects.AUTO)).add( + new ObjectMapper.Builder("child", Optional.empty()).add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())) + ).add(new KeywordFieldMapper.Builder("keyword1", IndexVersion.current())).build(rootContext); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> objectMapper.asFlattenedFieldMappers(rootContext, true) + ); + assertEquals( + "Object mapper [parent] was found in a context where subobjects is set to false. " + + "Auto-flattening [parent] failed because the value of [subobjects] is [auto]", + exception.getMessage() + ); + } + public void testFlattenExplicitSubobjectsTrue() { MapperBuilderContext rootContext = MapperBuilderContext.root(false, false); ObjectMapper objectMapper = new ObjectMapper.Builder("parent", Optional.of(ObjectMapper.Subobjects.ENABLED)).build(rootContext); IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> objectMapper.asFlattenedFieldMappers(rootContext) + () -> objectMapper.asFlattenedFieldMappers(rootContext, true) ); assertEquals( "Object mapper [parent] was found in a context where subobjects is set to false. " diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java index 46d03275ac3ce..619714119a05e 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesLifecycleListenerSingleNodeTests.java @@ -128,6 +128,8 @@ public void afterIndexRemoved(Index index, IndexSettings indexSettings, IndexRem newRouting = ShardRoutingHelper.moveToStarted(newRouting); IndexShardTestCase.updateRoutingEntry(shard, newRouting); assertEquals(6, counter.get()); + } catch (Exception ex) { + logger.warn("unexpected exception", ex); } finally { indicesService.removeIndex(idx, DELETED, "simon says", EsExecutors.DIRECT_EXECUTOR_SERVICE, ActionListener.noop()); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArraysTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArraysTests.java new file mode 100644 index 0000000000000..e57f39becef7c --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MemoryTrackingTDigestArraysTests.java @@ -0,0 +1,360 @@ +/* + * 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.search.aggregations.metrics; + +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.tdigest.arrays.TDigestArrays; +import org.elasticsearch.tdigest.arrays.TDigestByteArray; +import org.elasticsearch.tdigest.arrays.TDigestDoubleArray; +import org.elasticsearch.tdigest.arrays.TDigestIntArray; +import org.elasticsearch.tdigest.arrays.TDigestLongArray; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; + +public class MemoryTrackingTDigestArraysTests extends ESTestCase { + // Int arrays + + public void testIntEmpty() { + try (TDigestIntArray array = intArray(0)) { + assertThat(array.size(), equalTo(0)); + } + } + + public void testIntGetAndSet() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestIntArray array = intArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + int value = randomInt(); + for (int i = 9; i < initialSize; i++) { + array.set(i, value); + } + + for (int i = 0; i < initialSize; i++) { + if (i < 9) { + assertThat(array.get(i), equalTo(0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testIntResize() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestIntArray array = intArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + // Fill with a non-zero value + int value = randomBoolean() ? randomIntBetween(Integer.MIN_VALUE, -1) : randomIntBetween(1, Integer.MAX_VALUE); + for (int i = 0; i < initialSize; i++) { + array.set(i, value); + } + + // Resize to a size-1 + array.resize(initialSize - 1); + assertThat(array.size(), equalTo(initialSize - 1)); + + for (int i = 0; i < initialSize - 1; i++) { + assertThat(array.get(i), equalTo(value)); + } + + // Resize to the original size + 1 + array.resize(initialSize + 1); + assertThat(array.size(), equalTo(initialSize + 1)); + + // Ensure all new elements are 0 + for (int i = 0; i < initialSize - 1; i++) { + if (i < initialSize) { + assertThat(array.get(i), equalTo(value)); + } else { + assertThat(array.get(i), equalTo(0)); + } + } + } + } + + public void testIntBulkSet() { + int initialSize = randomIntBetween(10, 1000); + int sourceArraySize = randomIntBetween(0, initialSize); + + try (TDigestIntArray array = intArray(initialSize); TDigestIntArray source = intArray(sourceArraySize)) { + assertThat(array.size(), equalTo(initialSize)); + assertThat(source.size(), equalTo(sourceArraySize)); + + int value = randomInt(); + for (int i = 0; i < sourceArraySize; i++) { + source.set(i, value); + } + + int initialOffset = randomIntBetween(0, initialSize - sourceArraySize); + int sourceOffset = randomIntBetween(0, sourceArraySize - 1); + int elementsToCopy = randomIntBetween(1, sourceArraySize - sourceOffset); + + array.set(initialOffset, source, sourceOffset, elementsToCopy); + + for (int i = 0; i < initialSize; i++) { + if (i < initialOffset || i >= initialOffset + elementsToCopy) { + assertThat(array.get(i), equalTo(0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + // Long arrays + + public void testLongEmpty() { + try (TDigestIntArray array = intArray(0)) { + assertThat(array.size(), equalTo(0)); + } + } + + public void testLongGetAndSet() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestLongArray array = longArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + long value = randomLong(); + for (int i = 9; i < initialSize; i++) { + array.set(i, value); + } + + for (int i = 0; i < initialSize; i++) { + if (i < 9) { + assertThat(array.get(i), equalTo(0L)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testLongResize() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestLongArray array = longArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + // Fill with a non-zero value + long value = randomBoolean() ? randomLongBetween(Long.MIN_VALUE, -1) : randomLongBetween(1, Long.MAX_VALUE); + for (int i = 0; i < initialSize; i++) { + array.set(i, value); + } + + // Resize to a size-1 + array.resize(initialSize - 1); + assertThat(array.size(), equalTo(initialSize - 1)); + + for (int i = 0; i < initialSize - 1; i++) { + assertThat(array.get(i), equalTo(value)); + } + + // Resize to the original size + 1 + array.resize(initialSize + 1); + assertThat(array.size(), equalTo(initialSize + 1)); + + // Ensure all new elements are 0 + for (int i = 0; i < initialSize - 1; i++) { + if (i < initialSize) { + assertThat(array.get(i), equalTo(value)); + } else { + assertThat(array.get(i), equalTo(0L)); + } + } + } + } + + // Byte arrays + + public void testByteEmpty() { + try (TDigestByteArray array = byteArray(0)) { + assertThat(array.size(), equalTo(0)); + } + } + + public void testByteGetAndSet() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestByteArray array = byteArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + byte value = randomByte(); + for (int i = 9; i < initialSize; i++) { + array.set(i, value); + } + + for (int i = 0; i < initialSize; i++) { + if (i < 9) { + assertThat(array.get(i), equalTo((byte) 0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testByteResize() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestByteArray array = byteArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + // Fill with a non-zero value + byte value = randomBoolean() ? randomByteBetween(Byte.MIN_VALUE, (byte) -1) : randomByteBetween((byte) 1, Byte.MAX_VALUE); + for (int i = 0; i < initialSize; i++) { + array.set(i, value); + } + + // Resize to a size-1 + array.resize(initialSize - 1); + assertThat(array.size(), equalTo(initialSize - 1)); + + for (int i = 0; i < initialSize - 1; i++) { + assertThat(array.get(i), equalTo(value)); + } + + // Resize to the original size + 1 + array.resize(initialSize + 1); + assertThat(array.size(), equalTo(initialSize + 1)); + + // Ensure all new elements are 0 + for (int i = 0; i < initialSize - 1; i++) { + if (i < initialSize) { + assertThat(array.get(i), equalTo(value)); + } else { + assertThat(array.get(i), equalTo((byte) 0)); + } + } + } + } + + // Double arrays + + public void testDoubleEmpty() { + try (TDigestDoubleArray array = doubleArray(0)) { + assertThat(array.size(), equalTo(0)); + } + } + + public void testDoubleGetAndSet() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestDoubleArray array = doubleArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + double value = randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true); + for (int i = 9; i < initialSize; i++) { + array.set(i, value); + } + + for (int i = 0; i < initialSize; i++) { + if (i < 9) { + assertThat(array.get(i), equalTo(0.0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testDoubleAdd() { + int initialSize = randomIntBetween(10, 1000); + try (TDigestDoubleArray array = doubleArray(initialSize)) { + assertThat(array.size(), equalTo(initialSize)); + + int newValueCount = randomIntBetween(1, 100); + if (randomBoolean()) { + array.ensureCapacity(initialSize + newValueCount); + } + double value = randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true); + for (int i = 0; i < newValueCount; i++) { + array.add(value); + } + + for (int i = 0; i < newValueCount; i++) { + if (i < initialSize) { + assertThat(array.get(i), equalTo(0.0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testDoubleBulkSet() { + int initialSize = randomIntBetween(10, 1000); + int sourceArraySize = randomIntBetween(0, initialSize); + + try (TDigestDoubleArray array = doubleArray(initialSize); TDigestDoubleArray source = doubleArray(sourceArraySize)) { + assertThat(array.size(), equalTo(initialSize)); + assertThat(source.size(), equalTo(sourceArraySize)); + + double value = randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true); + for (int i = 0; i < sourceArraySize; i++) { + source.set(i, value); + } + + int initialOffset = randomIntBetween(0, initialSize - sourceArraySize); + int sourceOffset = randomIntBetween(0, sourceArraySize - 1); + int elementsToCopy = randomIntBetween(1, sourceArraySize - sourceOffset); + + array.set(initialOffset, source, sourceOffset, elementsToCopy); + + for (int i = 0; i < initialSize; i++) { + if (i < initialOffset || i >= initialOffset + elementsToCopy) { + assertThat(array.get(i), equalTo(0.0)); + } else { + assertThat(array.get(i), equalTo(value)); + } + } + } + } + + public void testDoubleSort() { + try (TDigestDoubleArray array = doubleArray(0)) { + int elementsToAdd = randomIntBetween(0, 100); + array.ensureCapacity(elementsToAdd); + for (int i = 0; i < elementsToAdd; i++) { + array.add(randomDoubleBetween(-Double.MAX_VALUE, Double.MAX_VALUE, true)); + } + + array.sort(); + + double previous = -Double.MAX_VALUE; + for (int i = 0; i < array.size(); i++) { + double current = array.get(i); + assertThat(current, greaterThanOrEqualTo(previous)); + previous = current; + } + } + } + + // Helpers + + private TDigestIntArray intArray(int initialSize) { + return arrays().newIntArray(initialSize); + } + + private TDigestLongArray longArray(int initialSize) { + return arrays().newLongArray(initialSize); + } + + private TDigestByteArray byteArray(int initialSize) { + return arrays().newByteArray(initialSize); + } + + private TDigestDoubleArray doubleArray(int initialSize) { + return arrays().newDoubleArray(initialSize); + } + + private TDigestArrays arrays() { + return new MemoryTrackingTDigestArrays(newLimitedBreaker(ByteSizeValue.ofMb(100))); + } +} diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java index e7799a133b5af..56d3d674b28ca 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestStateTests.java @@ -16,8 +16,9 @@ import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.Releasables; import org.elasticsearch.tdigest.arrays.TDigestArrays; -import org.elasticsearch.tdigest.arrays.WrapperTDigestArrays; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; @@ -33,140 +34,150 @@ public class TDigestStateTests extends ESTestCase { public void testMoreThan4BValues() { // Regression test for #19528 // See https://github.com/tdunning/t-digest/pull/70/files#diff-4487072cee29b939694825647928f742R439 - TDigestState digest = TDigestState.create(arrays(), 100); - for (int i = 0; i < 1000; ++i) { - digest.add(randomDouble()); - } - final int count = 1 << 29; - for (int i = 0; i < 10; ++i) { - digest.add(randomDouble(), count); - } - assertEquals(1000 + 10L * (1 << 29), digest.size()); - assertTrue(digest.size() > 2L * Integer.MAX_VALUE); - final double[] quantiles = new double[] { 0, 0.1, 0.5, 0.9, 1, randomDouble() }; - Arrays.sort(quantiles); - double prev = Double.NEGATIVE_INFINITY; - for (double q : quantiles) { - final double v = digest.quantile(q); - logger.trace("q=" + q + ", v=" + v); - assertThat(v, Matchers.either(Matchers.closeTo(prev, 0.0000001D)).or(Matchers.greaterThan(prev))); - assertTrue("Unexpectedly low value: " + v, v >= 0.0); - assertTrue("Unexpectedly high value: " + v, v <= 1.0); - prev = v; + try (TDigestState digest = TDigestState.create(arrays(), 100)) { + for (int i = 0; i < 1000; ++i) { + digest.add(randomDouble()); + } + final int count = 1 << 29; + for (int i = 0; i < 10; ++i) { + digest.add(randomDouble(), count); + } + assertEquals(1000 + 10L * (1 << 29), digest.size()); + assertTrue(digest.size() > 2L * Integer.MAX_VALUE); + final double[] quantiles = new double[] { 0, 0.1, 0.5, 0.9, 1, randomDouble() }; + Arrays.sort(quantiles); + double prev = Double.NEGATIVE_INFINITY; + for (double q : quantiles) { + final double v = digest.quantile(q); + logger.trace("q=" + q + ", v=" + v); + assertThat(v, Matchers.either(Matchers.closeTo(prev, 0.0000001D)).or(Matchers.greaterThan(prev))); + assertTrue("Unexpectedly low value: " + v, v >= 0.0); + assertTrue("Unexpectedly high value: " + v, v <= 1.0); + prev = v; + } } } public void testEqualsHashCode() { - final TDigestState empty1 = new EmptyTDigestState(); - final TDigestState empty2 = new EmptyTDigestState(); - final TDigestState a = TDigestState.create(arrays(), 200); - final TDigestState b = TDigestState.create(arrays(), 100); - final TDigestState c = TDigestState.create(arrays(), 100); + try ( + TDigestState empty1 = new EmptyTDigestState(); + TDigestState empty2 = new EmptyTDigestState(); + TDigestState a = TDigestState.create(arrays(), 200); + TDigestState b = TDigestState.create(arrays(), 100); + TDigestState c = TDigestState.create(arrays(), 100); + ) { - assertEquals(empty1, empty2); - assertEquals(empty1.hashCode(), empty2.hashCode()); + assertEquals(empty1, empty2); + assertEquals(empty1.hashCode(), empty2.hashCode()); - assertNotEquals(a, b); - assertNotEquals(a.hashCode(), b.hashCode()); + assertNotEquals(a, b); + assertNotEquals(a.hashCode(), b.hashCode()); - assertNotEquals(a, c); - assertNotEquals(a.hashCode(), c.hashCode()); + assertNotEquals(a, c); + assertNotEquals(a.hashCode(), c.hashCode()); - assertEquals(b, c); - assertEquals(b.hashCode(), c.hashCode()); + assertEquals(b, c); + assertEquals(b.hashCode(), c.hashCode()); - for (int i = 0; i < 100; i++) { - double value = randomDouble(); - a.add(value); - b.add(value); - c.add(value); - } + for (int i = 0; i < 100; i++) { + double value = randomDouble(); + a.add(value); + b.add(value); + c.add(value); + } - assertNotEquals(a, b); - assertNotEquals(a.hashCode(), b.hashCode()); + assertNotEquals(a, b); + assertNotEquals(a.hashCode(), b.hashCode()); - assertNotEquals(a, c); - assertNotEquals(a.hashCode(), c.hashCode()); + assertNotEquals(a, c); + assertNotEquals(a.hashCode(), c.hashCode()); - assertEquals(b, c); - assertEquals(b.hashCode(), c.hashCode()); + assertEquals(b, c); + assertEquals(b.hashCode(), c.hashCode()); - b.add(randomDouble()); - c.add(randomDouble()); + b.add(randomDouble()); + c.add(randomDouble()); - assertNotEquals(b, c); - assertNotEquals(b.hashCode(), c.hashCode()); + assertNotEquals(b, c); + assertNotEquals(b.hashCode(), c.hashCode()); + } } public void testHash() { final HashMap map = new HashMap<>(); final Set set = new HashSet<>(); - final TDigestState empty1 = new EmptyTDigestState(); - final TDigestState empty2 = new EmptyTDigestState(); - final TDigestState a = TDigestState.create(arrays(), 200); - final TDigestState b = TDigestState.create(arrays(), 100); - final TDigestState c = TDigestState.create(arrays(), 100); - - a.add(randomDouble()); - b.add(randomDouble()); - c.add(randomDouble()); - expectThrows(UnsupportedOperationException.class, () -> empty1.add(randomDouble())); - expectThrows(UnsupportedOperationException.class, () -> empty2.add(a)); - - map.put("empty1", empty1); - map.put("empty2", empty2); - map.put("a", a); - map.put("b", b); - map.put("c", c); - set.add(empty1); - set.add(empty2); - set.add(a); - set.add(b); - set.add(c); - - assertEquals(5, map.size()); - assertEquals(4, set.size()); - - assertEquals(empty1, map.get("empty1")); - assertEquals(empty2, map.get("empty2")); - assertEquals(a, map.get("a")); - assertEquals(b, map.get("b")); - assertEquals(c, map.get("c")); - - assertTrue(set.stream().anyMatch(digest -> digest.equals(a))); - assertTrue(set.stream().anyMatch(digest -> digest.equals(b))); - assertTrue(set.stream().anyMatch(digest -> digest.equals(c))); - assertTrue(set.stream().anyMatch(digest -> digest.equals(empty1))); - assertTrue(set.stream().anyMatch(digest -> digest.equals(empty2))); + try ( + TDigestState empty1 = new EmptyTDigestState(); + TDigestState empty2 = new EmptyTDigestState(); + TDigestState a = TDigestState.create(arrays(), 200); + TDigestState b = TDigestState.create(arrays(), 100); + TDigestState c = TDigestState.create(arrays(), 100); + ) { + + a.add(randomDouble()); + b.add(randomDouble()); + c.add(randomDouble()); + expectThrows(UnsupportedOperationException.class, () -> empty1.add(randomDouble())); + expectThrows(UnsupportedOperationException.class, () -> empty2.add(a)); + + map.put("empty1", empty1); + map.put("empty2", empty2); + map.put("a", a); + map.put("b", b); + map.put("c", c); + set.add(empty1); + set.add(empty2); + set.add(a); + set.add(b); + set.add(c); + + assertEquals(5, map.size()); + assertEquals(4, set.size()); + + assertEquals(empty1, map.get("empty1")); + assertEquals(empty2, map.get("empty2")); + assertEquals(a, map.get("a")); + assertEquals(b, map.get("b")); + assertEquals(c, map.get("c")); + + assertTrue(set.stream().anyMatch(digest -> digest.equals(a))); + assertTrue(set.stream().anyMatch(digest -> digest.equals(b))); + assertTrue(set.stream().anyMatch(digest -> digest.equals(c))); + assertTrue(set.stream().anyMatch(digest -> digest.equals(empty1))); + assertTrue(set.stream().anyMatch(digest -> digest.equals(empty2))); + } } public void testFactoryMethods() { - TDigestState fast = TDigestState.create(arrays(), 100); - TDigestState anotherFast = TDigestState.create(arrays(), 100); - TDigestState accurate = TDigestState.createOptimizedForAccuracy(arrays(), 100); - TDigestState anotherAccurate = TDigestState.createUsingParamsFrom(accurate); - - for (int i = 0; i < 100; i++) { - fast.add(i); - anotherFast.add(i); - accurate.add(i); - anotherAccurate.add(i); - } + try ( + TDigestState fast = TDigestState.create(arrays(), 100); + TDigestState anotherFast = TDigestState.create(arrays(), 100); + TDigestState accurate = TDigestState.createOptimizedForAccuracy(arrays(), 100); + TDigestState anotherAccurate = TDigestState.createUsingParamsFrom(accurate); + ) { - for (double p : new double[] { 0.1, 1, 10, 25, 50, 75, 90, 99, 99.9 }) { - double q = p / 100; - assertEquals(fast.quantile(q), accurate.quantile(q), 0.5); - assertEquals(fast.quantile(q), anotherFast.quantile(q), 1e-5); - assertEquals(accurate.quantile(q), anotherAccurate.quantile(q), 1e-5); + for (int i = 0; i < 100; i++) { + fast.add(i); + anotherFast.add(i); + accurate.add(i); + anotherAccurate.add(i); + } + + for (double p : new double[] { 0.1, 1, 10, 25, 50, 75, 90, 99, 99.9 }) { + double q = p / 100; + assertEquals(fast.quantile(q), accurate.quantile(q), 0.5); + assertEquals(fast.quantile(q), anotherFast.quantile(q), 1e-5); + assertEquals(accurate.quantile(q), anotherAccurate.quantile(q), 1e-5); + } + + assertEquals(fast, anotherFast); + assertEquals(accurate, anotherAccurate); + assertNotEquals(fast, accurate); + assertNotEquals(anotherFast, anotherAccurate); } - - assertEquals(fast, anotherFast); - assertEquals(accurate, anotherAccurate); - assertNotEquals(fast, accurate); - assertNotEquals(anotherFast, anotherAccurate); } - private static TDigestState writeToAndReadFrom(TDigestState state, TransportVersion version) throws IOException { + private TDigestState writeToAndReadFrom(TDigestState state, TransportVersion version) throws IOException { BytesRef serializedAggs = serialize(state, version); try ( StreamInput in = new NamedWriteableAwareStreamInput( @@ -203,9 +214,11 @@ public void testSerialization() throws IOException { TDigestState serializedBackwardsCompatible = writeToAndReadFrom(state, TransportVersions.V_8_8_1); assertNotEquals(serializedBackwardsCompatible, state); assertEquals(serializedBackwardsCompatible, backwardsCompatible); + + Releasables.close(state, backwardsCompatible, serialized, serializedBackwardsCompatible); } - private static TDigestArrays arrays() { - return WrapperTDigestArrays.INSTANCE; + private TDigestArrays arrays() { + return new MemoryTrackingTDigestArrays(newLimitedBreaker(ByteSizeValue.ofMb(100))); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 00cfedb257187..e6fc32a8ebe1b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -57,6 +57,7 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.CompositeBytesReference; @@ -577,6 +578,21 @@ public final void before() { } } + private final List breakers = Collections.synchronizedList(new ArrayList<>()); + + protected final CircuitBreaker newLimitedBreaker(ByteSizeValue max) { + CircuitBreaker breaker = new MockBigArrays.LimitedBreaker("", max); + breakers.add(breaker); + return breaker; + } + + @After + public final void allBreakersMemoryReleased() { + for (CircuitBreaker breaker : breakers) { + assertThat(breaker.getUsed(), equalTo(0L)); + } + } + /** * Whether or not we check after each test whether it has left warnings behind. That happens if any deprecated feature or syntax * was used by the test and the test didn't assert on it using {@link #assertWarnings(String...)}. diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index ff66d59a21c5b..7a04384298933 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -1536,7 +1536,9 @@ private void randomlyResetClients() { // only reset the clients on nightly tests, it causes heavy load... if (RandomizedTest.isNightly() && rarely(random)) { final Collection nodesAndClients = nodes.values(); + logger.info("Resetting [{}] node clients on internal test cluster", nodesAndClients.size()); for (NodeAndClient nodeAndClient : nodesAndClients) { + logger.info("Resetting [{}] node client on internal test cluster", nodeAndClient.name); nodeAndClient.resetClient(); } } diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java index cb98f9de31ff5..7df791bf11559 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java @@ -18,7 +18,8 @@ public enum FeatureFlag { TIME_SERIES_MODE("es.index_mode_feature_flag_registered=true", Version.fromString("8.0.0"), null), FAILURE_STORE_ENABLED("es.failure_store_feature_flag_enabled=true", Version.fromString("8.12.0"), null), - CHUNKING_SETTINGS_ENABLED("es.inference_chunking_settings_feature_flag_enabled=true", Version.fromString("8.16.0"), null); + CHUNKING_SETTINGS_ENABLED("es.inference_chunking_settings_feature_flag_enabled=true", Version.fromString("8.16.0"), null), + INFERENCE_SCALE_TO_ZERO("es.inference_scale_to_zero_feature_flag_enabled=true", Version.fromString("8.16.0"), null); public final String systemProperty; public final Version from; diff --git a/x-pack/plugin/core/src/main/java/module-info.java b/x-pack/plugin/core/src/main/java/module-info.java index 72436bb9d5171..47848310fe781 100644 --- a/x-pack/plugin/core/src/main/java/module-info.java +++ b/x-pack/plugin/core/src/main/java/module-info.java @@ -228,6 +228,7 @@ exports org.elasticsearch.xpack.core.watcher.trigger; exports org.elasticsearch.xpack.core.watcher.watch; exports org.elasticsearch.xpack.core.watcher; + exports org.elasticsearch.xpack.cluster.settings; provides org.elasticsearch.action.admin.cluster.node.info.ComponentVersionNumber with diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/settings/ClusterSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/settings/ClusterSettings.java new file mode 100644 index 0000000000000..1127889783f16 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/settings/ClusterSettings.java @@ -0,0 +1,19 @@ +/* + * 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.cluster.settings; + +import org.elasticsearch.common.settings.Setting; + +public class ClusterSettings { + public static final Setting CLUSTER_LOGSDB_ENABLED = Setting.boolSetting( + "cluster.logsdb.enabled", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateTrainedModelDeploymentAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateTrainedModelDeploymentAction.java index 7fca223b2ee7e..cb578fdb157de 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateTrainedModelDeploymentAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateTrainedModelDeploymentAction.java @@ -161,7 +161,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public ActionRequestValidationException validate() { ActionRequestValidationException validationException = new ActionRequestValidationException(); if (numberOfAllocations != null) { - if (numberOfAllocations < 1) { + if (numberOfAllocations < 0 || (isInternal == false && numberOfAllocations == 0)) { validationException.addValidationError("[" + NUMBER_OF_ALLOCATIONS + "] must be a positive integer"); } if (isInternal == false diff --git a/x-pack/plugin/core/template-resources/src/main/resources/logs@settings-logsdb.json b/x-pack/plugin/core/template-resources/src/main/resources/logs@settings-logsdb.json new file mode 100644 index 0000000000000..eabdd6fb9fad2 --- /dev/null +++ b/x-pack/plugin/core/template-resources/src/main/resources/logs@settings-logsdb.json @@ -0,0 +1,26 @@ +{ + "template": { + "settings": { + "index": { + "lifecycle": { + "name": "logs" + }, + "mode": "logsdb", + "codec": "best_compression", + "mapping": { + "ignore_malformed": true, + "total_fields": { + "ignore_dynamic_beyond_limit": true + } + }, + "default_pipeline": "logs@default-pipeline" + } + } + }, + "_meta": { + "description": "default settings for the logs index template installed by x-pack", + "managed": true + }, + "version": ${xpack.stack.template.version}, + "deprecated": ${xpack.stack.template.deprecated} +} diff --git a/x-pack/plugin/core/template-resources/src/main/resources/logs@settings.json b/x-pack/plugin/core/template-resources/src/main/resources/logs@settings.json index e9a9f2611ad7b..ca2659b8d8dea 100644 --- a/x-pack/plugin/core/template-resources/src/main/resources/logs@settings.json +++ b/x-pack/plugin/core/template-resources/src/main/resources/logs@settings.json @@ -5,7 +5,6 @@ "lifecycle": { "name": "logs" }, - "mode": "${xpack.stack.template.logsdb.index.mode}", "codec": "best_compression", "mapping": { "ignore_malformed": true, diff --git a/x-pack/plugin/inference/qa/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/inference/qa/mixed/HuggingFaceServiceMixedIT.java b/x-pack/plugin/inference/qa/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/inference/qa/mixed/HuggingFaceServiceMixedIT.java index a2793f9060d8a..59d3faf6489a6 100644 --- a/x-pack/plugin/inference/qa/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/inference/qa/mixed/HuggingFaceServiceMixedIT.java +++ b/x-pack/plugin/inference/qa/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/inference/qa/mixed/HuggingFaceServiceMixedIT.java @@ -84,6 +84,7 @@ public void testElser() throws IOException { final String inferenceId = "mixed-cluster-elser"; final String upgradedClusterId = "upgraded-cluster-elser"; + elserServer.enqueue(new MockResponse().setResponseCode(200).setBody(elserResponse())); put(inferenceId, elserConfig(getUrl(elserServer)), TaskType.SPARSE_EMBEDDING); var configs = (List>) get(TaskType.SPARSE_EMBEDDING, inferenceId).get("endpoints"); diff --git a/x-pack/plugin/inference/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/application/HuggingFaceServiceUpgradeIT.java b/x-pack/plugin/inference/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/application/HuggingFaceServiceUpgradeIT.java index 36ee472cc0a13..9c9a377bbb001 100644 --- a/x-pack/plugin/inference/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/application/HuggingFaceServiceUpgradeIT.java +++ b/x-pack/plugin/inference/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/application/HuggingFaceServiceUpgradeIT.java @@ -117,6 +117,7 @@ public void testElser() throws IOException { var testTaskType = TaskType.SPARSE_EMBEDDING; if (isOldCluster()) { + elserServer.enqueue(new MockResponse().setResponseCode(200).setBody(elserResponse())); put(oldClusterId, elserConfig(getUrl(elserServer)), testTaskType); var configs = (List>) get(testTaskType, oldClusterId).get(old_cluster_endpoint_identifier); assertThat(configs, hasSize(1)); @@ -136,6 +137,7 @@ public void testElser() throws IOException { assertElser(oldClusterId); // New endpoint + elserServer.enqueue(new MockResponse().setResponseCode(200).setBody(elserResponse())); put(upgradedClusterId, elserConfig(getUrl(elserServer)), testTaskType); configs = (List>) get(upgradedClusterId).get("endpoints"); assertThat(configs, hasSize(1)); diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/ServiceUtils.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/ServiceUtils.java index 6c4904f8918a7..6eb0331913009 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/ServiceUtils.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/ServiceUtils.java @@ -202,6 +202,13 @@ public static ElasticsearchStatusException unknownSettingsError(Map invalidModelType) { + throw new ElasticsearchStatusException( + Strings.format("Can't update embedding details for model with unexpected type %s", invalidModelType), + RestStatus.BAD_REQUEST + ); + } + public static String missingSettingErrorMsg(String settingName, String scope) { return Strings.format("[%s] does not contain the required setting [%s]", scope, settingName); } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioService.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioService.java index 08eb67ca744a4..422fc5b0ed720 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioService.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioService.java @@ -35,6 +35,7 @@ import org.elasticsearch.xpack.inference.services.googleaistudio.completion.GoogleAiStudioCompletionModel; import org.elasticsearch.xpack.inference.services.googleaistudio.embeddings.GoogleAiStudioEmbeddingsModel; import org.elasticsearch.xpack.inference.services.googleaistudio.embeddings.GoogleAiStudioEmbeddingsServiceSettings; +import org.elasticsearch.xpack.inference.services.validation.ModelValidatorBuilder; import java.util.List; import java.util.Map; @@ -187,30 +188,29 @@ public TransportVersion getMinimalSupportedVersion() { @Override public void checkModelConfig(Model model, ActionListener listener) { - if (model instanceof GoogleAiStudioEmbeddingsModel embeddingsModel) { - ServiceUtils.getEmbeddingSize( - model, - this, - listener.delegateFailureAndWrap((l, size) -> l.onResponse(updateModelWithEmbeddingDetails(embeddingsModel, size))) - ); - } else { - listener.onResponse(model); - } + // TODO: Remove this function once all services have been updated to use the new model validators + ModelValidatorBuilder.buildModelValidator(model.getTaskType()).validate(this, model, listener); } - private GoogleAiStudioEmbeddingsModel updateModelWithEmbeddingDetails(GoogleAiStudioEmbeddingsModel model, int embeddingSize) { - var similarityFromModel = model.getServiceSettings().similarity(); - var similarityToUse = similarityFromModel == null ? SimilarityMeasure.DOT_PRODUCT : similarityFromModel; + @Override + public Model updateModelWithEmbeddingDetails(Model model, int embeddingSize) { + if (model instanceof GoogleAiStudioEmbeddingsModel embeddingsModel) { + var serviceSettings = embeddingsModel.getServiceSettings(); + var similarityFromModel = serviceSettings.similarity(); + var similarityToUse = similarityFromModel == null ? SimilarityMeasure.DOT_PRODUCT : similarityFromModel; - GoogleAiStudioEmbeddingsServiceSettings serviceSettings = new GoogleAiStudioEmbeddingsServiceSettings( - model.getServiceSettings().modelId(), - model.getServiceSettings().maxInputTokens(), - embeddingSize, - similarityToUse, - model.getServiceSettings().rateLimitSettings() - ); + var updatedServiceSettings = new GoogleAiStudioEmbeddingsServiceSettings( + serviceSettings.modelId(), + serviceSettings.maxInputTokens(), + embeddingSize, + similarityToUse, + serviceSettings.rateLimitSettings() + ); - return new GoogleAiStudioEmbeddingsModel(model, serviceSettings); + return new GoogleAiStudioEmbeddingsModel(embeddingsModel, updatedServiceSettings); + } else { + throw ServiceUtils.invalidModelTypeForUpdateModelWithEmbeddingDetails(model.getClass()); + } } @Override diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceService.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceService.java index bdfa87e77b708..6b142edca80aa 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceService.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceService.java @@ -29,6 +29,7 @@ import org.elasticsearch.xpack.inference.services.ServiceUtils; import org.elasticsearch.xpack.inference.services.huggingface.elser.HuggingFaceElserModel; import org.elasticsearch.xpack.inference.services.huggingface.embeddings.HuggingFaceEmbeddingsModel; +import org.elasticsearch.xpack.inference.services.validation.ModelValidatorBuilder; import java.util.List; import java.util.Map; @@ -67,34 +68,31 @@ protected HuggingFaceModel createModel( @Override public void checkModelConfig(Model model, ActionListener listener) { + // TODO: Remove this function once all services have been updated to use the new model validators + ModelValidatorBuilder.buildModelValidator(model.getTaskType()).validate(this, model, listener); + } + + @Override + public Model updateModelWithEmbeddingDetails(Model model, int embeddingSize) { if (model instanceof HuggingFaceEmbeddingsModel embeddingsModel) { - ServiceUtils.getEmbeddingSize( - model, - this, - listener.delegateFailureAndWrap((l, size) -> l.onResponse(updateModelWithEmbeddingDetails(embeddingsModel, size))) + var serviceSettings = embeddingsModel.getServiceSettings(); + var similarityFromModel = serviceSettings.similarity(); + var similarityToUse = similarityFromModel == null ? SimilarityMeasure.COSINE : similarityFromModel; + + var updatedServiceSettings = new HuggingFaceServiceSettings( + serviceSettings.uri(), + similarityToUse, + embeddingSize, + embeddingsModel.getTokenLimit(), + serviceSettings.rateLimitSettings() ); + + return new HuggingFaceEmbeddingsModel(embeddingsModel, updatedServiceSettings); } else { - listener.onResponse(model); + throw ServiceUtils.invalidModelTypeForUpdateModelWithEmbeddingDetails(model.getClass()); } } - private static HuggingFaceEmbeddingsModel updateModelWithEmbeddingDetails(HuggingFaceEmbeddingsModel model, int embeddingSize) { - // default to cosine similarity - var similarity = model.getServiceSettings().similarity() == null - ? SimilarityMeasure.COSINE - : model.getServiceSettings().similarity(); - - var serviceSettings = new HuggingFaceServiceSettings( - model.getServiceSettings().uri(), - similarity, - embeddingSize, - model.getTokenLimit(), - model.getServiceSettings().rateLimitSettings() - ); - - return new HuggingFaceEmbeddingsModel(model, serviceSettings); - } - @Override protected void doChunkedInfer( Model model, diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/mistral/MistralService.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/mistral/MistralService.java index 1acc13f50778b..221951f7a621e 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/mistral/MistralService.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/mistral/MistralService.java @@ -33,6 +33,7 @@ import org.elasticsearch.xpack.inference.services.ServiceUtils; import org.elasticsearch.xpack.inference.services.mistral.embeddings.MistralEmbeddingsModel; import org.elasticsearch.xpack.inference.services.mistral.embeddings.MistralEmbeddingsServiceSettings; +import org.elasticsearch.xpack.inference.services.validation.ModelValidatorBuilder; import java.util.List; import java.util.Map; @@ -214,32 +215,28 @@ private MistralEmbeddingsModel createModelFromPersistent( @Override public void checkModelConfig(Model model, ActionListener listener) { - if (model instanceof MistralEmbeddingsModel embeddingsModel) { - ServiceUtils.getEmbeddingSize( - model, - this, - listener.delegateFailureAndWrap((l, size) -> l.onResponse(updateEmbeddingModelConfig(embeddingsModel, size))) - ); - } else { - listener.onResponse(model); - } + // TODO: Remove this function once all services have been updated to use the new model validators + ModelValidatorBuilder.buildModelValidator(model.getTaskType()).validate(this, model, listener); } - private MistralEmbeddingsModel updateEmbeddingModelConfig(MistralEmbeddingsModel embeddingsModel, int embeddingsSize) { - var embeddingServiceSettings = embeddingsModel.getServiceSettings(); - - var similarityFromModel = embeddingsModel.getServiceSettings().similarity(); - var similarityToUse = similarityFromModel == null ? SimilarityMeasure.DOT_PRODUCT : similarityFromModel; + @Override + public Model updateModelWithEmbeddingDetails(Model model, int embeddingSize) { + if (model instanceof MistralEmbeddingsModel embeddingsModel) { + var serviceSettings = embeddingsModel.getServiceSettings(); - MistralEmbeddingsServiceSettings serviceSettings = new MistralEmbeddingsServiceSettings( - embeddingServiceSettings.modelId(), - embeddingsSize, - embeddingServiceSettings.maxInputTokens(), - similarityToUse, - embeddingServiceSettings.rateLimitSettings() - ); + var similarityFromModel = embeddingsModel.getServiceSettings().similarity(); + var similarityToUse = similarityFromModel == null ? SimilarityMeasure.DOT_PRODUCT : similarityFromModel; - return new MistralEmbeddingsModel(embeddingsModel, serviceSettings); + MistralEmbeddingsServiceSettings updatedServiceSettings = new MistralEmbeddingsServiceSettings( + serviceSettings.modelId(), + embeddingSize, + serviceSettings.maxInputTokens(), + similarityToUse, + serviceSettings.rateLimitSettings() + ); + return new MistralEmbeddingsModel(embeddingsModel, updatedServiceSettings); + } else { + throw ServiceUtils.invalidModelTypeForUpdateModelWithEmbeddingDetails(model.getClass()); + } } - } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/openai/OpenAiService.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/openai/OpenAiService.java index 7cea1ec7df46c..f9565a915124f 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/openai/OpenAiService.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/openai/OpenAiService.java @@ -12,7 +12,6 @@ import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionListener; import org.elasticsearch.core.Nullable; -import org.elasticsearch.core.Strings; import org.elasticsearch.core.TimeValue; import org.elasticsearch.inference.ChunkedInferenceServiceResults; import org.elasticsearch.inference.ChunkingOptions; @@ -35,6 +34,7 @@ import org.elasticsearch.xpack.inference.services.ConfigurationParseContext; import org.elasticsearch.xpack.inference.services.SenderService; import org.elasticsearch.xpack.inference.services.ServiceComponents; +import org.elasticsearch.xpack.inference.services.ServiceUtils; import org.elasticsearch.xpack.inference.services.openai.completion.OpenAiChatCompletionModel; import org.elasticsearch.xpack.inference.services.openai.embeddings.OpenAiEmbeddingsModel; import org.elasticsearch.xpack.inference.services.openai.embeddings.OpenAiEmbeddingsServiceSettings; @@ -307,10 +307,7 @@ public Model updateModelWithEmbeddingDetails(Model model, int embeddingSize) { return new OpenAiEmbeddingsModel(embeddingsModel, updatedServiceSettings); } else { - throw new ElasticsearchStatusException( - Strings.format("Can't update embedding details for model with unexpected type %s", model.getClass()), - RestStatus.BAD_REQUEST - ); + throw ServiceUtils.invalidModelTypeForUpdateModelWithEmbeddingDetails(model.getClass()); } } diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidator.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidator.java index 9fc5748746085..6233a7e0b6b29 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidator.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidator.java @@ -1,3 +1,4 @@ + /* * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one * or more contributor license agreements. Licensed under the Elastic License @@ -33,14 +34,25 @@ public void validate(InferenceService service, Model model, ActionListener { + ActionListener.wrap(r -> { if (r != null) { - delegate.onResponse(r); + listener.onResponse(r); } else { - delegate.onFailure( - new ElasticsearchStatusException("Could not make a validation call to the selected service", RestStatus.BAD_REQUEST) + listener.onFailure( + new ElasticsearchStatusException( + "Could not complete inference endpoint creation as validation call to service returned null response.", + RestStatus.BAD_REQUEST + ) ); } + }, e -> { + listener.onFailure( + new ElasticsearchStatusException( + "Could not complete inference endpoint creation as validation call to service threw an exception.", + RestStatus.BAD_REQUEST, + e + ) + ); }) ); } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioServiceTests.java index a8882bb244512..89d6a010bbc07 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/googleaistudio/GoogleAiStudioServiceTests.java @@ -914,6 +914,45 @@ public void testCheckModelConfig_DoesNotUpdateSimilarity_WhenItIsSpecifiedAsCosi } } + public void testUpdateModelWithEmbeddingDetails_InvalidModelProvided() throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new GoogleAiStudioService(senderFactory, createWithEmptySettings(threadPool))) { + var model = GoogleAiStudioCompletionModelTests.createModel(randomAlphaOfLength(10), randomAlphaOfLength(10)); + assertThrows( + ElasticsearchStatusException.class, + () -> { service.updateModelWithEmbeddingDetails(model, randomNonNegativeInt()); } + ); + } + } + + public void testUpdateModelWithEmbeddingDetails_NullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(null); + } + + public void testUpdateModelWithEmbeddingDetails_NonNullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(randomFrom(SimilarityMeasure.values())); + } + + private void testUpdateModelWithEmbeddingDetails_Successful(SimilarityMeasure similarityMeasure) throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new GoogleAiStudioService(senderFactory, createWithEmptySettings(threadPool))) { + var embeddingSize = randomNonNegativeInt(); + var model = GoogleAiStudioEmbeddingsModelTests.createModel( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomNonNegativeInt(), + similarityMeasure + ); + + Model updatedModel = service.updateModelWithEmbeddingDetails(model, embeddingSize); + + SimilarityMeasure expectedSimilarityMeasure = similarityMeasure == null ? SimilarityMeasure.DOT_PRODUCT : similarityMeasure; + assertEquals(expectedSimilarityMeasure, updatedModel.getServiceSettings().similarity()); + assertEquals(embeddingSize, updatedModel.getServiceSettings().dimensions().intValue()); + } + } + public static Map buildExpectationCompletions(List completions) { return Map.of( ChatCompletionResults.COMPLETION, diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceServiceTests.java index f68aedd69f365..5ea9f82e5b60c 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/huggingface/HuggingFaceServiceTests.java @@ -595,6 +595,45 @@ public void testCheckModelConfig_DefaultsSimilarityToCosine() throws IOException } } + public void testUpdateModelWithEmbeddingDetails_InvalidModelProvided() throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new HuggingFaceService(senderFactory, createWithEmptySettings(threadPool))) { + var model = HuggingFaceElserModelTests.createModel(randomAlphaOfLength(10), randomAlphaOfLength(10)); + assertThrows( + ElasticsearchStatusException.class, + () -> { service.updateModelWithEmbeddingDetails(model, randomNonNegativeInt()); } + ); + } + } + + public void testUpdateModelWithEmbeddingDetails_NullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(null); + } + + public void testUpdateModelWithEmbeddingDetails_NonNullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(randomFrom(SimilarityMeasure.values())); + } + + private void testUpdateModelWithEmbeddingDetails_Successful(SimilarityMeasure similarityMeasure) throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new HuggingFaceService(senderFactory, createWithEmptySettings(threadPool))) { + var embeddingSize = randomNonNegativeInt(); + var model = HuggingFaceEmbeddingsModelTests.createModel( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomNonNegativeInt(), + randomNonNegativeInt(), + similarityMeasure + ); + + Model updatedModel = service.updateModelWithEmbeddingDetails(model, embeddingSize); + + SimilarityMeasure expectedSimilarityMeasure = similarityMeasure == null ? SimilarityMeasure.COSINE : similarityMeasure; + assertEquals(expectedSimilarityMeasure, updatedModel.getServiceSettings().similarity()); + assertEquals(embeddingSize, updatedModel.getServiceSettings().dimensions().intValue()); + } + } + public void testChunkedInfer_CallsInfer_TextEmbedding_ConvertsFloatResponse() throws IOException { var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/mistral/MistralServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/mistral/MistralServiceTests.java index c833f00c4c433..9d0fd954c44f9 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/mistral/MistralServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/mistral/MistralServiceTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.inference.action.InferenceAction; import org.elasticsearch.xpack.core.inference.results.InferenceChunkedTextEmbeddingFloatResults; +import org.elasticsearch.xpack.inference.ModelConfigurationsTests; import org.elasticsearch.xpack.inference.external.http.HttpClientManager; import org.elasticsearch.xpack.inference.external.http.sender.HttpRequestSender; import org.elasticsearch.xpack.inference.external.http.sender.HttpRequestSenderTests; @@ -38,6 +39,7 @@ import org.elasticsearch.xpack.inference.services.mistral.embeddings.MistralEmbeddingModelTests; import org.elasticsearch.xpack.inference.services.mistral.embeddings.MistralEmbeddingsModel; import org.elasticsearch.xpack.inference.services.mistral.embeddings.MistralEmbeddingsServiceSettings; +import org.elasticsearch.xpack.inference.services.settings.RateLimitSettingsTests; import org.hamcrest.CoreMatchers; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; @@ -388,6 +390,48 @@ public void testCheckModelConfig_ForEmbeddingsModel_Works() throws IOException { } } + public void testUpdateModelWithEmbeddingDetails_InvalidModelProvided() throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new MistralService(senderFactory, createWithEmptySettings(threadPool))) { + var model = new Model(ModelConfigurationsTests.createRandomInstance()); + + assertThrows( + ElasticsearchStatusException.class, + () -> { service.updateModelWithEmbeddingDetails(model, randomNonNegativeInt()); } + ); + } + } + + public void testUpdateModelWithEmbeddingDetails_NullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(null); + } + + public void testUpdateModelWithEmbeddingDetails_NonNullSimilarityInOriginalModel() throws IOException { + testUpdateModelWithEmbeddingDetails_Successful(randomFrom(SimilarityMeasure.values())); + } + + private void testUpdateModelWithEmbeddingDetails_Successful(SimilarityMeasure similarityMeasure) throws IOException { + var senderFactory = HttpRequestSenderTests.createSenderFactory(threadPool, clientManager); + try (var service = new MistralService(senderFactory, createWithEmptySettings(threadPool))) { + var embeddingSize = randomNonNegativeInt(); + var model = MistralEmbeddingModelTests.createModel( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomNonNegativeInt(), + randomNonNegativeInt(), + similarityMeasure, + RateLimitSettingsTests.createRandom() + ); + + Model updatedModel = service.updateModelWithEmbeddingDetails(model, embeddingSize); + + SimilarityMeasure expectedSimilarityMeasure = similarityMeasure == null ? SimilarityMeasure.DOT_PRODUCT : similarityMeasure; + assertEquals(expectedSimilarityMeasure, updatedModel.getServiceSettings().similarity()); + assertEquals(embeddingSize, updatedModel.getServiceSettings().dimensions().intValue()); + } + } + public void testInfer_ThrowsErrorWhenModelIsNotMistralEmbeddingsModel() throws IOException { var sender = mock(Sender.class); diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/openai/OpenAiServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/openai/OpenAiServiceTests.java index e4a304f818328..a5e8c1d7eb26e 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/openai/OpenAiServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/openai/OpenAiServiceTests.java @@ -1433,7 +1433,7 @@ private void testUpdateModelWithEmbeddingDetails_Successful(SimilarityMeasure si randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), - null, + similarityMeasure, randomNonNegativeInt(), randomNonNegativeInt(), randomBoolean() @@ -1441,7 +1441,8 @@ private void testUpdateModelWithEmbeddingDetails_Successful(SimilarityMeasure si Model updatedModel = service.updateModelWithEmbeddingDetails(model, embeddingSize); - assertEquals(SimilarityMeasure.DOT_PRODUCT, updatedModel.getServiceSettings().similarity()); + SimilarityMeasure expectedSimilarityMeasure = similarityMeasure == null ? SimilarityMeasure.DOT_PRODUCT : similarityMeasure; + assertEquals(expectedSimilarityMeasure, updatedModel.getServiceSettings().similarity()); assertEquals(embeddingSize, updatedModel.getServiceSettings().dimensions().intValue()); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidatorTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidatorTests.java index 23000ce431e7b..ef295e4070cc3 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidatorTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/validation/SimpleServiceIntegrationValidatorTests.java @@ -122,7 +122,6 @@ private void verifyCallToService(boolean withQuery) { eq(InferenceAction.Request.DEFAULT_TIMEOUT), any() ); - verify(mockActionListener).delegateFailureAndWrap(any()); verifyNoMoreInteractions(mockInferenceService, mockModel, mockActionListener, mockInferenceServiceResults); } } diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java index e38f953be96a3..833555a7884ea 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.List; +import static org.elasticsearch.xpack.cluster.settings.ClusterSettings.CLUSTER_LOGSDB_ENABLED; import static org.elasticsearch.xpack.logsdb.SyntheticSourceLicenseService.FALLBACK_SETTING; public class LogsDBPlugin extends Plugin { @@ -24,9 +25,12 @@ public class LogsDBPlugin extends Plugin { private final Settings settings; private final SyntheticSourceLicenseService licenseService; + private final LogsdbIndexModeSettingsProvider logsdbIndexModeSettingsProvider; + public LogsDBPlugin(Settings settings) { this.settings = settings; this.licenseService = new SyntheticSourceLicenseService(settings); + this.logsdbIndexModeSettingsProvider = new LogsdbIndexModeSettingsProvider(settings); } @Override @@ -34,6 +38,10 @@ public Collection createComponents(PluginServices services) { licenseService.setLicenseState(XPackPlugin.getSharedLicenseState()); var clusterSettings = services.clusterService().getClusterSettings(); clusterSettings.addSettingsUpdateConsumer(FALLBACK_SETTING, licenseService::setSyntheticSourceFallback); + clusterSettings.addSettingsUpdateConsumer( + CLUSTER_LOGSDB_ENABLED, + logsdbIndexModeSettingsProvider::updateClusterIndexModeLogsdbEnabled + ); // Nothing to share here: return super.createComponents(services); } @@ -43,11 +51,11 @@ public Collection getAdditionalIndexSettingProviders(Index if (DiscoveryNode.isStateless(settings)) { return List.of(); } - return List.of(new SyntheticSourceIndexSettingsProvider(licenseService)); + return List.of(new SyntheticSourceIndexSettingsProvider(licenseService), logsdbIndexModeSettingsProvider); } @Override public List> getSettings() { - return List.of(FALLBACK_SETTING); + return List.of(FALLBACK_SETTING, CLUSTER_LOGSDB_ENABLED); } } diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java new file mode 100644 index 0000000000000..3f6bb66dfa438 --- /dev/null +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.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.xpack.logsdb; + +import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.regex.Regex; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettingProvider; +import org.elasticsearch.index.IndexSettings; + +import java.time.Instant; +import java.util.List; +import java.util.Locale; + +import static org.elasticsearch.xpack.cluster.settings.ClusterSettings.CLUSTER_LOGSDB_ENABLED; + +final class LogsdbIndexModeSettingsProvider implements IndexSettingProvider { + private static final String LOGS_PATTERN = "logs-*-*"; + private volatile boolean isLogsdbEnabled; + + LogsdbIndexModeSettingsProvider(final Settings settings) { + this.isLogsdbEnabled = CLUSTER_LOGSDB_ENABLED.get(settings); + } + + void updateClusterIndexModeLogsdbEnabled(boolean isLogsdbEnabled) { + this.isLogsdbEnabled = isLogsdbEnabled; + } + + @Override + public Settings getAdditionalIndexSettings( + final String indexName, + final String dataStreamName, + boolean isTimeSeries, + final Metadata metadata, + final Instant resolvedAt, + final Settings settings, + final List combinedTemplateMappings + ) { + if (isLogsdbEnabled == false || dataStreamName == null) { + return Settings.EMPTY; + } + + final IndexMode indexMode = resolveIndexMode(settings.get(IndexSettings.MODE.getKey())); + if (indexMode != null) { + return Settings.EMPTY; + } + + if (usesLogsAtSettingsComponentTemplate(metadata, dataStreamName) && matchesLogsPattern(dataStreamName)) { + return Settings.builder().put("index.mode", IndexMode.LOGSDB.getName()).build(); + } + + return Settings.EMPTY; + } + + private static boolean matchesLogsPattern(final String name) { + return Regex.simpleMatch(LOGS_PATTERN, name); + } + + private IndexMode resolveIndexMode(final String mode) { + return mode != null ? Enum.valueOf(IndexMode.class, mode.toUpperCase(Locale.ROOT)) : null; + } + + private boolean usesLogsAtSettingsComponentTemplate(final Metadata metadata, final String name) { + final String template = MetadataIndexTemplateService.findV2Template(metadata, name, false); + if (template == null) { + return false; + } + final ComposableIndexTemplate composableIndexTemplate = metadata.templatesV2().get(template); + if (composableIndexTemplate == null) { + return false; + } + for (final String componentTemplate : composableIndexTemplate.composedOf()) { + if ("logs@settings".equals(componentTemplate)) { + return true; + } + } + return false; + } + +} diff --git a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProviderTests.java b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProviderTests.java new file mode 100644 index 0000000000000..eeb5389644c02 --- /dev/null +++ b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProviderTests.java @@ -0,0 +1,326 @@ +/* + * 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.logsdb; + +import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; +import org.elasticsearch.cluster.metadata.ComposableIndexTemplateMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.Template; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.Map; + +public class LogsdbIndexModeSettingsProviderTests extends ESTestCase { + + public static final String DEFAULT_MAPPING = """ + { + "_doc": { + "properties": { + "@timestamp": { + "type": "date" + }, + "message": { + "type": "keyword" + }, + "host.name": { + "type": "keyword" + } + } + } + } + """; + + public void testLogsDbDisabled() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", false).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testOnIndexCreation() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + "logs-apache-production", + null, + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testOnExplicitStandardIndex() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.builder().put(IndexSettings.MODE.getKey(), IndexMode.STANDARD.getName()).build(), + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testOnExplicitTimeSeriesIndex() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.builder().put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES.getName()).build(), + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testNonLogsDataStream() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testWithoutLogsComponentTemplate() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of()), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testWithLogsComponentTemplate() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@settings")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertIndexMode(additionalIndexSettings, IndexMode.LOGSDB.getName()); + } + + public void testWithMultipleComponentTemplates() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@settings", "logs@custom")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertIndexMode(additionalIndexSettings, IndexMode.LOGSDB.getName()); + } + + public void testWithCustomComponentTemplatesOnly() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@custom", "custom-component-template")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testNonMatchingTemplateIndexPattern() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("standard-apache-production"), List.of("logs@settings")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testCaseSensitivity() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "LOGS-apache-production", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testMultipleHyphensInDataStreamName() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", true).build() + ); + + final Settings additionalIndexSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production-eu", + false, + Metadata.EMPTY_METADATA, + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(additionalIndexSettings.isEmpty()); + } + + public void testBeforeAndAFterSettingUpdate() throws IOException { + final LogsdbIndexModeSettingsProvider provider = new LogsdbIndexModeSettingsProvider( + Settings.builder().put("cluster.logsdb.enabled", false).build() + ); + + final Settings beforeSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@settings")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(beforeSettings.isEmpty()); + + provider.updateClusterIndexModeLogsdbEnabled(true); + + final Settings afterSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@settings")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertIndexMode(afterSettings, IndexMode.LOGSDB.getName()); + + provider.updateClusterIndexModeLogsdbEnabled(false); + + final Settings laterSettings = provider.getAdditionalIndexSettings( + null, + "logs-apache-production", + false, + buildMetadata(List.of("*"), List.of("logs@settings")), + Instant.now().truncatedTo(ChronoUnit.SECONDS), + Settings.EMPTY, + List.of(new CompressedXContent(DEFAULT_MAPPING)) + ); + + assertTrue(laterSettings.isEmpty()); + } + + private static Metadata buildMetadata(final List indexPatterns, final List componentTemplates) throws IOException { + final Template template = new Template(Settings.EMPTY, new CompressedXContent(DEFAULT_MAPPING), null); + final ComposableIndexTemplate composableTemplate = ComposableIndexTemplate.builder() + .indexPatterns(indexPatterns) + .template(template) + .componentTemplates(componentTemplates) + .priority(1_000L) + .version(1L) + .build(); + return Metadata.builder() + .putCustom(ComposableIndexTemplateMetadata.TYPE, new ComposableIndexTemplateMetadata(Map.of("composable", composableTemplate))) + .build(); + } + + private void assertIndexMode(final Settings settings, final String expectedIndexMode) { + assertEquals(expectedIndexMode, settings.get(IndexSettings.MODE.getKey())); + } + +} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 1bc867a849090..f8a590a23a2c1 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -327,6 +327,7 @@ import org.elasticsearch.xpack.ml.dataframe.process.results.AnalyticsResult; import org.elasticsearch.xpack.ml.dataframe.process.results.MemoryUsageEstimationResult; import org.elasticsearch.xpack.ml.inference.TrainedModelStatsService; +import org.elasticsearch.xpack.ml.inference.adaptiveallocations.AdaptiveAllocationsScalerService; import org.elasticsearch.xpack.ml.inference.assignment.TrainedModelAssignmentClusterService; import org.elasticsearch.xpack.ml.inference.assignment.TrainedModelAssignmentService; import org.elasticsearch.xpack.ml.inference.deployment.DeploymentManager; @@ -1285,13 +1286,21 @@ public Collection createComponents(PluginServices services) { new MlAutoscalingDeciderService(memoryTracker, settings, nodeAvailabilityZoneMapper, clusterService) ); - MlInitializationService mlInitializationService = new MlInitializationService( - settings, + AdaptiveAllocationsScalerService adaptiveAllocationsScalerService = new AdaptiveAllocationsScalerService( threadPool, clusterService, client, inferenceAuditor, telemetryProvider.getMeterRegistry(), + machineLearningExtension.get().isNlpEnabled() + ); + + MlInitializationService mlInitializationService = new MlInitializationService( + settings, + threadPool, + clusterService, + client, + adaptiveAllocationsScalerService, mlAssignmentNotifier, machineLearningExtension.get().isAnomalyDetectionEnabled(), machineLearningExtension.get().isDataFrameAnalyticsEnabled(), @@ -1317,6 +1326,7 @@ public Collection createComponents(PluginServices services) { jobManagerHolder, autodetectProcessManager, mlInitializationService, + adaptiveAllocationsScalerService, jobDataCountsPersister, datafeedRunner, datafeedManager, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java index 98dfb13d9e3e4..45a71a80de077 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java @@ -30,11 +30,9 @@ import org.elasticsearch.common.component.LifecycleListener; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.gateway.GatewayService; -import org.elasticsearch.telemetry.metric.MeterRegistry; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ml.annotations.AnnotationIndex; import org.elasticsearch.xpack.ml.inference.adaptiveallocations.AdaptiveAllocationsScalerService; -import org.elasticsearch.xpack.ml.notifications.InferenceAuditor; import java.util.Collections; import java.util.Map; @@ -67,8 +65,7 @@ public final class MlInitializationService implements ClusterStateListener { ThreadPool threadPool, ClusterService clusterService, Client client, - InferenceAuditor inferenceAuditor, - MeterRegistry meterRegistry, + AdaptiveAllocationsScalerService adaptiveAllocationsScalerService, MlAssignmentNotifier mlAssignmentNotifier, boolean isAnomalyDetectionEnabled, boolean isDataFrameAnalyticsEnabled, @@ -88,7 +85,7 @@ public final class MlInitializationService implements ClusterStateListener { isDataFrameAnalyticsEnabled, isNlpEnabled ), - new AdaptiveAllocationsScalerService(threadPool, clusterService, client, inferenceAuditor, meterRegistry, isNlpEnabled), + adaptiveAllocationsScalerService, clusterService ); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportExternalInferModelAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportExternalInferModelAction.java index 545dcfbefecec..5603e9c4dca8d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportExternalInferModelAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportExternalInferModelAction.java @@ -13,6 +13,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.ml.action.InferModelAction; +import org.elasticsearch.xpack.ml.inference.adaptiveallocations.AdaptiveAllocationsScalerService; import org.elasticsearch.xpack.ml.inference.loadingservice.ModelLoadingService; import org.elasticsearch.xpack.ml.inference.persistence.TrainedModelProvider; @@ -25,7 +26,8 @@ public TransportExternalInferModelAction( Client client, ClusterService clusterService, XPackLicenseState licenseState, - TrainedModelProvider trainedModelProvider + TrainedModelProvider trainedModelProvider, + AdaptiveAllocationsScalerService adaptiveAllocationsScalerService ) { super( InferModelAction.EXTERNAL_NAME, @@ -35,7 +37,8 @@ public TransportExternalInferModelAction( client, clusterService, licenseState, - trainedModelProvider + trainedModelProvider, + adaptiveAllocationsScalerService ); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportInternalInferModelAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportInternalInferModelAction.java index 0c4064348b3f6..b69f8c7d62eb2 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportInternalInferModelAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportInternalInferModelAction.java @@ -42,6 +42,7 @@ import org.elasticsearch.xpack.core.ml.inference.results.ErrorInferenceResults; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.ml.MachineLearning; +import org.elasticsearch.xpack.ml.inference.adaptiveallocations.AdaptiveAllocationsScalerService; import org.elasticsearch.xpack.ml.inference.loadingservice.LocalModel; import org.elasticsearch.xpack.ml.inference.loadingservice.ModelLoadingService; import org.elasticsearch.xpack.ml.inference.persistence.TrainedModelProvider; @@ -66,6 +67,7 @@ public class TransportInternalInferModelAction extends HandledTransportAction format("[%s] model deployment not allocated to any node", assignment.getDeploymentId())); - listener.onFailure( - ExceptionsHelper.conflictStatusException("Trained model deployment [" + request.getId() + "] is not allocated to any nodes") - ); + String message = "Trained model deployment [" + request.getId() + "] is not allocated to any nodes"; + boolean starting = adaptiveAllocationsScalerService.maybeStartAllocation(assignment); + if (starting) { + message += "; starting deployment of one allocation"; + } + logger.debug(message); + listener.onFailure(ExceptionsHelper.conflictStatusException(message)); return; } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScaler.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScaler.java index 044556d1b30ac..05e7202b8efe9 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScaler.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScaler.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Strings; +import org.elasticsearch.core.TimeValue; /** * Processes measured requests counts and inference times and decides whether @@ -21,6 +22,12 @@ public class AdaptiveAllocationsScaler { static final double SCALE_UP_THRESHOLD = 0.9; private static final double SCALE_DOWN_THRESHOLD = 0.85; + /** + * The time interval without any requests that has to pass, before scaling down + * to zero allocations (in case min_allocations = 0). + */ + private static final long SCALE_TO_ZERO_AFTER_NO_REQUESTS_TIME_SECONDS = TimeValue.timeValueMinutes(15).getSeconds(); + /** * If the max_number_of_allocations is not set, use this value for now to prevent scaling up * to high numbers due to possible bugs or unexpected behaviour in the scaler. @@ -33,6 +40,7 @@ public class AdaptiveAllocationsScaler { private final String deploymentId; private final KalmanFilter1d requestRateEstimator; private final KalmanFilter1d inferenceTimeEstimator; + private double timeWithoutRequestsSeconds; private int numberOfAllocations; private int neededNumberOfAllocations; @@ -55,6 +63,7 @@ public class AdaptiveAllocationsScaler { // the number of allocations changes, which is passed explicitly to the estimator. requestRateEstimator = new KalmanFilter1d(deploymentId + ":rate", 100, true); inferenceTimeEstimator = new KalmanFilter1d(deploymentId + ":time", 100, false); + timeWithoutRequestsSeconds = 0.0; this.numberOfAllocations = numberOfAllocations; neededNumberOfAllocations = numberOfAllocations; minNumberOfAllocations = null; @@ -73,6 +82,11 @@ void setMinMaxNumberOfAllocations(Integer minNumberOfAllocations, Integer maxNum void process(AdaptiveAllocationsScalerService.Stats stats, double timeIntervalSeconds, int numberOfAllocations) { lastMeasuredQueueSize = stats.pendingCount(); + if (stats.requestCount() > 0) { + timeWithoutRequestsSeconds = 0.0; + } else { + timeWithoutRequestsSeconds += timeIntervalSeconds; + } // The request rate (per second) is the request count divided by the time. // Assuming a Poisson process for the requests, the variance in the request @@ -145,7 +159,7 @@ Integer scale() { numberOfAllocations--; } - this.neededNumberOfAllocations = numberOfAllocations; + neededNumberOfAllocations = numberOfAllocations; if (maxNumberOfAllocations == null) { numberOfAllocations = Math.min(numberOfAllocations, MAX_NUMBER_OF_ALLOCATIONS_SAFEGUARD); @@ -156,6 +170,13 @@ Integer scale() { if (maxNumberOfAllocations != null) { numberOfAllocations = Math.min(numberOfAllocations, maxNumberOfAllocations); } + if (ScaleToZeroFeatureFlag.isEnabled() + && (minNumberOfAllocations == null || minNumberOfAllocations == 0) + && timeWithoutRequestsSeconds > SCALE_TO_ZERO_AFTER_NO_REQUESTS_TIME_SECONDS) { + logger.debug("[{}] adaptive allocations scaler: scaling down to zero, because of no requests.", deploymentId); + numberOfAllocations = 0; + neededNumberOfAllocations = 0; + } if (numberOfAllocations != oldNumberOfAllocations) { logger.debug( diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerService.java index bbe90f769818b..775279a6b2553 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerService.java @@ -415,49 +415,60 @@ private void processDeploymentStats(GetDeploymentStatsAction.Response statsRespo if (newNumberOfAllocations > numberOfAllocations.get(deploymentId)) { lastScaleUpTimesMillis.put(deploymentId, now); } - UpdateTrainedModelDeploymentAction.Request updateRequest = new UpdateTrainedModelDeploymentAction.Request(deploymentId); - updateRequest.setNumberOfAllocations(newNumberOfAllocations); - updateRequest.setIsInternal(true); - ClientHelper.executeAsyncWithOrigin( - client, - ClientHelper.ML_ORIGIN, - UpdateTrainedModelDeploymentAction.INSTANCE, - updateRequest, - ActionListener.wrap(updateResponse -> { - logger.info("adaptive allocations scaler: scaled [{}] to [{}] allocations.", deploymentId, newNumberOfAllocations); - threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME) - .execute( - () -> inferenceAuditor.info( - deploymentId, - Strings.format( - "adaptive allocations scaler: scaled [%s] to [%s] allocations.", - deploymentId, - newNumberOfAllocations - ) - ) - ); - }, e -> { - logger.atLevel(Level.WARN) - .withThrowable(e) - .log( - "adaptive allocations scaler: scaling [{}] to [{}] allocations failed.", - deploymentId, - newNumberOfAllocations - ); - threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME) - .execute( - () -> inferenceAuditor.warning( - deploymentId, - Strings.format( - "adaptive allocations scaler: scaling [%s] to [%s] allocations failed.", - deploymentId, - newNumberOfAllocations - ) - ) - ); - }) - ); + updateNumberOfAllocations(deploymentId, newNumberOfAllocations); } } } + + public boolean maybeStartAllocation(TrainedModelAssignment assignment) { + if (ScaleToZeroFeatureFlag.isEnabled() + && assignment.getAdaptiveAllocationsSettings() != null + && assignment.getAdaptiveAllocationsSettings().getEnabled() == Boolean.TRUE) { + lastScaleUpTimesMillis.put(assignment.getDeploymentId(), System.currentTimeMillis()); + updateNumberOfAllocations(assignment.getDeploymentId(), 1); + return true; + } + return false; + } + + private void updateNumberOfAllocations(String deploymentId, int numberOfAllocations) { + UpdateTrainedModelDeploymentAction.Request updateRequest = new UpdateTrainedModelDeploymentAction.Request(deploymentId); + updateRequest.setNumberOfAllocations(numberOfAllocations); + updateRequest.setIsInternal(true); + ClientHelper.executeAsyncWithOrigin( + client, + ClientHelper.ML_ORIGIN, + UpdateTrainedModelDeploymentAction.INSTANCE, + updateRequest, + ActionListener.wrap(updateResponse -> { + logger.info("adaptive allocations scaler: scaled [{}] to [{}] allocations.", deploymentId, numberOfAllocations); + threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME) + .execute( + () -> inferenceAuditor.info( + deploymentId, + Strings.format( + "adaptive allocations scaler: scaled [%s] to [%s] allocations.", + deploymentId, + numberOfAllocations + ) + ) + ); + }, e -> { + logger.atLevel(Level.WARN) + .withThrowable(e) + .log("adaptive allocations scaler: scaling [{}] to [{}] allocations failed.", deploymentId, numberOfAllocations); + threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME) + .execute( + () -> inferenceAuditor.warning( + deploymentId, + Strings.format( + "adaptive allocations scaler: scaling [%s] to [%s] allocations failed.", + deploymentId, + numberOfAllocations + ) + ) + ); + }) + ); + } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/ScaleToZeroFeatureFlag.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/ScaleToZeroFeatureFlag.java new file mode 100644 index 0000000000000..072b8c5593c93 --- /dev/null +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/ScaleToZeroFeatureFlag.java @@ -0,0 +1,20 @@ +/* + * 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.ml.inference.adaptiveallocations; + +import org.elasticsearch.common.util.FeatureFlag; + +public class ScaleToZeroFeatureFlag { + private ScaleToZeroFeatureFlag() {} + + private static final FeatureFlag FEATURE_FLAG = new FeatureFlag("inference_scale_to_zero"); + + public static boolean isEnabled() { + return FEATURE_FLAG.isEnabled(); + } +} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java index a5b9597886e15..80c957ecb7a09 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java @@ -17,11 +17,9 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; -import org.elasticsearch.telemetry.metric.MeterRegistry; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.ml.inference.adaptiveallocations.AdaptiveAllocationsScalerService; -import org.elasticsearch.xpack.ml.notifications.InferenceAuditor; import org.junit.Before; import java.util.Map; @@ -40,8 +38,7 @@ public class MlInitializationServiceTests extends ESTestCase { private ThreadPool threadPool; private ClusterService clusterService; private Client client; - private InferenceAuditor inferenceAuditor; - private MeterRegistry meterRegistry; + private AdaptiveAllocationsScalerService adaptiveAllocationsScalerService; private MlAssignmentNotifier mlAssignmentNotifier; @Before @@ -50,8 +47,7 @@ public void setUpMocks() { threadPool = deterministicTaskQueue.getThreadPool(); clusterService = mock(ClusterService.class); client = mock(Client.class); - inferenceAuditor = mock(InferenceAuditor.class); - meterRegistry = mock(MeterRegistry.class); + adaptiveAllocationsScalerService = mock(AdaptiveAllocationsScalerService.class); mlAssignmentNotifier = mock(MlAssignmentNotifier.class); when(clusterService.getClusterName()).thenReturn(CLUSTER_NAME); @@ -77,8 +73,7 @@ public void testInitialize() { threadPool, clusterService, client, - inferenceAuditor, - meterRegistry, + adaptiveAllocationsScalerService, mlAssignmentNotifier, true, true, @@ -94,8 +89,7 @@ public void testInitialize_noMasterNode() { threadPool, clusterService, client, - inferenceAuditor, - meterRegistry, + adaptiveAllocationsScalerService, mlAssignmentNotifier, true, true, diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerTests.java index 08097357725d0..44aaba88c58a8 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerTests.java @@ -146,4 +146,47 @@ public void testAutoscaling_maxAllocationsSafeguard() { adaptiveAllocationsScaler.setMinMaxNumberOfAllocations(2, 77); assertThat(adaptiveAllocationsScaler.scale(), equalTo(77)); } + + public void testAutoscaling_scaleDownToZeroAllocations() { + AdaptiveAllocationsScaler adaptiveAllocationsScaler = new AdaptiveAllocationsScaler("test-deployment", 1); + // 1 hour with 1 request per 1 seconds, so don't scale. + for (int i = 0; i < 3600; i++) { + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(1, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + } + // 15 minutes with no requests, so don't scale. + for (int i = 0; i < 900; i++) { + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(0, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + } + // 1 second with a request, so don't scale. + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(1, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + // 15 minutes with no requests, so don't scale. + for (int i = 0; i < 900; i++) { + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(0, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + } + // another second with no requests, so scale to zero allocations. + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(0, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), equalTo(0)); + // 15 minutes with no requests, so don't scale. + for (int i = 0; i < 900; i++) { + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(0, 0, 0, 0.05), 1, 0); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + } + } + + public void testAutoscaling_dontScaleDownToZeroAllocationsWhenMinAllocationsIsSet() { + assumeTrue("Should only run if adaptive allocations feature flag is enabled", ScaleToZeroFeatureFlag.isEnabled()); + + AdaptiveAllocationsScaler adaptiveAllocationsScaler = new AdaptiveAllocationsScaler("test-deployment", 1); + adaptiveAllocationsScaler.setMinMaxNumberOfAllocations(1, null); + + // 1 hour with no requests, + for (int i = 0; i < 3600; i++) { + adaptiveAllocationsScaler.process(new AdaptiveAllocationsScalerService.Stats(1, 0, 0, 0.05), 1, 1); + assertThat(adaptiveAllocationsScaler.scale(), nullValue()); + } + } } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java index 32337f0d66896..44cbf03f220a1 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerIntegTests.java @@ -26,18 +26,24 @@ import org.elasticsearch.xpack.core.security.action.user.PutUserResponse; import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.hamcrest.Matchers; +import org.junit.After; import org.junit.Before; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -45,6 +51,14 @@ public class SecurityIndexManagerIntegTests extends SecurityIntegTestCase { + private final int concurrentCallsToOnAvailable = 6; + private final ExecutorService executor = Executors.newFixedThreadPool(concurrentCallsToOnAvailable); + + @After + public void shutdownExecutor() { + executor.shutdown(); + } + public void testConcurrentOperationsTryingToCreateSecurityIndexAndAlias() throws Exception { final int processors = Runtime.getRuntime().availableProcessors(); final int numThreads = Math.min(50, scaledRandomIntBetween((processors + 1) / 2, 4 * processors)); // up to 50 threads @@ -110,6 +124,12 @@ public void testOnIndexAvailableForSearchIndexCompletesWithinTimeout() throws Ex // pick longer wait than in the assertBusy that waits for below to ensure index has had enough time to initialize securityIndexManager.onIndexAvailableForSearch((ActionListener) future, TimeValue.timeValueSeconds(40)); + // check listener added + assertThat( + securityIndexManager.getStateChangeListeners(), + hasItem(instanceOf(SecurityIndexManager.StateConsumerWithCancellable.class)) + ); + createSecurityIndexWithWaitForActiveShards(); assertBusy( @@ -121,6 +141,12 @@ public void testOnIndexAvailableForSearchIndexCompletesWithinTimeout() throws Ex // security index creation is complete and index is available for search; therefore whenIndexAvailableForSearch should report // success in time future.actionGet(); + + // check no remaining listeners + assertThat( + securityIndexManager.getStateChangeListeners(), + not(hasItem(instanceOf(SecurityIndexManager.StateConsumerWithCancellable.class))) + ); } @SuppressWarnings("unchecked") @@ -152,6 +178,69 @@ public void testOnIndexAvailableForSearchIndexAlreadyAvailable() throws Exceptio securityIndexManager.onIndexAvailableForSearch((ActionListener) future, TimeValue.timeValueSeconds(10)); future.actionGet(); } + + // check no remaining listeners + assertThat( + securityIndexManager.getStateChangeListeners(), + not(hasItem(instanceOf(SecurityIndexManager.StateConsumerWithCancellable.class))) + ); + } + + @SuppressWarnings("unchecked") + public void testOnIndexAvailableForSearchIndexUnderConcurrentLoad() throws Exception { + final SecurityIndexManager securityIndexManager = internalCluster().getInstances(NativePrivilegeStore.class) + .iterator() + .next() + .getSecurityIndexManager(); + // Long time out calls should all succeed + final List> futures = new ArrayList<>(); + for (int i = 0; i < concurrentCallsToOnAvailable / 2; i++) { + final Future future = executor.submit(() -> { + try { + final ActionFuture f = new PlainActionFuture<>(); + securityIndexManager.onIndexAvailableForSearch((ActionListener) f, TimeValue.timeValueSeconds(40)); + f.actionGet(); + } catch (Exception ex) { + fail(ex, "should not have encountered exception"); + } + return null; + }); + futures.add(future); + } + + // short time-out tasks should all time out + for (int i = 0; i < concurrentCallsToOnAvailable / 2; i++) { + final Future future = executor.submit(() -> { + expectThrows(ElasticsearchTimeoutException.class, () -> { + final ActionFuture f = new PlainActionFuture<>(); + securityIndexManager.onIndexAvailableForSearch((ActionListener) f, TimeValue.timeValueMillis(10)); + f.actionGet(); + }); + return null; + }); + futures.add(future); + } + + // Sleep a second for short-running calls to timeout + Thread.sleep(1000); + + createSecurityIndexWithWaitForActiveShards(); + // ensure security index manager state is fully in the expected precondition state for this test (ready for search) + assertBusy( + () -> assertThat(securityIndexManager.isAvailable(SecurityIndexManager.Availability.SEARCH_SHARDS), is(true)), + 30, + TimeUnit.SECONDS + ); + + for (var future : futures) { + future.get(10, TimeUnit.SECONDS); + } + + // check no remaining listeners + assertThat( + securityIndexManager.getStateChangeListeners(), + not(hasItem(instanceOf(SecurityIndexManager.StateConsumerWithCancellable.class))) + ); } @SuppressWarnings("unchecked") @@ -163,9 +252,24 @@ public void testOnIndexAvailableForSearchIndexWaitTimeOut() { .next() .getSecurityIndexManager(); - final ActionFuture future = new PlainActionFuture<>(); - securityIndexManager.onIndexAvailableForSearch((ActionListener) future, TimeValue.timeValueMillis(100)); - expectThrows(ElasticsearchTimeoutException.class, future::actionGet); + { + final ActionFuture future = new PlainActionFuture<>(); + securityIndexManager.onIndexAvailableForSearch((ActionListener) future, TimeValue.timeValueMillis(100)); + expectThrows(ElasticsearchTimeoutException.class, future::actionGet); + } + + // Also works with 0 timeout + { + final ActionFuture future = new PlainActionFuture<>(); + securityIndexManager.onIndexAvailableForSearch((ActionListener) future, TimeValue.timeValueMillis(0)); + expectThrows(ElasticsearchTimeoutException.class, future::actionGet); + } + + // check no remaining listeners + assertThat( + securityIndexManager.getStateChangeListeners(), + not(hasItem(instanceOf(SecurityIndexManager.StateConsumerWithCancellable.class))) + ); } public void testSecurityIndexSettingsCannotBeChanged() throws Exception { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index a6c8de003c159..6d9b0ef6aeebe 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -54,6 +54,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -364,45 +365,80 @@ public void accept(State previousState, State nextState) { * Notifies {@code listener} once the security index is available, or calls {@code onFailure} on {@code timeout}. */ public void onIndexAvailableForSearch(ActionListener listener, TimeValue timeout) { - logger.info("Will wait for security index [{}] to become available for search", getConcreteIndexName()); + logger.info("Will wait for security index [{}] for [{}] to become available for search", getConcreteIndexName(), timeout); - final ActionListener notifyOnceListener = ActionListener.notifyOnce(listener); + if (state.indexAvailableForSearch) { + logger.debug("Security index [{}] is already available", getConcreteIndexName()); + listener.onResponse(null); + return; + } + final AtomicBoolean isDone = new AtomicBoolean(false); final var indexAvailableForSearchListener = new StateConsumerWithCancellable() { @Override public void accept(SecurityIndexManager.State previousState, SecurityIndexManager.State nextState) { if (nextState.indexAvailableForSearch) { - assert cancellable != null; - // cancel and removeStateListener are idempotent - cancellable.cancel(); - removeStateListener(this); - notifyOnceListener.onResponse(null); + if (isDone.compareAndSet(false, true)) { + cancel(); + removeStateListener(this); + listener.onResponse(null); + } } } }; + // add listener _before_ registering timeout -- this way we are guaranteed it gets removed (either by timeout below, or successful + // completion above) + addStateListener(indexAvailableForSearchListener); + // schedule failure handling on timeout -- keep reference to cancellable so a successful completion can cancel the timeout - indexAvailableForSearchListener.cancellable = client.threadPool().schedule(() -> { - removeStateListener(indexAvailableForSearchListener); - notifyOnceListener.onFailure( - new ElasticsearchTimeoutException( - "timed out waiting for security index [" + getConcreteIndexName() + "] to become available for search" - ) - ); - }, timeout, client.threadPool().generic()); + indexAvailableForSearchListener.setCancellable(client.threadPool().schedule(() -> { + if (isDone.compareAndSet(false, true)) { + removeStateListener(indexAvailableForSearchListener); + listener.onFailure( + new ElasticsearchTimeoutException( + "timed out waiting for security index [" + getConcreteIndexName() + "] to become available for search" + ) + ); + } + }, timeout, client.threadPool().generic())); + } - // in case the state has meanwhile changed to available, return immediately - if (state.indexAvailableForSearch) { - indexAvailableForSearchListener.cancellable.cancel(); - notifyOnceListener.onResponse(null); - } else { - addStateListener(indexAvailableForSearchListener); - } + // pkg-private for testing + List> getStateChangeListeners() { + return stateChangeListeners; } - private abstract static class StateConsumerWithCancellable + /** + * This class ensures that if cancel() is called _before_ setCancellable(), the passed-in cancellable is still correctly cancelled on + * a subsequent setCancellable() call. + */ + // pkg-private for testing + abstract static class StateConsumerWithCancellable implements - BiConsumer { - volatile Scheduler.ScheduledCancellable cancellable; + BiConsumer, + Scheduler.Cancellable { + private volatile Scheduler.ScheduledCancellable cancellable; + private volatile boolean cancelled = false; + + void setCancellable(Scheduler.ScheduledCancellable cancellable) { + this.cancellable = cancellable; + if (cancelled) { + cancel(); + } + } + + public boolean cancel() { + cancelled = true; + if (cancellable != null) { + // cancellable is idempotent, so it's fine to potentially call it multiple times + return cancellable.cancel(); + } + return isCancelled(); + } + + public boolean isCancelled() { + return cancelled; + } } private Tuple checkIndexAvailable(ClusterState state) { diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/70_locale.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/70_locale.yml index 05edf6cdfb5a8..5a9a2a21e21bc 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/70_locale.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/70_locale.yml @@ -29,7 +29,7 @@ setup: - do: allowed_warnings_regex: - "No limit defined, adding default limit of \\[.*\\]" - - "Date format \\[MMMM\\] contains textual field specifiers that could change in JDK 23" + - "Date format \\[MMMM\\] contains textual field specifiers that could change in JDK 23.*" esql.query: body: query: 'FROM events | eval fixed_format = date_format("MMMM", @timestamp), variable_format = date_format(format, @timestamp) | sort @timestamp | keep @timestamp, fixed_format, variable_format' @@ -51,7 +51,7 @@ setup: - do: allowed_warnings_regex: - "No limit defined, adding default limit of \\[.*\\]" - - "Date format \\[MMMM\\] contains textual field specifiers that could change in JDK 23" + - "Date format \\[MMMM\\] contains textual field specifiers that could change in JDK 23.*" esql.query: body: query: 'FROM events | eval fixed_format = date_format("MMMM", @timestamp), variable_format = date_format(format, @timestamp) | sort @timestamp | keep @timestamp, fixed_format, variable_format' diff --git a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/LegacyStackTemplateRegistry.java b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/LegacyStackTemplateRegistry.java index 62d22c0c0a9cc..b2dc04c1178e4 100644 --- a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/LegacyStackTemplateRegistry.java +++ b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/LegacyStackTemplateRegistry.java @@ -51,12 +51,7 @@ public class LegacyStackTemplateRegistry extends IndexTemplateRegistry { private final FeatureService featureService; private volatile boolean stackTemplateEnabled; - private static final Map ADDITIONAL_TEMPLATE_VARIABLES = Map.of( - "xpack.stack.template.deprecated", - "true", - "xpack.stack.template.logsdb.index.mode", - "standard" - ); + private static final Map ADDITIONAL_TEMPLATE_VARIABLES = Map.of("xpack.stack.template.deprecated", "true"); // General mappings conventions for any data that ends up in a data stream public static final String DATA_STREAMS_MAPPINGS_COMPONENT_TEMPLATE_NAME = "data-streams-mappings"; diff --git a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackPlugin.java b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackPlugin.java index cc127883652af..71d01798323d3 100644 --- a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackPlugin.java +++ b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackPlugin.java @@ -23,7 +23,7 @@ public StackPlugin(Settings settings) { @Override public List> getSettings() { - return List.of(StackTemplateRegistry.STACK_TEMPLATES_ENABLED, StackTemplateRegistry.CLUSTER_LOGSDB_ENABLED); + return List.of(StackTemplateRegistry.STACK_TEMPLATES_ENABLED); } @Override diff --git a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackTemplateRegistry.java b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackTemplateRegistry.java index 592842f61eee8..b45f17e434388 100644 --- a/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackTemplateRegistry.java +++ b/x-pack/plugin/stack/src/main/java/org/elasticsearch/xpack/stack/StackTemplateRegistry.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.features.FeatureService; import org.elasticsearch.features.NodeFeature; -import org.elasticsearch.index.IndexMode; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.XContentParserConfiguration; @@ -36,6 +35,8 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.xpack.cluster.settings.ClusterSettings.CLUSTER_LOGSDB_ENABLED; + public class StackTemplateRegistry extends IndexTemplateRegistry { private static final Logger logger = LogManager.getLogger(StackTemplateRegistry.class); @@ -58,15 +59,6 @@ public class StackTemplateRegistry extends IndexTemplateRegistry { Setting.Property.Dynamic ); - /** - * if index.mode "logsdb" is applied by default in logs@settings for 'logs-*-*' - */ - public static final Setting CLUSTER_LOGSDB_ENABLED = Setting.boolSetting( - "cluster.logsdb.enabled", - false, - Setting.Property.NodeScope - ); - private final ClusterService clusterService; private final FeatureService featureService; private final Map componentTemplateConfigs; @@ -167,15 +159,10 @@ private Map loadComponentTemplateConfigs(boolean logs ), new IndexTemplateConfig( LOGS_SETTINGS_COMPONENT_TEMPLATE_NAME, - "/logs@settings.json", + logsDbEnabled ? "/logs@settings-logsdb.json" : "/logs@settings.json", REGISTRY_VERSION, TEMPLATE_VERSION_VARIABLE, - Map.of( - "xpack.stack.template.deprecated", - "false", - "xpack.stack.template.logsdb.index.mode", - logsDbEnabled ? IndexMode.LOGSDB.getName() : IndexMode.STANDARD.getName() - ) + Map.of("xpack.stack.template.deprecated", "false") ), new IndexTemplateConfig( METRICS_MAPPINGS_COMPONENT_TEMPLATE_NAME,