From 2ecfb397ad4bf4a83eaa4497b9b187058e2f3722 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 20 Sep 2024 07:45:40 -0700 Subject: [PATCH 01/46] Remove plugin classloader indirection (#113154) Extensible plugins use a custom classloader for other plugin jars. When extensible plugins were first added, the transport client still existed, and elasticsearch plugins did not exist in the transport client (at least not the ones that create classloaders). Yet the transport client still created a PluginsService. An indirection was used to avoid creating separate classloaders when the transport client had created the PluginsService. The transport client was removed in 8.0, but the indirection still exists. This commit removes that indirection layer. --- ...alDistributionModuleCheckTaskProvider.java | 1 - libs/plugin-classloader/build.gradle | 18 ------------- .../src/main/java/module-info.java | 12 --------- server/build.gradle | 2 -- server/src/main/java/module-info.java | 1 - .../plugins}/ExtendedPluginsClassLoader.java | 4 +-- .../plugins/PluginLoaderIndirection.java | 26 ------------------- .../elasticsearch/plugins/PluginsService.java | 2 +- .../elasticsearch/bootstrap/security.policy | 5 ---- 9 files changed, 3 insertions(+), 68 deletions(-) delete mode 100644 libs/plugin-classloader/build.gradle delete mode 100644 libs/plugin-classloader/src/main/java/module-info.java rename {libs/plugin-classloader/src/main/java/org/elasticsearch/plugins/loader => server/src/main/java/org/elasticsearch/plugins}/ExtendedPluginsClassLoader.java (94%) delete mode 100644 server/src/main/java/org/elasticsearch/plugins/PluginLoaderIndirection.java diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java index f7d2b26e069e1..bbf411dbf04fa 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java @@ -59,7 +59,6 @@ public class InternalDistributionModuleCheckTaskProvider { "org.elasticsearch.nativeaccess", "org.elasticsearch.plugin", "org.elasticsearch.plugin.analysis", - "org.elasticsearch.pluginclassloader", "org.elasticsearch.securesm", "org.elasticsearch.server", "org.elasticsearch.simdvec", diff --git a/libs/plugin-classloader/build.gradle b/libs/plugin-classloader/build.gradle deleted file mode 100644 index f54bec211286a..0000000000000 --- a/libs/plugin-classloader/build.gradle +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", 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". - */ - -// This is only required because :server needs this at runtime. -// We'll be removing this in 8.0 so for now just publish the JAR to make dependency resolution work. -apply plugin: 'elasticsearch.publish' - -tasks.named("test").configure { enabled = false } - -// test depend on ES core... -tasks.named('forbiddenApisMain').configure { enabled = false} -tasks.named("jarHell").configure { enabled = false } diff --git a/libs/plugin-classloader/src/main/java/module-info.java b/libs/plugin-classloader/src/main/java/module-info.java deleted file mode 100644 index 90549c5c4a01b..0000000000000 --- a/libs/plugin-classloader/src/main/java/module-info.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", 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". - */ - -module org.elasticsearch.pluginclassloader { - exports org.elasticsearch.plugins.loader; -} diff --git a/server/build.gradle b/server/build.gradle index 5492ca00e2d3b..5c12d47da8102 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -40,8 +40,6 @@ dependencies { api project(":libs:elasticsearch-tdigest") implementation project(":libs:elasticsearch-simdvec") - implementation project(':libs:elasticsearch-plugin-classloader') - // lucene api "org.apache.lucene:lucene-core:${versions.lucene}" api "org.apache.lucene:lucene-analysis-common:${versions.lucene}" diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index 696624a4a8f27..4369c35c50ea3 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -25,7 +25,6 @@ requires org.elasticsearch.nativeaccess; requires org.elasticsearch.geo; requires org.elasticsearch.lz4; - requires org.elasticsearch.pluginclassloader; requires org.elasticsearch.securesm; requires org.elasticsearch.xcontent; requires org.elasticsearch.logging; diff --git a/libs/plugin-classloader/src/main/java/org/elasticsearch/plugins/loader/ExtendedPluginsClassLoader.java b/server/src/main/java/org/elasticsearch/plugins/ExtendedPluginsClassLoader.java similarity index 94% rename from libs/plugin-classloader/src/main/java/org/elasticsearch/plugins/loader/ExtendedPluginsClassLoader.java rename to server/src/main/java/org/elasticsearch/plugins/ExtendedPluginsClassLoader.java index be3e76bd83396..d9bf0d653bb62 100644 --- a/libs/plugin-classloader/src/main/java/org/elasticsearch/plugins/loader/ExtendedPluginsClassLoader.java +++ b/server/src/main/java/org/elasticsearch/plugins/ExtendedPluginsClassLoader.java @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.plugins.loader; +package org.elasticsearch.plugins; import java.security.AccessController; import java.security.PrivilegedAction; @@ -17,7 +17,7 @@ /** * A classloader that is a union over the parent core classloader and classloaders of extended plugins. */ -public class ExtendedPluginsClassLoader extends ClassLoader { +class ExtendedPluginsClassLoader extends ClassLoader { /** Loaders of plugins extended by a plugin. */ private final List extendedLoaders; diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginLoaderIndirection.java b/server/src/main/java/org/elasticsearch/plugins/PluginLoaderIndirection.java deleted file mode 100644 index a5ca26283231d..0000000000000 --- a/server/src/main/java/org/elasticsearch/plugins/PluginLoaderIndirection.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", 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.plugins; - -import org.elasticsearch.plugins.loader.ExtendedPluginsClassLoader; - -import java.util.List; - -// TODO: remove this indirection now that transport client is gone -/** - * This class exists solely as an intermediate layer to avoid causing PluginsService - * to load ExtendedPluginsClassLoader when used in the transport client. - */ -class PluginLoaderIndirection { - - static ClassLoader createLoader(ClassLoader parent, List extendedLoaders) { - return ExtendedPluginsClassLoader.create(parent, extendedLoaders); - } -} diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 47f0a50ff309c..d5dd6d62d615e 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -468,7 +468,7 @@ private void loadBundle( ); } - final ClassLoader parentLoader = PluginLoaderIndirection.createLoader( + final ClassLoader parentLoader = ExtendedPluginsClassLoader.create( getClass().getClassLoader(), extendedPlugins.stream().map(LoadedPlugin::loader).toList() ); diff --git a/server/src/main/resources/org/elasticsearch/bootstrap/security.policy b/server/src/main/resources/org/elasticsearch/bootstrap/security.policy index 756e8106f631a..55abdc84fc8fb 100644 --- a/server/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/server/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -53,11 +53,6 @@ grant codeBase "${codebase.lucene-misc}" { permission java.nio.file.LinkPermission "hard"; }; -grant codeBase "${codebase.elasticsearch-plugin-classloader}" { - // needed to create the classloader which allows plugins to extend other plugins - permission java.lang.RuntimePermission "createClassLoader"; -}; - grant codeBase "${codebase.elasticsearch-core}" { permission java.lang.RuntimePermission "createClassLoader"; permission java.lang.RuntimePermission "getClassLoader"; From aa71954ccb003fff3491b5041fb8c4856ab208a2 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 20 Sep 2024 09:14:22 -0600 Subject: [PATCH 02/46] [ES|QL] Use describe method to generate OrdinalGroupingOperator profile message (#113150) * Use describe method to generate aggregators string * Improve message * remove incorrect query change --- .../operator/OrdinalsGroupingOperator.java | 7 ++++- .../xpack/esql/qa/single_node/RestEsqlIT.java | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java index b5ae35bfc8d7f..5e0e625abb914 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java @@ -46,6 +46,7 @@ import java.util.Objects; import java.util.function.IntFunction; import java.util.function.Supplier; +import java.util.stream.Collectors; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.joining; @@ -325,7 +326,11 @@ private static void checkState(boolean condition, String msg) { @Override public String toString() { - return this.getClass().getSimpleName() + "[" + "aggregators=" + aggregatorFactories + "]"; + String aggregatorDescriptions = aggregatorFactories.stream() + .map(factory -> "\"" + factory.describe() + "\"") + .collect(Collectors.joining(", ")); + + return this.getClass().getSimpleName() + "[" + "aggregators=[" + aggregatorDescriptions + "]]"; } record SegmentID(int shardIndex, int segmentIndex) { diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java index 44550c62bd7c5..2a882550b67b1 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java @@ -323,6 +323,35 @@ public void testProfile() throws IOException { ); } + public void testProfileOrdinalsGroupingOperator() throws IOException { + indexTimestampData(1); + + RequestObjectBuilder builder = requestObjectBuilder().query(fromIndex() + " | STATS AVG(value) BY test.keyword"); + builder.profile(true); + if (Build.current().isSnapshot()) { + // Lock to shard level partitioning, so we get consistent profile output + builder.pragmas(Settings.builder().put("data_partitioning", "shard").build()); + } + Map result = runEsql(builder); + + List> signatures = new ArrayList<>(); + @SuppressWarnings("unchecked") + List> profiles = (List>) ((Map) result.get("profile")).get("drivers"); + for (Map p : profiles) { + fixTypesOnProfile(p); + assertThat(p, commonProfile()); + List sig = new ArrayList<>(); + @SuppressWarnings("unchecked") + List> operators = (List>) p.get("operators"); + for (Map o : operators) { + sig.add((String) o.get("operator")); + } + signatures.add(sig); + } + + assertThat(signatures.get(0).get(2), equalTo("OrdinalsGroupingOperator[aggregators=[\"sum of longs\", \"count\"]]")); + } + public void testInlineStatsProfile() throws IOException { assumeTrue("INLINESTATS only available on snapshots", Build.current().isSnapshot()); indexTimestampData(1); From d68f2fa4a69352a37fe3e49fbdd5aa034666baac Mon Sep 17 00:00:00 2001 From: Pm Ching <41728178+pionCham@users.noreply.github.com> Date: Fri, 20 Sep 2024 23:34:24 +0800 Subject: [PATCH 03/46] fix a couple of docs typos (#112901) --- docs/reference/esql/functions/kibana/docs/mv_avg.md | 2 +- docs/reference/esql/functions/kibana/docs/mv_sum.md | 2 +- docs/reference/query-dsl/sparse-vector-query.asciidoc | 2 +- docs/reference/query-dsl/text-expansion-query.asciidoc | 2 +- docs/reference/query-dsl/weighted-tokens-query.asciidoc | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/reference/esql/functions/kibana/docs/mv_avg.md b/docs/reference/esql/functions/kibana/docs/mv_avg.md index c3d7e5423f724..c5163f36129bf 100644 --- a/docs/reference/esql/functions/kibana/docs/mv_avg.md +++ b/docs/reference/esql/functions/kibana/docs/mv_avg.md @@ -3,7 +3,7 @@ This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../READ --> ### MV_AVG -Converts a multivalued field into a single valued field containing the average of all of the values. +Converts a multivalued field into a single valued field containing the average of all the values. ``` ROW a=[3, 5, 1, 6] diff --git a/docs/reference/esql/functions/kibana/docs/mv_sum.md b/docs/reference/esql/functions/kibana/docs/mv_sum.md index 16285d3c7229b..987017b34b743 100644 --- a/docs/reference/esql/functions/kibana/docs/mv_sum.md +++ b/docs/reference/esql/functions/kibana/docs/mv_sum.md @@ -3,7 +3,7 @@ This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../READ --> ### MV_SUM -Converts a multivalued field into a single valued field containing the sum of all of the values. +Converts a multivalued field into a single valued field containing the sum of all the values. ``` ROW a=[3, 5, 6] diff --git a/docs/reference/query-dsl/sparse-vector-query.asciidoc b/docs/reference/query-dsl/sparse-vector-query.asciidoc index 08dd7ab7f4470..399cf29d4dd12 100644 --- a/docs/reference/query-dsl/sparse-vector-query.asciidoc +++ b/docs/reference/query-dsl/sparse-vector-query.asciidoc @@ -104,7 +104,7 @@ Default: `5`. `tokens_weight_threshold`:: (Optional, float) preview:[] -Tokens whose weight is less than `tokens_weight_threshold` are considered nonsignificant and pruned. +Tokens whose weight is less than `tokens_weight_threshold` are considered insignificant and pruned. This value must be between 0 and 1. Default: `0.4`. diff --git a/docs/reference/query-dsl/text-expansion-query.asciidoc b/docs/reference/query-dsl/text-expansion-query.asciidoc index 8faecad1dbdb9..235a413df686f 100644 --- a/docs/reference/query-dsl/text-expansion-query.asciidoc +++ b/docs/reference/query-dsl/text-expansion-query.asciidoc @@ -68,7 +68,7 @@ Default: `5`. `tokens_weight_threshold`:: (Optional, float) preview:[] -Tokens whose weight is less than `tokens_weight_threshold` are considered nonsignificant and pruned. +Tokens whose weight is less than `tokens_weight_threshold` are considered insignificant and pruned. This value must be between 0 and 1. Default: `0.4`. diff --git a/docs/reference/query-dsl/weighted-tokens-query.asciidoc b/docs/reference/query-dsl/weighted-tokens-query.asciidoc index d4318665a9778..fb051f4229df6 100644 --- a/docs/reference/query-dsl/weighted-tokens-query.asciidoc +++ b/docs/reference/query-dsl/weighted-tokens-query.asciidoc @@ -58,7 +58,7 @@ This value must between 1 and 100. Default: `5`. `tokens_weight_threshold`:: -(Optional, float) Tokens whose weight is less than `tokens_weight_threshold` are considered nonsignificant and pruned. +(Optional, float) Tokens whose weight is less than `tokens_weight_threshold` are considered insignificant and pruned. This value must be between 0 and 1. Default: `0.4`. From 634d0e78838f9d43db8cc47b9228192c845ec4c3 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 01:41:45 +1000 Subject: [PATCH 04/46] Mute org.elasticsearch.xpack.security.authz.SecurityScrollTests testSearchAndClearScroll #113285 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 093768ee4c161..36cbd27b9b41f 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -274,6 +274,9 @@ tests: - class: org.elasticsearch.datastreams.logsdb.qa.StandardVersusLogsIndexModeRandomDataChallengeRestIT method: testMatchAllQuery issue: https://github.com/elastic/elasticsearch/issues/113265 +- class: org.elasticsearch.xpack.security.authz.SecurityScrollTests + method: testSearchAndClearScroll + issue: https://github.com/elastic/elasticsearch/issues/113285 # Examples: # From 33a73a8111fb7aa80667f56c4c1468dbb165c3af Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 20 Sep 2024 17:16:03 +0100 Subject: [PATCH 05/46] Trigger merges after recovery (#113102) We may have shut a shard down while merges were still pending (or adjusted the merge policy while the shard was down) meaning that after recovery its segments do not reflect the desired state according to the merge policy. With this commit we invoke `IndexWriter#maybeMerge()` at the end of recovery to check for, and execute, any such lost merges. --- docs/changelog/113102.yaml | 5 + .../decider/DiskThresholdDeciderIT.java | 4 +- .../indices/recovery/IndexRecoveryIT.java | 79 ++++++++++ .../elasticsearch/index/shard/IndexShard.java | 16 ++ .../elasticsearch/indices/IndicesService.java | 43 ++++-- .../indices/PostRecoveryMerger.java | 145 ++++++++++++++++++ 6 files changed, 273 insertions(+), 19 deletions(-) create mode 100644 docs/changelog/113102.yaml create mode 100644 server/src/main/java/org/elasticsearch/indices/PostRecoveryMerger.java diff --git a/docs/changelog/113102.yaml b/docs/changelog/113102.yaml new file mode 100644 index 0000000000000..ea9022e634caf --- /dev/null +++ b/docs/changelog/113102.yaml @@ -0,0 +1,5 @@ +pr: 113102 +summary: Trigger merges after recovery +area: Recovery +type: enhancement +issues: [] diff --git a/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java b/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java index ac1c44de6dcf6..2a275cf563d86 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java @@ -255,9 +255,9 @@ private Set getShardIds(final String nodeId, final String indexName) { /** * Index documents until all the shards are at least WATERMARK_BYTES in size, and return the one with the smallest size */ - private ShardSizes createReasonableSizedShards(final String indexName) throws InterruptedException { + private ShardSizes createReasonableSizedShards(final String indexName) { while (true) { - indexRandom(true, indexName, scaledRandomIntBetween(100, 10000)); + indexRandom(false, indexName, scaledRandomIntBetween(100, 10000)); forceMerge(); refresh(); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java index 61cf49ff6ca4e..9f1a094c80623 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java @@ -34,6 +34,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.action.admin.indices.recovery.RecoveryRequest; import org.elasticsearch.action.admin.indices.recovery.RecoveryResponse; +import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags; import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.admin.indices.stats.ShardStats; @@ -86,6 +87,7 @@ import org.elasticsearch.index.analysis.AbstractTokenFilterFactory; import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.index.engine.Engine; +import org.elasticsearch.index.engine.Segment; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.SeqNoFieldMapper; import org.elasticsearch.index.recovery.RecoveryStats; @@ -137,8 +139,10 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; +import java.util.function.LongSupplier; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.util.stream.Stream; import static java.util.Collections.singletonMap; import static java.util.stream.Collectors.toList; @@ -146,7 +150,10 @@ import static org.elasticsearch.action.DocWriteResponse.Result.UPDATED; import static org.elasticsearch.action.support.ActionTestUtils.assertNoFailureListener; import static org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING; +import static org.elasticsearch.index.MergePolicyConfig.INDEX_MERGE_ENABLED; import static org.elasticsearch.index.seqno.SequenceNumbers.NO_OPS_PERFORMED; +import static org.elasticsearch.indices.IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING; +import static org.elasticsearch.node.NodeRoleSettings.NODE_ROLES_SETTING; import static org.elasticsearch.node.RecoverySettingsChunkSizePlugin.CHUNK_SIZE_SETTING; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; @@ -158,6 +165,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -1957,6 +1965,77 @@ public void accept(long globalCheckpoint, Exception e) { recoveryCompleteListener.onResponse(null); } + public void testPostRecoveryMerge() throws Exception { + internalCluster().startMasterOnlyNode(); + final var dataNode = internalCluster().startDataOnlyNode(); + final var indexName = randomIdentifier(); + createIndex(indexName, indexSettings(1, 0).put(INDEX_MERGE_ENABLED, false).build()); + + final var initialSegmentCount = 20; + for (int i = 0; i < initialSegmentCount; i++) { + indexDoc(indexName, Integer.toString(i), "f", randomAlphaOfLength(10)); + refresh(indexName); // force a one-doc segment + } + flush(indexName); // commit all the one-doc segments + + final LongSupplier searchableSegmentCountSupplier = () -> indicesAdmin().prepareSegments(indexName) + .get(SAFE_AWAIT_TIMEOUT) + .getIndices() + .get(indexName) + .getShards() + .get(0) + .shards()[0].getSegments() + .stream() + .filter(Segment::isSearch) + .count(); + + assertEquals(initialSegmentCount, searchableSegmentCountSupplier.getAsLong()); + + // force a recovery by restarting the node, re-enabling merges while the node is down, but configure the node not to be in the hot + // or content tiers so that it does not do any post-recovery merge + internalCluster().restartNode(dataNode, new InternalTestCluster.RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) { + final var request = new UpdateSettingsRequest(Settings.builder().putNull(INDEX_MERGE_ENABLED).build(), indexName); + request.reopen(true); + safeGet(indicesAdmin().updateSettings(request)); + return Settings.builder() + .putList(NODE_ROLES_SETTING.getKey(), randomNonEmptySubsetOf(List.of("data_warm", "data_cold"))) + .build(); + } + }); + + ensureGreen(indexName); + final var mergeStats = indicesAdmin().prepareStats(indexName).clear().setMerge(true).get().getIndex(indexName).getShards()[0] + .getStats() + .getMerge(); + assertEquals(0, mergeStats.getCurrent()); + assertEquals(0, mergeStats.getTotal()); + assertEquals(initialSegmentCount, searchableSegmentCountSupplier.getAsLong()); + + // force a recovery by restarting the node again, but this time putting it into the hot or content tiers to enable post-recovery + // merges + internalCluster().restartNode(dataNode, new InternalTestCluster.RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) { + return Settings.builder() + .putList( + NODE_ROLES_SETTING.getKey(), + Stream.concat( + Stream.of(randomFrom("data", "data_content", "data_hot")), + Stream.of("data", "data_content", "data_hot", "data_warm", "data_cold").filter(p -> randomBoolean()) + ).distinct().toList() + ) + // set the inactive time to zero so that we flush immediately after every merge, rather than having the test wait 5min + .put(SHARD_INACTIVE_TIME_SETTING.getKey(), TimeValue.ZERO) + .build(); + } + }); + + ensureGreen(indexName); + assertBusy(() -> assertThat(searchableSegmentCountSupplier.getAsLong(), lessThan((long) initialSegmentCount))); + } + private void assertGlobalCheckpointIsStableAndSyncedInAllNodes(String indexName, List nodes, int shard) throws Exception { assertThat(nodes, is(not(empty()))); diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index c4f0d72c06a80..62d2aa1f026f7 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -1514,6 +1514,22 @@ public void forceMerge(ForceMergeRequest forceMerge) throws IOException { engine.forceMerge(forceMerge.flush(), forceMerge.maxNumSegments(), forceMerge.onlyExpungeDeletes(), forceMerge.forceMergeUUID()); } + public void triggerPendingMerges() throws IOException { + switch (state /* single volatile read */) { + case STARTED, POST_RECOVERY -> getEngine().forceMerge( + // don't immediately flush - if any merging happens then we don't wait for it anyway + false, + // don't apply any segment count limit, we only want to call IndexWriter#maybeMerge + ForceMergeRequest.Defaults.MAX_NUM_SEGMENTS, + // don't look for expunge-delete merges, we only want to call IndexWriter#maybeMerge + false, + // force-merge UUID is not used when calling IndexWriter#maybeMerge + null + ); + // otherwise shard likely closed and maybe reopened, nothing to do + } + } + /** * Creates a new {@link IndexCommit} snapshot from the currently running engine. All resources referenced by this * commit won't be freed until the commit / snapshot is closed. diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index 89f651468068d..43bfe7e0e7e2c 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -262,6 +262,7 @@ public class IndicesService extends AbstractLifecycleComponent private final TimestampFieldMapperService timestampFieldMapperService; private final CheckedBiConsumer requestCacheKeyDifferentiator; private final MapperMetrics mapperMetrics; + private final PostRecoveryMerger postRecoveryMerger; @Override protected void doStart() { @@ -378,6 +379,8 @@ public void onRemoval(ShardId shardId, String fieldName, boolean wasEvicted, lon clusterService.getClusterSettings().addSettingsUpdateConsumer(ALLOW_EXPENSIVE_QUERIES, this::setAllowExpensiveQueries); this.timestampFieldMapperService = new TimestampFieldMapperService(settings, threadPool, this); + + this.postRecoveryMerger = new PostRecoveryMerger(settings, threadPool.executor(ThreadPool.Names.FORCE_MERGE), this::getShardOrNull); } private static final String DANGLING_INDICES_UPDATE_THREAD_NAME = "DanglingIndices#updateTask"; @@ -890,23 +893,29 @@ public void createShard( RecoveryState recoveryState = indexService.createRecoveryState(shardRouting, targetNode, sourceNode); IndexShard indexShard = indexService.createShard(shardRouting, globalCheckpointSyncer, retentionLeaseSyncer); indexShard.addShardFailureCallback(onShardFailure); - indexShard.startRecovery(recoveryState, recoveryTargetService, recoveryListener, repositoriesService, (mapping, listener) -> { - assert recoveryState.getRecoverySource().getType() == RecoverySource.Type.LOCAL_SHARDS - : "mapping update consumer only required by local shards recovery"; - AcknowledgedRequest putMappingRequestAcknowledgedRequest = new PutMappingRequest().setConcreteIndex( - shardRouting.index() - ) - .setConcreteIndex(shardRouting.index()) // concrete index - no name clash, it uses uuid - .source(mapping.source().string(), XContentType.JSON); - // concrete index - no name clash, it uses uuid - client.execute( - featureService.clusterHasFeature(clusterService.state(), SUPPORTS_AUTO_PUT) - ? TransportAutoPutMappingAction.TYPE - : TransportPutMappingAction.TYPE, - putMappingRequestAcknowledgedRequest.ackTimeout(TimeValue.MAX_VALUE).masterNodeTimeout(TimeValue.MAX_VALUE), - new RefCountAwareThreadedActionListener<>(threadPool.generic(), listener.map(ignored -> null)) - ); - }, this, clusterStateVersion); + indexShard.startRecovery( + recoveryState, + recoveryTargetService, + postRecoveryMerger.maybeMergeAfterRecovery(shardRouting, recoveryListener), + repositoriesService, + (mapping, listener) -> { + assert recoveryState.getRecoverySource().getType() == RecoverySource.Type.LOCAL_SHARDS + : "mapping update consumer only required by local shards recovery"; + AcknowledgedRequest putMappingRequestAcknowledgedRequest = new PutMappingRequest() + // concrete index - no name clash, it uses uuid + .setConcreteIndex(shardRouting.index()) + .source(mapping.source().string(), XContentType.JSON); + client.execute( + featureService.clusterHasFeature(clusterService.state(), SUPPORTS_AUTO_PUT) + ? TransportAutoPutMappingAction.TYPE + : TransportPutMappingAction.TYPE, + putMappingRequestAcknowledgedRequest.ackTimeout(TimeValue.MAX_VALUE).masterNodeTimeout(TimeValue.MAX_VALUE), + new RefCountAwareThreadedActionListener<>(threadPool.generic(), listener.map(ignored -> null)) + ); + }, + this, + clusterStateVersion + ); } @Override diff --git a/server/src/main/java/org/elasticsearch/indices/PostRecoveryMerger.java b/server/src/main/java/org/elasticsearch/indices/PostRecoveryMerger.java new file mode 100644 index 0000000000000..11c8a2b4d6048 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/indices/PostRecoveryMerger.java @@ -0,0 +1,145 @@ +/* + * 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.indices; + +import org.apache.lucene.index.IndexWriter; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThrottledTaskRunner; +import org.elasticsearch.core.Releasable; +import org.elasticsearch.core.Strings; +import org.elasticsearch.index.shard.IndexShard; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.shard.ShardLongFieldRange; +import org.elasticsearch.indices.recovery.PeerRecoveryTargetService; +import org.elasticsearch.indices.recovery.RecoveryFailedException; +import org.elasticsearch.indices.recovery.RecoveryState; +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; + +import java.util.concurrent.Executor; +import java.util.function.Function; + +import static org.elasticsearch.cluster.node.DiscoveryNodeRole.DATA_CONTENT_NODE_ROLE; +import static org.elasticsearch.cluster.node.DiscoveryNodeRole.DATA_HOT_NODE_ROLE; +import static org.elasticsearch.cluster.node.DiscoveryNodeRole.DATA_ROLE; +import static org.elasticsearch.cluster.node.DiscoveryNodeRole.INDEX_ROLE; + +/** + * Triggers a check for pending merges when a shard completes recovery. + */ +class PostRecoveryMerger { + + private static final Logger logger = LogManager.getLogger(PostRecoveryMerger.class); + + private static final boolean TRIGGER_MERGE_AFTER_RECOVERY; + + static { + final var propertyValue = System.getProperty("es.trigger_merge_after_recovery"); + if (propertyValue == null) { + TRIGGER_MERGE_AFTER_RECOVERY = true; + } else if ("false".equals(propertyValue)) { + TRIGGER_MERGE_AFTER_RECOVERY = false; + } else { + throw new IllegalStateException( + "system property [es.trigger_merge_after_recovery] may only be set to [false], but was [" + propertyValue + "]" + ); + } + } + + /** + * Throttled runner to avoid multiple concurrent calls to {@link IndexWriter#maybeMerge()}: we do not need to execute these things + * especially quickly, as long as they happen eventually, and each such call may involve some IO (reading the soft-deletes doc values to + * count deleted docs). Note that we're not throttling any actual merges, just the checks to see what merges might be needed. Throttling + * merges across shards is a separate issue, but normally this mechanism won't trigger any new merges anyway. + */ + private final ThrottledTaskRunner postRecoveryMergeRunner; + + private final Function shardFunction; + private final boolean enabled; + + PostRecoveryMerger(Settings settings, Executor executor, Function shardFunction) { + this.postRecoveryMergeRunner = new ThrottledTaskRunner(getClass().getCanonicalName(), 1, executor); + this.shardFunction = shardFunction; + this.enabled = + // enabled globally ... + TRIGGER_MERGE_AFTER_RECOVERY + // ... and we are a node that expects nontrivial amounts of indexing work + && (DiscoveryNode.hasRole(settings, DATA_HOT_NODE_ROLE) + || DiscoveryNode.hasRole(settings, DATA_CONTENT_NODE_ROLE) + || DiscoveryNode.hasRole(settings, DATA_ROLE) + || DiscoveryNode.hasRole(settings, INDEX_ROLE)); + } + + PeerRecoveryTargetService.RecoveryListener maybeMergeAfterRecovery( + ShardRouting shardRouting, + PeerRecoveryTargetService.RecoveryListener recoveryListener + ) { + if (enabled == false) { + return recoveryListener; + } + + if (shardRouting.isPromotableToPrimary() == false) { + return recoveryListener; + } + + final var shardId = shardRouting.shardId(); + return new PeerRecoveryTargetService.RecoveryListener() { + @Override + public void onRecoveryDone( + RecoveryState state, + ShardLongFieldRange timestampMillisFieldRange, + ShardLongFieldRange eventIngestedMillisFieldRange + ) { + postRecoveryMergeRunner.enqueueTask(new PostRecoveryMerge(shardId)); + recoveryListener.onRecoveryDone(state, timestampMillisFieldRange, eventIngestedMillisFieldRange); + } + + @Override + public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) { + recoveryListener.onRecoveryFailure(e, sendShardFailure); + } + }; + } + + class PostRecoveryMerge implements ActionListener { + private final ShardId shardId; + + PostRecoveryMerge(ShardId shardId) { + this.shardId = shardId; + } + + @Override + public void onResponse(Releasable releasable) { + try (releasable) { + final var indexShard = shardFunction.apply(shardId); + if (indexShard == null) { + return; + } + + indexShard.triggerPendingMerges(); + } catch (Exception e) { + logFailure(e); + } + } + + @Override + public void onFailure(Exception e) { + logFailure(e); + } + + private void logFailure(Exception e) { + // post-recovery merge is a best-effort thing, failure needs no special handling + logger.debug(() -> Strings.format("failed to execute post-recovery merge of [%s]", shardId), e); + } + } +} From 5a1577137ee098b06e7d212f52aded37f4092a66 Mon Sep 17 00:00:00 2001 From: Oleksandr Kolomiiets Date: Fri, 20 Sep 2024 09:42:57 -0700 Subject: [PATCH 06/46] Apply workaround for known issue to logsdb test data generation (#113214) --- muted-tests.yml | 3 --- .../datageneration/datasource/DataSourceRequest.java | 9 ++++++--- .../datasource/DefaultMappingParametersHandler.java | 7 ++++++- .../fields/GenericSubObjectFieldDataGenerator.java | 3 ++- .../logsdb/datageneration/fields/PredefinedField.java | 4 ++-- 5 files changed, 16 insertions(+), 10 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 36cbd27b9b41f..a350c8209ae5a 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -235,9 +235,6 @@ tests: - class: org.elasticsearch.xpack.inference.rest.ServerSentEventsRestActionListenerTests method: testErrorMidStream issue: https://github.com/elastic/elasticsearch/issues/113179 -- class: org.elasticsearch.logsdb.datageneration.DataGeneratorTests - method: testDataGeneratorProducesValidMappingAndDocument - issue: https://github.com/elastic/elasticsearch/issues/112966 - class: org.elasticsearch.xpack.core.security.authz.RoleDescriptorTests method: testHasPrivilegesOtherThanIndex issue: https://github.com/elastic/elasticsearch/issues/113202 diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DataSourceRequest.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DataSourceRequest.java index ce843fc3e15ee..81e120511a40f 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DataSourceRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DataSourceRequest.java @@ -104,9 +104,12 @@ public DataSourceResponse.ObjectArrayGenerator accept(DataSourceHandler handler) } } - record LeafMappingParametersGenerator(String fieldName, FieldType fieldType, Set eligibleCopyToFields) - implements - DataSourceRequest { + record LeafMappingParametersGenerator( + String fieldName, + FieldType fieldType, + Set eligibleCopyToFields, + DynamicMapping dynamicMapping + ) implements DataSourceRequest { public DataSourceResponse.LeafMappingParametersGenerator accept(DataSourceHandler handler) { return handler.handle(this); } diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java index 86cf071ce8696..89850cd56bbd0 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java @@ -10,6 +10,7 @@ package org.elasticsearch.logsdb.datageneration.datasource; import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.logsdb.datageneration.fields.DynamicMapping; import org.elasticsearch.test.ESTestCase; import java.util.HashMap; @@ -48,7 +49,11 @@ private Supplier> keywordMapping( // We only add copy_to to keywords because we get into trouble with numeric fields that are copied to dynamic fields. // If first copied value is numeric, dynamic field is created with numeric field type and then copy of text values fail. // Actual value being copied does not influence the core logic of copy_to anyway. - if (ESTestCase.randomDouble() <= 0.05) { + // + // TODO + // We don't use copy_to on fields that are inside an object with dynamic: strict + // because we'll hit https://github.com/elastic/elasticsearch/issues/113049. + if (request.dynamicMapping() != DynamicMapping.FORBIDDEN && ESTestCase.randomDouble() <= 0.05) { var options = request.eligibleCopyToFields() .stream() .filter(f -> f.equals(request.fieldName()) == false) diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/GenericSubObjectFieldDataGenerator.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/GenericSubObjectFieldDataGenerator.java index c6d9d94d61892..ba03b2f91c53c 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/GenericSubObjectFieldDataGenerator.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/GenericSubObjectFieldDataGenerator.java @@ -62,7 +62,8 @@ List generateChildFields(DynamicMapping dynamicMapping) { new DataSourceRequest.LeafMappingParametersGenerator( fieldName, fieldTypeInfo.fieldType(), - context.getEligibleCopyToDestinations() + context.getEligibleCopyToDestinations(), + dynamicMapping ) ); var generator = fieldTypeInfo.fieldType() diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/PredefinedField.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/PredefinedField.java index 7b1bf4a77dc72..57e3ce3ce2a86 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/PredefinedField.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/fields/PredefinedField.java @@ -21,7 +21,7 @@ public interface PredefinedField { FieldDataGenerator generator(DataSource dataSource); - record WithType(String fieldName, FieldType fieldType) implements PredefinedField { + record WithType(String fieldName, FieldType fieldType, DynamicMapping dynamicMapping) implements PredefinedField { @Override public String name() { return fieldName; @@ -31,7 +31,7 @@ public String name() { public FieldDataGenerator generator(DataSource dataSource) { // copy_to currently not supported for predefined fields, use WithGenerator if needed var mappingParametersGenerator = dataSource.get( - new DataSourceRequest.LeafMappingParametersGenerator(fieldName, fieldType, Set.of()) + new DataSourceRequest.LeafMappingParametersGenerator(fieldName, fieldType, Set.of(), dynamicMapping) ); return fieldType().generator(fieldName, dataSource, mappingParametersGenerator); } From 01cb679858fbb86a71df79c9545463466a184944 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 20 Sep 2024 17:45:34 +0100 Subject: [PATCH 07/46] Make `CreateDataStreamClusterStateUpdateRequest` a record (#113282) No need to extend `ClusterStateUpdateRequest` here. --- .../MetadataCreateDataStreamService.java | 65 ++++++------------- .../MetadataCreateDataStreamServiceTests.java | 12 ++-- 2 files changed, 27 insertions(+), 50 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java index ef542419348fc..69f753233b418 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java @@ -21,7 +21,6 @@ import org.elasticsearch.cluster.AckedClusterStateUpdateTask; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.routing.allocation.allocator.AllocationActionListener; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; @@ -143,13 +142,19 @@ public ClusterState createDataStream( ); } - public static final class CreateDataStreamClusterStateUpdateRequest extends ClusterStateUpdateRequest< - CreateDataStreamClusterStateUpdateRequest> { - - private final boolean performReroute; - private final String name; - private final long startTime; - private final SystemDataStreamDescriptor descriptor; + public record CreateDataStreamClusterStateUpdateRequest( + String name, + long startTime, + @Nullable SystemDataStreamDescriptor systemDataStreamDescriptor, + TimeValue masterNodeTimeout, + TimeValue ackTimeout, + boolean performReroute + ) { + public CreateDataStreamClusterStateUpdateRequest { + Objects.requireNonNull(name); + Objects.requireNonNull(masterNodeTimeout); + Objects.requireNonNull(ackTimeout); + } public CreateDataStreamClusterStateUpdateRequest(String name) { this(name, System.currentTimeMillis(), null, TimeValue.ZERO, TimeValue.ZERO, true); @@ -159,42 +164,14 @@ public CreateDataStreamClusterStateUpdateRequest( String name, SystemDataStreamDescriptor systemDataStreamDescriptor, TimeValue masterNodeTimeout, - TimeValue timeout, + TimeValue ackTimeout, boolean performReroute ) { - this(name, System.currentTimeMillis(), systemDataStreamDescriptor, masterNodeTimeout, timeout, performReroute); - } - - public CreateDataStreamClusterStateUpdateRequest( - String name, - long startTime, - SystemDataStreamDescriptor systemDataStreamDescriptor, - TimeValue masterNodeTimeout, - TimeValue timeout, - boolean performReroute - ) { - this.name = name; - this.startTime = startTime; - this.descriptor = systemDataStreamDescriptor; - this.performReroute = performReroute; - masterNodeTimeout(masterNodeTimeout); - ackTimeout(timeout); + this(name, System.currentTimeMillis(), systemDataStreamDescriptor, masterNodeTimeout, ackTimeout, performReroute); } public boolean isSystem() { - return descriptor != null; - } - - public boolean performReroute() { - return performReroute; - } - - public SystemDataStreamDescriptor getSystemDataStreamDescriptor() { - return descriptor; - } - - long getStartTime() { - return startTime; + return systemDataStreamDescriptor != null; } } @@ -243,7 +220,7 @@ static ClusterState createDataStream( boolean initializeFailureStore ) throws Exception { String dataStreamName = request.name; - SystemDataStreamDescriptor systemDataStreamDescriptor = request.getSystemDataStreamDescriptor(); + SystemDataStreamDescriptor systemDataStreamDescriptor = request.systemDataStreamDescriptor(); boolean isSystemDataStreamName = metadataCreateIndexService.getSystemIndices().isSystemDataStream(request.name); assert (isSystemDataStreamName && systemDataStreamDescriptor != null) || (isSystemDataStreamName == false && systemDataStreamDescriptor == null) @@ -292,13 +269,13 @@ static ClusterState createDataStream( if (isSystem) { throw new IllegalArgumentException("Failure stores are not supported on system data streams"); } - String failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, initialGeneration, request.getStartTime()); + String failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, initialGeneration, request.startTime()); currentState = createFailureStoreIndex( metadataCreateIndexService, "initialize_data_stream", settings, currentState, - request.getStartTime(), + request.startTime(), dataStreamName, template, failureStoreIndexName, @@ -308,7 +285,7 @@ static ClusterState createDataStream( } if (writeIndex == null) { - String firstBackingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, initialGeneration, request.getStartTime()); + String firstBackingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, initialGeneration, request.startTime()); currentState = createBackingIndex( metadataCreateIndexService, currentState, @@ -397,7 +374,7 @@ private static ClusterState createBackingIndex( firstBackingIndexName ).dataStreamName(dataStreamName) .systemDataStreamDescriptor(systemDataStreamDescriptor) - .nameResolvedInstant(request.getStartTime()) + .nameResolvedInstant(request.startTime()) .performReroute(request.performReroute()) .setMatchingTemplate(template); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamServiceTests.java index 81c19946753db..bbcf1ca33a0c2 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamServiceTests.java @@ -244,8 +244,8 @@ public void testCreateDataStreamWithFailureStoreInitialized() throws Exception { ActionListener.noop(), true ); - var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.getStartTime()); - var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.getStartTime()); + var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.startTime()); + var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.startTime()); assertThat(newState.metadata().dataStreams().size(), equalTo(1)); assertThat(newState.metadata().dataStreams().get(dataStreamName).getName(), equalTo(dataStreamName)); assertThat(newState.metadata().dataStreams().get(dataStreamName).isSystem(), is(false)); @@ -284,8 +284,8 @@ public void testCreateDataStreamWithFailureStoreUninitialized() throws Exception ActionListener.noop(), false ); - var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.getStartTime()); - var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.getStartTime()); + var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.startTime()); + var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.startTime()); assertThat(newState.metadata().dataStreams().size(), equalTo(1)); assertThat(newState.metadata().dataStreams().get(dataStreamName).getName(), equalTo(dataStreamName)); assertThat(newState.metadata().dataStreams().get(dataStreamName).isSystem(), is(false)); @@ -321,8 +321,8 @@ public void testCreateDataStreamWithFailureStoreWithRefreshRate() throws Excepti ActionListener.noop(), true ); - var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.getStartTime()); - var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.getStartTime()); + var backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1, req.startTime()); + var failureStoreIndexName = DataStream.getDefaultFailureStoreName(dataStreamName, 1, req.startTime()); assertThat(newState.metadata().dataStreams().size(), equalTo(1)); assertThat(newState.metadata().dataStreams().get(dataStreamName).getName(), equalTo(dataStreamName)); assertThat(newState.metadata().index(backingIndexName), notNullValue()); From 67e32e5c823f984098f81ae3c96037ef46200d7b Mon Sep 17 00:00:00 2001 From: Patrick Doyle <810052+prdoyle@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:12:27 -0400 Subject: [PATCH 08/46] Initial trivial hello-world entitlements agent (#113112) * Initial hello-world entitlements agent * Respond to Ryan's comments * License header * Fix forbidden APIs setup * Rename EntitlementAgent * Automated refactor missed one * Automated rename really let me down here * Very serious test name * README files for the new modules * Use "tasks.named('jar')" Co-authored-by: Rene Groeschke * Use 'tasks.named('test')' Co-authored-by: Rene Groeschke * More deferral of gradle tasks Co-authored-by: Rene Groeschke * Even more deferral Co-authored-by: Rene Groeschke * FIx gradle syntax for javaagent arg --------- Co-authored-by: Rene Groeschke --- .../tools/entitlement-agent/README.md | 10 +++++ .../tools/entitlement-agent/build.gradle | 39 +++++++++++++++++++ .../src/main/java/module-info.java | 13 +++++++ .../entitlement/agent/EntitlementAgent.java | 21 ++++++++++ .../agent/EntitlementAgentTests.java | 29 ++++++++++++++ libs/entitlement-runtime/README.md | 14 +++++++ libs/entitlement-runtime/build.gradle | 24 ++++++++++++ .../src/main/java/module-info.java | 14 +++++++ .../runtime/api/EntitlementChecks.java | 22 +++++++++++ settings.gradle | 1 + 10 files changed, 187 insertions(+) create mode 100644 distribution/tools/entitlement-agent/README.md create mode 100644 distribution/tools/entitlement-agent/build.gradle create mode 100644 distribution/tools/entitlement-agent/src/main/java/module-info.java create mode 100644 distribution/tools/entitlement-agent/src/main/java/org/elasticsearch/entitlement/agent/EntitlementAgent.java create mode 100644 distribution/tools/entitlement-agent/src/test/java/org/elasticsearch/entitlement/agent/EntitlementAgentTests.java create mode 100644 libs/entitlement-runtime/README.md create mode 100644 libs/entitlement-runtime/build.gradle create mode 100644 libs/entitlement-runtime/src/main/java/module-info.java create mode 100644 libs/entitlement-runtime/src/main/java/org/elasticsearch/entitlement/runtime/api/EntitlementChecks.java diff --git a/distribution/tools/entitlement-agent/README.md b/distribution/tools/entitlement-agent/README.md new file mode 100644 index 0000000000000..ff40651706a7f --- /dev/null +++ b/distribution/tools/entitlement-agent/README.md @@ -0,0 +1,10 @@ +### Entitlement Agent + +This is a java agent that instruments sensitive class library methods with calls into the `entitlement-runtime` module to check for permissions granted under the _entitlements_ system. + +The entitlements system provides an alternative to the legacy `SecurityManager` system, which is deprecated for removal. +With this agent, the Elasticsearch server can retain some control over which class library methods can be invoked by which callers. + +This module is responsible for inserting the appropriate bytecode to achieve enforcement of the rules governed by the `entitlement-runtime` module. + +It is not responsible for permission granting or checking logic. That responsibility lies with `entitlement-runtime`. diff --git a/distribution/tools/entitlement-agent/build.gradle b/distribution/tools/entitlement-agent/build.gradle new file mode 100644 index 0000000000000..56e2ffac53fd7 --- /dev/null +++ b/distribution/tools/entitlement-agent/build.gradle @@ -0,0 +1,39 @@ +/* + * 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". + */ + +apply plugin: 'elasticsearch.build' + +configurations { + entitlementRuntime +} + +dependencies { + entitlementRuntime project(":libs:elasticsearch-entitlement-runtime") + implementation project(":libs:elasticsearch-entitlement-runtime") + testImplementation project(":test:framework") +} + +tasks.named('test').configure { + dependsOn('jar') + jvmArgs "-javaagent:${ tasks.named('jar').flatMap{ it.archiveFile }.get()}" +} + +tasks.named('jar').configure { + manifest { + attributes( + 'Premain-Class': 'org.elasticsearch.entitlement.agent.EntitlementAgent' + , 'Can-Retransform-Classes': 'true' + ) + } +} + +tasks.named('forbiddenApisMain').configure { + replaceSignatureFiles 'jdk-signatures' +} + diff --git a/distribution/tools/entitlement-agent/src/main/java/module-info.java b/distribution/tools/entitlement-agent/src/main/java/module-info.java new file mode 100644 index 0000000000000..df6fc154fc67f --- /dev/null +++ b/distribution/tools/entitlement-agent/src/main/java/module-info.java @@ -0,0 +1,13 @@ +/* + * 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". + */ + +module org.elasticsearch.entitlement.agent { + requires java.instrument; + requires org.elasticsearch.entitlement.runtime; +} diff --git a/distribution/tools/entitlement-agent/src/main/java/org/elasticsearch/entitlement/agent/EntitlementAgent.java b/distribution/tools/entitlement-agent/src/main/java/org/elasticsearch/entitlement/agent/EntitlementAgent.java new file mode 100644 index 0000000000000..b843e42f4a03e --- /dev/null +++ b/distribution/tools/entitlement-agent/src/main/java/org/elasticsearch/entitlement/agent/EntitlementAgent.java @@ -0,0 +1,21 @@ +/* + * 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.entitlement.agent; + +import org.elasticsearch.entitlement.runtime.api.EntitlementChecks; + +import java.lang.instrument.Instrumentation; + +public class EntitlementAgent { + + public static void premain(String agentArgs, Instrumentation inst) throws Exception { + EntitlementChecks.setAgentBooted(); + } +} diff --git a/distribution/tools/entitlement-agent/src/test/java/org/elasticsearch/entitlement/agent/EntitlementAgentTests.java b/distribution/tools/entitlement-agent/src/test/java/org/elasticsearch/entitlement/agent/EntitlementAgentTests.java new file mode 100644 index 0000000000000..3927465570c98 --- /dev/null +++ b/distribution/tools/entitlement-agent/src/test/java/org/elasticsearch/entitlement/agent/EntitlementAgentTests.java @@ -0,0 +1,29 @@ +/* + * 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.entitlement.agent; + +import org.elasticsearch.entitlement.runtime.api.EntitlementChecks; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.ESTestCase.WithoutSecurityManager; + +/** + * This is an end-to-end test that runs with the javaagent installed. + * It should exhaustively test every instrumented method to make sure it passes with the entitlement + * and fails without it. + * See {@code build.gradle} for how we set the command line arguments for this test. + */ +@WithoutSecurityManager +public class EntitlementAgentTests extends ESTestCase { + + public void testAgentBooted() { + assertTrue(EntitlementChecks.isAgentBooted()); + } + +} diff --git a/libs/entitlement-runtime/README.md b/libs/entitlement-runtime/README.md new file mode 100644 index 0000000000000..49cbc873c9de5 --- /dev/null +++ b/libs/entitlement-runtime/README.md @@ -0,0 +1,14 @@ +### Entitlement runtime + +This module implements mechanisms to grant and check permissions under the _entitlements_ system. + +The entitlements system provides an alternative to the legacy `SecurityManager` system, which is deprecated for removal. +The `entitlement-agent` tool instruments sensitive class library methods with calls to this module, in order to enforce the controls. + +This module is responsible for: +- Defining which class library methods are sensitive +- Defining what permissions should be checked for each sensitive method +- Implementing the permission checks +- Offering a "grant" API to grant permissions + +It is not responsible for anything to do with bytecode instrumentation; that responsibility lies with `entitlement-agent`. diff --git a/libs/entitlement-runtime/build.gradle b/libs/entitlement-runtime/build.gradle new file mode 100644 index 0000000000000..a552dd7d5ba47 --- /dev/null +++ b/libs/entitlement-runtime/build.gradle @@ -0,0 +1,24 @@ +/* + * 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". + */ +apply plugin: 'elasticsearch.build' +apply plugin: 'elasticsearch.publish' + +dependencies { + compileOnly project(':libs:elasticsearch-core') + + testImplementation project(":test:framework") +} + +tasks.named('forbiddenApisMain').configure { + replaceSignatureFiles 'jdk-signatures' +} + +tasks.named('forbiddenApisMain').configure { + replaceSignatureFiles 'jdk-signatures' +} diff --git a/libs/entitlement-runtime/src/main/java/module-info.java b/libs/entitlement-runtime/src/main/java/module-info.java new file mode 100644 index 0000000000000..13849f0658d72 --- /dev/null +++ b/libs/entitlement-runtime/src/main/java/module-info.java @@ -0,0 +1,14 @@ +/* + * 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". + */ + +module org.elasticsearch.entitlement.runtime { + requires org.elasticsearch.base; + + exports org.elasticsearch.entitlement.runtime.api to org.elasticsearch.entitlement.agent; +} diff --git a/libs/entitlement-runtime/src/main/java/org/elasticsearch/entitlement/runtime/api/EntitlementChecks.java b/libs/entitlement-runtime/src/main/java/org/elasticsearch/entitlement/runtime/api/EntitlementChecks.java new file mode 100644 index 0000000000000..c06e1e5b1f858 --- /dev/null +++ b/libs/entitlement-runtime/src/main/java/org/elasticsearch/entitlement/runtime/api/EntitlementChecks.java @@ -0,0 +1,22 @@ +/* + * 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.entitlement.runtime.api; + +public class EntitlementChecks { + static boolean isAgentBooted = false; + + public static void setAgentBooted() { + isAgentBooted = true; + } + + public static boolean isAgentBooted() { + return isAgentBooted; + } +} diff --git a/settings.gradle b/settings.gradle index ab87861105156..2926a9a303375 100644 --- a/settings.gradle +++ b/settings.gradle @@ -91,6 +91,7 @@ List projects = [ 'distribution:tools:keystore-cli', 'distribution:tools:geoip-cli', 'distribution:tools:ansi-console', + 'distribution:tools:entitlement-agent', 'server', 'test:framework', 'test:fixtures:azure-fixture', From af896313df79032dfccd3b363f34cf4914bc2917 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 03:12:33 +1000 Subject: [PATCH 09/46] Mute org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT test {date.EvalDateFormatString SYNC} #113293 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index a350c8209ae5a..9bb1f22fde215 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -274,6 +274,9 @@ tests: - class: org.elasticsearch.xpack.security.authz.SecurityScrollTests method: testSearchAndClearScroll issue: https://github.com/elastic/elasticsearch/issues/113285 +- class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT + method: test {date.EvalDateFormatString SYNC} + issue: https://github.com/elastic/elasticsearch/issues/113293 # Examples: # From 36874b0da6605fa8ed43c25f88dc471590df7303 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 03:12:46 +1000 Subject: [PATCH 10/46] Mute org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT test {date.EvalDateFormat} #113294 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 9bb1f22fde215..c5d434e36c8fc 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -277,6 +277,9 @@ tests: - class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT method: test {date.EvalDateFormatString SYNC} issue: https://github.com/elastic/elasticsearch/issues/113293 +- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT + method: test {date.EvalDateFormat} + issue: https://github.com/elastic/elasticsearch/issues/113294 # Examples: # From d34ec20463b1c1f16f61ee8179ac4ea75ff5bdd2 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 03:12:57 +1000 Subject: [PATCH 11/46] Mute org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT test {date.DocsDateFormat} #113295 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index c5d434e36c8fc..f6110d0bf8825 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -280,6 +280,9 @@ tests: - class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT method: test {date.EvalDateFormat} issue: https://github.com/elastic/elasticsearch/issues/113294 +- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT + method: test {date.DocsDateFormat} + issue: https://github.com/elastic/elasticsearch/issues/113295 # Examples: # From 417f308528e17c2f4a4a2ee2aa26b15b31f9d895 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 03:13:07 +1000 Subject: [PATCH 12/46] Mute org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT test {stats.DocsStatsGroupByMultipleValues} #113296 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index f6110d0bf8825..2fbff4abd5c06 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -283,6 +283,9 @@ tests: - class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT method: test {date.DocsDateFormat} issue: https://github.com/elastic/elasticsearch/issues/113295 +- class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT + method: test {stats.DocsStatsGroupByMultipleValues} + issue: https://github.com/elastic/elasticsearch/issues/113296 # Examples: # From 079b4c355dab2c7acf8f8118efc30f4314897a32 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 04:06:27 +1000 Subject: [PATCH 13/46] Mute org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT #113298 --- muted-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 2fbff4abd5c06..6b7e30bb9c0e3 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -286,6 +286,8 @@ tests: - class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT method: test {stats.DocsStatsGroupByMultipleValues} issue: https://github.com/elastic/elasticsearch/issues/113296 +- class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT + issue: https://github.com/elastic/elasticsearch/issues/113298 # Examples: # From b9855b8e4e0f29069a50f4e41a4643beb532c75f Mon Sep 17 00:00:00 2001 From: Oleksandr Kolomiiets Date: Fri, 20 Sep 2024 12:08:55 -0700 Subject: [PATCH 14/46] Correctly identify parent of copy_to destination field for synthetic source purposes (#113153) --- .../indices.create/20_synthetic_source.yml | 410 +++++++++++++++++- .../index/mapper/DocumentParserContext.java | 19 +- .../mapper/IgnoredSourceFieldMapper.java | 44 +- .../index/mapper/MapperFeatures.java | 3 +- .../index/mapper/ObjectMapper.java | 159 +++++-- .../index/mapper/SourceFieldMapper.java | 3 + .../index/mapper/SourceLoader.java | 13 + .../index/mapper/XContentDataHelper.java | 28 +- .../mapper/IgnoredSourceFieldMapperTests.java | 31 ++ .../index/mapper/ObjectMapperTests.java | 36 ++ .../index/mapper/XContentDataHelperTests.java | 58 --- 11 files changed, 657 insertions(+), 147 deletions(-) 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 8a27eb3efd219..937c5f19ae5aa 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 @@ -1269,8 +1269,8 @@ synthetic_source with copy_to and ignored values: refresh: true body: name: "B" - k: ["5", "6"] - long: ["7", "8"] + k: ["55", "66"] + long: ["77", "88"] - do: search: @@ -1289,10 +1289,9 @@ synthetic_source with copy_to and ignored values: - match: hits.hits.1._source: name: "B" - k: ["5", "6"] - long: ["7", "8"] - - match: { hits.hits.1.fields.copy: ["5", "6", "7", "8"] } - + k: ["55", "66"] + long: ["77", "88"] + - match: { hits.hits.1.fields.copy: ["55", "66", "77", "88"] } --- synthetic_source with copy_to field having values in source: @@ -1553,3 +1552,402 @@ synthetic_source with copy_to and invalid values for copy: - match: { error.type: "document_parsing_exception" } - contains: { error.reason: "Copy-to currently only works for value-type fields" } + +--- +synthetic_source with copy_to pointing inside object: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + name: + type: keyword + my_values: + properties: + k: + type: keyword + ignore_above: 1 + copy_to: c.copy + long: + type: long + copy_to: c.copy + c: + properties: + copy: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + name: "A" + my_values: + k: "hello" + long: 100 + + - do: + index: + index: test + id: 2 + refresh: true + body: + name: "B" + my_values: + k: ["55", "66"] + long: [77, 88] + + - do: + index: + index: test + id: 3 + refresh: true + body: + name: "C" + my_values: + k: "hello" + long: 100 + c: + copy: "zap" + + - do: + search: + index: test + sort: name + body: + docvalue_fields: [ "c.copy" ] + + - match: + hits.hits.0._source: + name: "A" + my_values: + k: "hello" + long: 100 + - match: + hits.hits.0.fields: + c.copy: [ "100", "hello" ] + + - match: + hits.hits.1._source: + name: "B" + my_values: + k: ["55", "66"] + long: [77, 88] + - match: + hits.hits.1.fields: + c.copy: ["55", "66", "77", "88"] + + - match: + hits.hits.2._source: + name: "C" + my_values: + k: "hello" + long: 100 + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy: [ "100", "hello", "zap" ] + +--- +synthetic_source with copy_to pointing to ambiguous field: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing to ambiguous field and subobjects false: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: false + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing to ambiguous field and subobjects auto: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: auto + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing at dynamic field: + - requires: + test_runner_features: contains + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: c.copy + c: + properties: + f: + type: float + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hello" + + - do: + index: + index: test + id: 2 + refresh: true + body: + k: ["55", "66"] + + - do: + index: + index: test + id: 3 + refresh: true + body: + k: "hello" + c: + copy: "zap" + + - do: + search: + index: test + body: + docvalue_fields: [ "c.copy.keyword" ] + + - match: + hits.hits.0._source: + k: "hello" + - match: + hits.hits.0.fields: + c.copy.keyword: [ "hello" ] + + - match: + hits.hits.1._source: + k: ["55", "66"] + - match: + hits.hits.1.fields: + c.copy.keyword: [ "55", "66" ] + + - match: + hits.hits.2._source: + k: "hello" + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy.keyword: [ "hello", "zap" ] + +--- +synthetic_source with copy_to pointing inside dynamic object: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: c.copy + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hello" + + - do: + index: + index: test + id: 2 + refresh: true + body: + k: ["55", "66"] + + - do: + index: + index: test + id: 3 + refresh: true + body: + k: "hello" + c: + copy: "zap" + + - do: + search: + index: test + body: + docvalue_fields: [ "c.copy.keyword" ] + + - match: + hits.hits.0._source: + k: "hello" + - match: + hits.hits.0.fields: + c.copy.keyword: [ "hello" ] + + - match: + hits.hits.1._source: + k: ["55", "66"] + - match: + hits.hits.1.fields: + c.copy.keyword: [ "55", "66" ] + + - match: + hits.hits.2._source: + k: "hello" + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy.keyword: [ "hello", "zap" ] + 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 4d1b68214eddb..c2970d8716147 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; + /** * Fields that are copied from values of other fields via copy_to. * This per-document state is needed since it is possible @@ -453,26 +454,16 @@ public boolean isFieldAppliedFromTemplate(String name) { public void markFieldAsCopyTo(String fieldName) { copyToFields.add(fieldName); - if (mappingLookup.isSourceSynthetic() && indexSettings().getSkipIgnoredSourceWrite() == false) { - /* - Mark this field as containing copied data meaning it should not be present - in synthetic _source (to be consistent with stored _source). - Ignored source values take precedence over standard synthetic source implementation - so by adding this nothing entry we "disable" field in synthetic source. - Otherwise, it would be constructed f.e. from doc_values which leads to duplicate values - in copied field after reindexing. - - Note that this applies to fields that are copied from fields using ignored source themselves - and therefore we don't check for canAddIgnoredField(). - */ - ignoredFieldValues.add(IgnoredSourceFieldMapper.NameValue.fromContext(this, fieldName, XContentDataHelper.nothing())); - } } public boolean isCopyToDestinationField(String name) { return copyToFields.contains(name); } + public Set getCopyToFields() { + return copyToFields; + } + /** * Add a new mapper dynamically created while parsing. * diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java index 7b926b091b3a2..d57edb757ba10 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java @@ -26,9 +26,8 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; -import java.util.Comparator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Stream; @@ -113,6 +112,10 @@ NameValue cloneWithValue(BytesRef value) { assert value() == null; return new NameValue(name, parentOffset, value, doc); } + + boolean hasValue() { + return XContentDataHelper.isDataPresent(value); + } } static final class IgnoredValuesFieldMapperType extends StringFieldType { @@ -147,11 +150,38 @@ protected String contentType() { @Override public void postParse(DocumentParserContext context) { // Ignored values are only expected in synthetic mode. - assert context.getIgnoredFieldValues().isEmpty() || context.mappingLookup().isSourceSynthetic(); - List ignoredFieldValues = new ArrayList<>(context.getIgnoredFieldValues()); - // ensure consistent ordering when retrieving synthetic source - Collections.sort(ignoredFieldValues, Comparator.comparing(NameValue::name)); - for (NameValue nameValue : ignoredFieldValues) { + if (context.mappingLookup().isSourceSynthetic() == false) { + assert context.getIgnoredFieldValues().isEmpty(); + return; + } + + Collection ignoredValuesToWrite = context.getIgnoredFieldValues(); + if (context.getCopyToFields().isEmpty() == false && indexSettings.getSkipIgnoredSourceWrite() == false) { + /* + Mark fields as containing copied data meaning they should not be present + in synthetic _source (to be consistent with stored _source). + Ignored source values take precedence over standard synthetic source implementation + so by adding the `XContentDataHelper.voidValue()` entry we disable the field in synthetic source. + Otherwise, it would be constructed f.e. from doc_values which leads to duplicate values + in copied field after reindexing. + */ + var mutableList = new ArrayList<>(ignoredValuesToWrite); + for (String copyToField : context.getCopyToFields()) { + ObjectMapper parent = context.parent().findParentMapper(copyToField); + if (parent == null) { + // There are scenarios when this can happen: + // 1. all values of the field that is the source of copy_to are null + // 2. copy_to points at a field inside a disabled object + // 3. copy_to points at dynamic field which is not yet applied to mapping, we will process it properly on re-parse. + continue; + } + int offset = parent.isRoot() ? 0 : parent.fullPath().length() + 1; + mutableList.add(new IgnoredSourceFieldMapper.NameValue(copyToField, offset, XContentDataHelper.voidValue(), context.doc())); + } + ignoredValuesToWrite = mutableList; + } + + for (NameValue nameValue : ignoredValuesToWrite) { nameValue.doc().add(new StoredField(NAME, encode(nameValue))); } } 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 0f224c98241db..d18c3283ef909 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java @@ -40,7 +40,8 @@ public Set getFeatures() { Mapper.SYNTHETIC_SOURCE_KEEP_FEATURE, SourceFieldMapper.SYNTHETIC_SOURCE_WITH_COPY_TO_AND_DOC_VALUES_FALSE_SUPPORT, SourceFieldMapper.SYNTHETIC_SOURCE_COPY_TO_FIX, - FlattenedFieldMapper.IGNORE_ABOVE_SUPPORT + FlattenedFieldMapper.IGNORE_ABOVE_SUPPORT, + SourceFieldMapper.SYNTHETIC_SOURCE_COPY_TO_INSIDE_OBJECTS_FIX ); } } 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 e4ce38d6cec0b..f9c854749e885 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -808,6 +808,42 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep } + ObjectMapper findParentMapper(String leafFieldPath) { + var pathComponents = leafFieldPath.split("\\."); + int startPathComponent = 0; + + ObjectMapper current = this; + String pathInCurrent = leafFieldPath; + + while (current != null) { + if (current.mappers.containsKey(pathInCurrent)) { + return current; + } + + // Go one level down if possible + var parent = current; + current = null; + + var childMapperName = new StringBuilder(); + for (int i = startPathComponent; i < pathComponents.length - 1; i++) { + if (childMapperName.isEmpty() == false) { + childMapperName.append("."); + } + childMapperName.append(pathComponents[i]); + + var childMapper = parent.mappers.get(childMapperName.toString()); + if (childMapper instanceof ObjectMapper objectMapper) { + current = objectMapper; + startPathComponent = i + 1; + pathInCurrent = pathInCurrent.substring(childMapperName.length() + 1); + break; + } + } + } + + return null; + } + protected SourceLoader.SyntheticFieldLoader syntheticFieldLoader(Stream mappers, boolean isFragment) { var fields = mappers.sorted(Comparator.comparing(Mapper::fullPath)) .map(Mapper::syntheticFieldLoader) @@ -828,10 +864,18 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() { private class SyntheticSourceFieldLoader implements SourceLoader.SyntheticFieldLoader { private final List fields; private final boolean isFragment; + private boolean storedFieldLoadersHaveValues; private boolean docValuesLoadersHaveValues; private boolean ignoredValuesPresent; private List ignoredValues; + // If this loader has anything to write. + // In special cases this can be false even if doc values loaders or stored field loaders + // have values. + // F.e. objects that only contain fields that are destinations of copy_to. + private boolean writersHaveValues; + // Use an ordered map between field names and writers to order writing by field name. + private TreeMap currentWriters; private SyntheticSourceFieldLoader(List fields, boolean isFragment) { this.fields = fields; @@ -882,9 +926,55 @@ public boolean advanceToDoc(int docId) throws IOException { } } + @Override + public void prepare() { + if ((storedFieldLoadersHaveValues || docValuesLoadersHaveValues || ignoredValuesPresent) == false) { + writersHaveValues = false; + return; + } + + for (var loader : fields) { + // Currently this logic is only relevant for object loaders. + if (loader instanceof ObjectMapper.SyntheticSourceFieldLoader objectSyntheticFieldLoader) { + objectSyntheticFieldLoader.prepare(); + } + } + + currentWriters = new TreeMap<>(); + + if (ignoredValues != null && ignoredValues.isEmpty() == false) { + for (IgnoredSourceFieldMapper.NameValue value : ignoredValues) { + if (value.hasValue()) { + writersHaveValues |= true; + } + + var existing = currentWriters.get(value.name()); + if (existing == null) { + currentWriters.put(value.name(), new FieldWriter.IgnoredSource(value)); + } else if (existing instanceof FieldWriter.IgnoredSource isw) { + isw.mergeWith(value); + } + } + } + + for (SourceLoader.SyntheticFieldLoader field : fields) { + if (field.hasValue()) { + if (currentWriters.containsKey(field.fieldName()) == false) { + writersHaveValues |= true; + currentWriters.put(field.fieldName(), new FieldWriter.FieldLoader(field)); + } else { + // Skip if the field source is stored separately, to avoid double-printing. + // Make sure to reset the state of loader so that values stored inside will not + // be used after this document is finished. + field.reset(); + } + } + } + } + @Override public boolean hasValue() { - return storedFieldLoadersHaveValues || docValuesLoadersHaveValues || ignoredValuesPresent; + return writersHaveValues; } @Override @@ -892,12 +982,13 @@ public void write(XContentBuilder b) throws IOException { if (hasValue() == false) { return; } + if (isRoot() && isEnabled() == false) { // If the root object mapper is disabled, it is expected to contain // the source encapsulated within a single ignored source value. assert ignoredValues.size() == 1 : ignoredValues.size(); XContentDataHelper.decodeAndWrite(b, ignoredValues.get(0).value()); - ignoredValues = null; + softReset(); return; } @@ -907,41 +998,12 @@ public void write(XContentBuilder b) throws IOException { b.startObject(leafName()); } - if (ignoredValues != null && ignoredValues.isEmpty() == false) { - // Use an ordered map between field names and writer functions, to order writing by field name. - Map orderedFields = new TreeMap<>(); - for (IgnoredSourceFieldMapper.NameValue value : ignoredValues) { - var existing = orderedFields.get(value.name()); - if (existing == null) { - orderedFields.put(value.name(), new FieldWriter.IgnoredSource(value)); - } else if (existing instanceof FieldWriter.IgnoredSource isw) { - isw.mergeWith(value); - } - } - for (SourceLoader.SyntheticFieldLoader field : fields) { - if (field.hasValue()) { - if (orderedFields.containsKey(field.fieldName()) == false) { - orderedFields.put(field.fieldName(), new FieldWriter.FieldLoader(field)); - } else { - // Skip if the field source is stored separately, to avoid double-printing. - // Make sure to reset the state of loader so that values stored inside will not - // be used after this document is finished. - field.reset(); - } - } - } - - for (var writer : orderedFields.values()) { + for (var writer : currentWriters.values()) { + if (writer.hasValue()) { writer.writeTo(b); } - ignoredValues = null; - } else { - for (SourceLoader.SyntheticFieldLoader field : fields) { - if (field.hasValue()) { - field.write(b); - } - } } + b.endObject(); softReset(); } @@ -957,6 +1019,8 @@ private void softReset() { storedFieldLoadersHaveValues = false; docValuesLoadersHaveValues = false; ignoredValuesPresent = false; + ignoredValues = null; + writersHaveValues = false; } @Override @@ -986,34 +1050,49 @@ public String fieldName() { interface FieldWriter { void writeTo(XContentBuilder builder) throws IOException; + boolean hasValue(); + record FieldLoader(SourceLoader.SyntheticFieldLoader loader) implements FieldWriter { @Override public void writeTo(XContentBuilder builder) throws IOException { loader.write(builder); } + + @Override + public boolean hasValue() { + return loader.hasValue(); + } } class IgnoredSource implements FieldWriter { private final String fieldName; private final String leafName; - private final List values; + private final List encodedValues; IgnoredSource(IgnoredSourceFieldMapper.NameValue initialValue) { this.fieldName = initialValue.name(); this.leafName = initialValue.getFieldName(); - this.values = new ArrayList<>(); - this.values.add(initialValue.value()); + this.encodedValues = new ArrayList<>(); + if (initialValue.hasValue()) { + this.encodedValues.add(initialValue.value()); + } } @Override public void writeTo(XContentBuilder builder) throws IOException { - XContentDataHelper.writeMerged(builder, leafName, values); + XContentDataHelper.writeMerged(builder, leafName, encodedValues); + } + + @Override + public boolean hasValue() { + return encodedValues.isEmpty() == false; } public FieldWriter mergeWith(IgnoredSourceFieldMapper.NameValue nameValue) { assert Objects.equals(nameValue.name(), fieldName) : "IgnoredSource is merged with wrong field data"; - - values.add(nameValue.value()); + if (nameValue.hasValue()) { + encodedValues.add(nameValue.value()); + } return this; } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index 3318595ed7129..118cdbffc5db9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -48,6 +48,9 @@ public class SourceFieldMapper extends MetadataFieldMapper { "mapper.source.synthetic_source_with_copy_to_and_doc_values_false" ); public static final NodeFeature SYNTHETIC_SOURCE_COPY_TO_FIX = new NodeFeature("mapper.source.synthetic_source_copy_to_fix"); + public static final NodeFeature SYNTHETIC_SOURCE_COPY_TO_INSIDE_OBJECTS_FIX = new NodeFeature( + "mapper.source.synthetic_source_copy_to_inside_objects_fix" + ); public static final String NAME = "_source"; public static final String RECOVERY_SOURCE_NAME = "_recovery_source"; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java index baff3835d104b..ec255a53e7c5a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java @@ -209,6 +209,9 @@ public void write(LeafStoredFieldLoader storedFieldLoader, int docId, XContentBu if (docValuesLoader != null) { docValuesLoader.advanceToDoc(docId); } + + loader.prepare(); + // TODO accept a requested xcontent type if (loader.hasValue()) { loader.write(b); @@ -299,6 +302,16 @@ public String fieldName() { */ DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException; + /** + Perform any preprocessing needed before producing synthetic source + and deduce whether this mapper (and its children, if any) have values to write. + The expectation is for this method to be called before {@link SyntheticFieldLoader#hasValue()} + and {@link SyntheticFieldLoader#write(XContentBuilder)} are used. + */ + default void prepare() { + // Noop + } + /** * Has this field loaded any values for this document? */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java b/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java index 354f0ec92b0fa..8bacaf8505f91 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java @@ -72,7 +72,7 @@ public static BytesRef encodeXContentBuilder(XContentBuilder builder) throws IOE } /** - * Returns a special encoded value that signals that values of this field + * Returns a special encoded value that signals that this field * should not be present in synthetic source. * * An example is a field that has values copied to it using copy_to. @@ -80,7 +80,7 @@ public static BytesRef encodeXContentBuilder(XContentBuilder builder) throws IOE * synthetic _source same as it wouldn't be present in stored source. * @return */ - public static BytesRef nothing() { + public static BytesRef voidValue() { return new BytesRef(new byte[] { VOID_ENCODING }); } @@ -112,41 +112,27 @@ static void decodeAndWrite(XContentBuilder b, BytesRef r) throws IOException { /** * Writes encoded values to provided builder. If there are multiple values they are merged into * a single resulting array. + * + * Note that this method assumes all encoded parts have values that need to be written (are not VOID encoded). * @param b destination * @param fieldName name of the field that is written * @param encodedParts subset of field data encoded using methods of this class. Can contain arrays which will be flattened. * @throws IOException */ static void writeMerged(XContentBuilder b, String fieldName, List encodedParts) throws IOException { - var partsWithData = 0; - for (BytesRef encodedPart : encodedParts) { - if (isDataPresent(encodedPart)) { - partsWithData++; - } - } - - if (partsWithData == 0) { + if (encodedParts.isEmpty()) { return; } - if (partsWithData == 1) { + if (encodedParts.size() == 1) { b.field(fieldName); - for (BytesRef encodedPart : encodedParts) { - if (isDataPresent(encodedPart)) { - XContentDataHelper.decodeAndWrite(b, encodedPart); - } - } - + XContentDataHelper.decodeAndWrite(b, encodedParts.get(0)); return; } b.startArray(fieldName); for (var encodedValue : encodedParts) { - if (isDataPresent(encodedValue) == false) { - continue; - } - Optional encodedXContentType = switch ((char) encodedValue.bytes[encodedValue.offset]) { case CBOR_OBJECT_ENCODING, JSON_OBJECT_ENCODING, YAML_OBJECT_ENCODING, SMILE_OBJECT_ENCODING -> Optional.of( getXContentType(encodedValue) 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 0e97d2b3b46ae..eaa7bf6528203 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java @@ -1518,6 +1518,37 @@ public void testStoredNestedSubObjectWithNameOverlappingParentName() throws IOEx {"path":{"at":{"foo":"A"}}}""", syntheticSource); } + public void testCopyToLogicInsideObject() throws IOException { + DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> { + b.startObject("path"); + b.startObject("properties"); + { + b.startObject("at").field("type", "keyword").field("copy_to", "copy_top.copy").endObject(); + } + b.endObject(); + b.endObject(); + b.startObject("copy_top"); + b.startObject("properties"); + { + b.startObject("copy").field("type", "keyword").endObject(); + } + b.endObject(); + b.endObject(); + })).documentMapper(); + + CheckedConsumer document = b -> { + b.startObject("path"); + b.field("at", "A"); + b.endObject(); + }; + + var doc = documentMapper.parse(source(document)); + assertNotNull(doc.docs().get(0).getField("copy_top.copy")); + + var syntheticSource = syntheticSource(documentMapper, document); + assertEquals("{\"path\":{\"at\":\"A\"}}", 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 8ba57824cf434..3312c94e8a0e1 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java @@ -811,4 +811,40 @@ public void testFlattenExplicitSubobjectsTrue() { exception.getMessage() ); } + + public void testFindParentMapper() { + MapperBuilderContext rootContext = MapperBuilderContext.root(false, false); + + var rootBuilder = new RootObjectMapper.Builder("_doc", Optional.empty()); + rootBuilder.add(new KeywordFieldMapper.Builder("keyword", IndexVersion.current())); + + var child = new ObjectMapper.Builder("child", Optional.empty()); + child.add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())); + child.add(new KeywordFieldMapper.Builder("keyword.with.dot", IndexVersion.current())); + var secondLevelChild = new ObjectMapper.Builder("child2", Optional.empty()); + secondLevelChild.add(new KeywordFieldMapper.Builder("keyword22", IndexVersion.current())); + child.add(secondLevelChild); + rootBuilder.add(child); + + var childWithDot = new ObjectMapper.Builder("childwith.dot", Optional.empty()); + childWithDot.add(new KeywordFieldMapper.Builder("keyword3", IndexVersion.current())); + childWithDot.add(new KeywordFieldMapper.Builder("keyword4.with.dot", IndexVersion.current())); + rootBuilder.add(childWithDot); + + RootObjectMapper root = rootBuilder.build(rootContext); + + assertEquals("_doc", root.findParentMapper("keyword").fullPath()); + assertNull(root.findParentMapper("aa")); + + assertEquals("child", root.findParentMapper("child.keyword2").fullPath()); + assertEquals("child", root.findParentMapper("child.keyword.with.dot").fullPath()); + assertNull(root.findParentMapper("child.long")); + assertNull(root.findParentMapper("child.long.hello")); + assertEquals("child.child2", root.findParentMapper("child.child2.keyword22").fullPath()); + + assertEquals("childwith.dot", root.findParentMapper("childwith.dot.keyword3").fullPath()); + assertEquals("childwith.dot", root.findParentMapper("childwith.dot.keyword4.with.dot").fullPath()); + assertNull(root.findParentMapper("childwith.dot.long")); + assertNull(root.findParentMapper("childwith.dot.long.hello")); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java index 4de02178beec1..f4e114da1fa51 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java @@ -240,64 +240,6 @@ private void testWriteMergedWithMixedValues(Object value, List multipleV assertEquals(expected, map.get("foo")); } - public void testWriteMergedWithVoidValue() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - XContentDataHelper.writeMerged(destination, "field", List.of(XContentDataHelper.nothing())); - - destination.endObject(); - - assertEquals("{}", Strings.toString(destination)); - } - - public void testWriteMergedWithMultipleVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.nothing(), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{}", Strings.toString(destination)); - } - - public void testWriteMergedWithMixedVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - var value = XContentFactory.contentBuilder(XContentType.JSON).value(34); - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.encodeXContentBuilder(value), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{\"field\":34}", Strings.toString(destination)); - } - - public void testWriteMergedWithArraysAndVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - var value = XContentFactory.contentBuilder(XContentType.JSON).value(List.of(3, 4)); - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.encodeXContentBuilder(value), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{\"field\":[3,4]}", Strings.toString(destination)); - } - private Map executeWriteMergedOnRepeated(Object value) throws IOException { return executeWriteMergedOnTwoEncodedValues(value, value); } From c5caf84e2de3fd5cf474a249a2ed939afd24e204 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 20 Sep 2024 13:32:45 -0600 Subject: [PATCH 15/46] Move raw path into HttpPreRequest (#113231) Currently, the raw path is only available from the RestRequest. This makes the logic to determine if a handler supports streaming more challenging to evaluate. This commit moves the raw path into pre request to allow easier streaming support logic. --- .../http/netty4/Netty4HttpRequest.java | 8 ++++++++ .../netty4/Netty4HttpServerTransport.java | 5 +---- .../http/IncrementalBulkRestIT.java | 6 ++++++ .../elasticsearch/http/HttpPreRequest.java | 18 +++++++++++++++++ .../org/elasticsearch/rest/RestRequest.java | 20 +++++-------------- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequest.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequest.java index b04da46a2d7d7..a1aa211814520 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequest.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequest.java @@ -17,6 +17,7 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.QueryStringDecoder; import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http.cookie.ServerCookieDecoder; import io.netty.handler.codec.http.cookie.ServerCookieEncoder; @@ -48,6 +49,7 @@ public class Netty4HttpRequest implements HttpRequest { private final Exception inboundException; private final boolean pooled; private final int sequence; + private final QueryStringDecoder queryStringDecoder; Netty4HttpRequest(int sequence, io.netty.handler.codec.http.HttpRequest request, Netty4HttpRequestBodyStream contentStream) { this( @@ -94,6 +96,7 @@ private Netty4HttpRequest( this.pooled = pooled; this.released = released; this.inboundException = inboundException; + this.queryStringDecoder = new QueryStringDecoder(request.uri()); } @Override @@ -106,6 +109,11 @@ public String uri() { return request.uri(); } + @Override + public String rawPath() { + return queryStringDecoder.rawPath(); + } + @Override public HttpBody body() { assert released.get() == false; diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java index c6e7fa3517771..b7b59579529f3 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java @@ -374,10 +374,7 @@ protected HttpMessage createMessage(String[] initialLine) throws Exception { // combines the HTTP message pieces into a single full HTTP request (with headers and body) final HttpObjectAggregator aggregator = new Netty4HttpAggregator( handlingSettings.maxContentLength(), - httpPreRequest -> enabled.get() == false - || (httpPreRequest.uri().contains("_bulk") == false - || httpPreRequest.uri().contains("_bulk_update") - || httpPreRequest.uri().contains("/_xpack/monitoring/_bulk")) + httpPreRequest -> enabled.get() == false || (httpPreRequest.rawPath().endsWith("/_bulk") == false) ); aggregator.setMaxCumulationBufferComponents(transport.maxCompositeBufferComponents); ch.pipeline() diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/IncrementalBulkRestIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/IncrementalBulkRestIT.java index 08026e0435f33..2b24e53874e51 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/IncrementalBulkRestIT.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/IncrementalBulkRestIT.java @@ -29,6 +29,12 @@ @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 2, numClientNodes = 0) public class IncrementalBulkRestIT extends HttpSmokeTestCase { + public void testBulkUriMatchingDoesNotMatchBulkCapabilitiesApi() throws IOException { + Request request = new Request("GET", "/_capabilities?method=GET&path=%2F_bulk&capabilities=failure_store_status&pretty"); + Response response = getRestClient().performRequest(request); + assertEquals(200, response.getStatusLine().getStatusCode()); + } + public void testBulkMissingBody() throws IOException { Request request = new Request(randomBoolean() ? "POST" : "PUT", "/_bulk"); request.setJsonEntity(""); diff --git a/server/src/main/java/org/elasticsearch/http/HttpPreRequest.java b/server/src/main/java/org/elasticsearch/http/HttpPreRequest.java index 77454c5686538..ccf375aec60a5 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpPreRequest.java +++ b/server/src/main/java/org/elasticsearch/http/HttpPreRequest.java @@ -33,6 +33,24 @@ public interface HttpPreRequest { */ String uri(); + /** + * The uri without the query string. + */ + default String rawPath() { + String uri = uri(); + final int index = uri.indexOf('?'); + if (index >= 0) { + return uri.substring(0, index); + } else { + final int index2 = uri.indexOf('#'); + if (index2 >= 0) { + return uri.substring(0, index2); + } else { + return uri; + } + } + } + /** * Get all of the headers and values associated with the HTTP headers. * Modifications of this map are not supported. diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index e48677f46d57a..17eda305b5ccf 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -105,19 +105,19 @@ public boolean isContentConsumed() { protected RestRequest( XContentParserConfiguration parserConfig, Map params, - String path, + String rawPath, Map> headers, HttpRequest httpRequest, HttpChannel httpChannel ) { - this(parserConfig, params, path, headers, httpRequest, httpChannel, requestIdGenerator.incrementAndGet()); + this(parserConfig, params, rawPath, headers, httpRequest, httpChannel, requestIdGenerator.incrementAndGet()); } @SuppressWarnings("this-escape") private RestRequest( XContentParserConfiguration parserConfig, Map params, - String path, + String rawPath, Map> headers, HttpRequest httpRequest, HttpChannel httpChannel, @@ -149,7 +149,7 @@ private RestRequest( : parserConfig.withRestApiVersion(effectiveApiVersion); this.httpChannel = httpChannel; this.params = params; - this.rawPath = path; + this.rawPath = rawPath; this.headers = Collections.unmodifiableMap(headers); this.requestId = requestId; } @@ -204,11 +204,10 @@ void ensureSafeBuffers() { */ public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) { Map params = params(httpRequest.uri()); - String path = path(httpRequest.uri()); return new RestRequest( parserConfig, params, - path, + httpRequest.rawPath(), httpRequest.getHeaders(), httpRequest, httpChannel, @@ -229,15 +228,6 @@ private static Map params(final String uri) { return params; } - private static String path(final String uri) { - final int index = uri.indexOf('?'); - if (index >= 0) { - return uri.substring(0, index); - } else { - return uri; - } - } - /** * Creates a new REST request. The path is not decoded so this constructor will not throw a * {@link BadParameterException}. From a45ca31c01fcbf2465601ff6cdccdfc4e9e25071 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 06:15:16 +1000 Subject: [PATCH 16/46] Mute org.elasticsearch.integration.KibanaUserRoleIntegTests testGetIndex #113311 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 6b7e30bb9c0e3..d042e2f1aef26 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -288,6 +288,9 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/113296 - class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT issue: https://github.com/elastic/elasticsearch/issues/113298 +- class: org.elasticsearch.integration.KibanaUserRoleIntegTests + method: testGetIndex + issue: https://github.com/elastic/elasticsearch/issues/113311 # Examples: # From b8551130aa797331f65b0137f52aeea2429167f9 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 06:43:10 +1000 Subject: [PATCH 17/46] Mute org.elasticsearch.packaging.test.WindowsServiceTests test81JavaOptsInJvmOptions #113313 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index d042e2f1aef26..00a7a6748e09e 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -291,6 +291,9 @@ tests: - class: org.elasticsearch.integration.KibanaUserRoleIntegTests method: testGetIndex issue: https://github.com/elastic/elasticsearch/issues/113311 +- class: org.elasticsearch.packaging.test.WindowsServiceTests + method: test81JavaOptsInJvmOptions + issue: https://github.com/elastic/elasticsearch/issues/113313 # Examples: # From b85eb92fea997ee1ce0476efe396a3b444f2aaf8 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 07:01:07 +1000 Subject: [PATCH 18/46] Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=esql/50_index_patterns/disjoint_mappings} #113315 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 00a7a6748e09e..1515d1ad01ac2 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -294,6 +294,9 @@ tests: - class: org.elasticsearch.packaging.test.WindowsServiceTests method: test81JavaOptsInJvmOptions issue: https://github.com/elastic/elasticsearch/issues/113313 +- class: org.elasticsearch.xpack.test.rest.XPackRestIT + method: test {p0=esql/50_index_patterns/disjoint_mappings} + issue: https://github.com/elastic/elasticsearch/issues/113315 # Examples: # From f5b979b1115dbdbefea6c313d2fe491a47638615 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 07:10:36 +1000 Subject: [PATCH 19/46] Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=wildcard/10_wildcard_basic/Query_string wildcard query} #113316 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 1515d1ad01ac2..75d0242f0a921 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -297,6 +297,9 @@ tests: - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=esql/50_index_patterns/disjoint_mappings} issue: https://github.com/elastic/elasticsearch/issues/113315 +- class: org.elasticsearch.xpack.test.rest.XPackRestIT + method: test {p0=wildcard/10_wildcard_basic/Query_string wildcard query} + issue: https://github.com/elastic/elasticsearch/issues/113316 # Examples: # From 13f80b02511d98763a762c6251ba746314683c34 Mon Sep 17 00:00:00 2001 From: john-wagster Date: Fri, 20 Sep 2024 16:25:24 -0500 Subject: [PATCH 20/46] Deduplicate Nori and Kuromoji User Dictionary (#112768) added the ability to deduplicate the user dictionary optionally --- docs/changelog/112768.yaml | 5 ++ docs/plugins/analysis-kuromoji.asciidoc | 8 +- docs/plugins/analysis-nori.asciidoc | 9 +- .../kuromoji/KuromojiTokenizerFactory.java | 12 ++- .../kuromoji/KuromojiAnalysisTests.java | 21 ++++- .../analysis/nori/NoriTokenizerFactory.java | 4 +- .../analysis/nori/NoriAnalysisTests.java | 16 +++- .../index/analysis/Analysis.java | 40 ++++++--- .../index/analysis/AnalysisTests.java | 89 +++++++++++++++++++ 9 files changed, 186 insertions(+), 18 deletions(-) create mode 100644 docs/changelog/112768.yaml diff --git a/docs/changelog/112768.yaml b/docs/changelog/112768.yaml new file mode 100644 index 0000000000000..13d5b8eaae38f --- /dev/null +++ b/docs/changelog/112768.yaml @@ -0,0 +1,5 @@ +pr: 112768 +summary: Deduplicate Kuromoji User Dictionary +area: Search +type: enhancement +issues: [] diff --git a/docs/plugins/analysis-kuromoji.asciidoc b/docs/plugins/analysis-kuromoji.asciidoc index b1d1d5a751057..e8380ce0aca17 100644 --- a/docs/plugins/analysis-kuromoji.asciidoc +++ b/docs/plugins/analysis-kuromoji.asciidoc @@ -133,6 +133,11 @@ unknown words. It can be set to: Whether punctuation should be discarded from the output. Defaults to `true`. +`lenient`:: + + Whether the `user_dictionary` should be deduplicated on the provided `text`. + False by default causing duplicates to generate an error. + `user_dictionary`:: + -- @@ -221,7 +226,8 @@ PUT kuromoji_sample "type": "kuromoji_tokenizer", "mode": "extended", "discard_punctuation": "false", - "user_dictionary": "userdict_ja.txt" + "user_dictionary": "userdict_ja.txt", + "lenient": "true" } }, "analyzer": { diff --git a/docs/plugins/analysis-nori.asciidoc b/docs/plugins/analysis-nori.asciidoc index 1a3153fa3bea5..e7855f94758e1 100644 --- a/docs/plugins/analysis-nori.asciidoc +++ b/docs/plugins/analysis-nori.asciidoc @@ -58,6 +58,11 @@ It can be set to: Whether punctuation should be discarded from the output. Defaults to `true`. +`lenient`:: + + Whether the `user_dictionary` should be deduplicated on the provided `text`. + False by default causing duplicates to generate an error. + `user_dictionary`:: + -- @@ -104,7 +109,8 @@ PUT nori_sample "type": "nori_tokenizer", "decompound_mode": "mixed", "discard_punctuation": "false", - "user_dictionary": "userdict_ko.txt" + "user_dictionary": "userdict_ko.txt", + "lenient": "true" } }, "analyzer": { @@ -299,7 +305,6 @@ Which responds with: } -------------------------------------------------- - [[analysis-nori-speech]] ==== `nori_part_of_speech` token filter diff --git a/plugins/analysis-kuromoji/src/main/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiTokenizerFactory.java b/plugins/analysis-kuromoji/src/main/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiTokenizerFactory.java index a7fa63709d580..edb29a8f4c98e 100644 --- a/plugins/analysis-kuromoji/src/main/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiTokenizerFactory.java +++ b/plugins/analysis-kuromoji/src/main/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiTokenizerFactory.java @@ -33,6 +33,7 @@ public class KuromojiTokenizerFactory extends AbstractTokenizerFactory { private static final String NBEST_COST = "nbest_cost"; private static final String NBEST_EXAMPLES = "nbest_examples"; private static final String DISCARD_COMPOUND_TOKEN = "discard_compound_token"; + private static final String LENIENT = "lenient"; private final UserDictionary userDictionary; private final Mode mode; @@ -58,7 +59,15 @@ public static UserDictionary getUserDictionary(Environment env, Settings setting "It is not allowed to use [" + USER_DICT_PATH_OPTION + "] in conjunction" + " with [" + USER_DICT_RULES_OPTION + "]" ); } - List ruleList = Analysis.getWordList(env, settings, USER_DICT_PATH_OPTION, USER_DICT_RULES_OPTION, false, true); + List ruleList = Analysis.getWordList( + env, + settings, + USER_DICT_PATH_OPTION, + USER_DICT_RULES_OPTION, + LENIENT, + false, // typically don't want to remove comments as deduplication will provide better feedback + true + ); if (ruleList == null || ruleList.isEmpty()) { return null; } @@ -66,6 +75,7 @@ public static UserDictionary getUserDictionary(Environment env, Settings setting for (String line : ruleList) { sb.append(line).append(System.lineSeparator()); } + try (Reader rulesReader = new StringReader(sb.toString())) { return UserDictionary.open(rulesReader); } catch (IOException e) { diff --git a/plugins/analysis-kuromoji/src/test/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiAnalysisTests.java b/plugins/analysis-kuromoji/src/test/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiAnalysisTests.java index 1229b4f348911..f26213d86c5a9 100644 --- a/plugins/analysis-kuromoji/src/test/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiAnalysisTests.java +++ b/plugins/analysis-kuromoji/src/test/java/org/elasticsearch/plugin/analysis/kuromoji/KuromojiAnalysisTests.java @@ -445,7 +445,26 @@ public void testKuromojiAnalyzerDuplicateUserDictRule() throws Exception { ) .build(); IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> createTestAnalysis(settings)); - assertThat(exc.getMessage(), containsString("[制限スピード] in user dictionary at line [3]")); + assertThat(exc.getMessage(), containsString("[制限スピード] in user dictionary at line [4]")); + } + + public void testKuromojiAnalyzerDuplicateUserDictRuleDeduplication() throws Exception { + Settings settings = Settings.builder() + .put("index.analysis.analyzer.my_analyzer.type", "kuromoji") + .put("index.analysis.analyzer.my_analyzer.lenient", "true") + .putList( + "index.analysis.analyzer.my_analyzer.user_dictionary_rules", + "c++,c++,w,w", + "#comment", + "制限スピード,制限スピード,セイゲンスピード,テスト名詞", + "制限スピード,制限スピード,セイゲンスピード,テスト名詞" + ) + .build(); + TestAnalysis analysis = createTestAnalysis(settings); + Analyzer analyzer = analysis.indexAnalyzers.get("my_analyzer"); + try (TokenStream stream = analyzer.tokenStream("", "制限スピード")) { + assertTokenStreamContents(stream, new String[] { "制限スピード" }); + } } public void testDiscardCompoundToken() throws Exception { diff --git a/plugins/analysis-nori/src/main/java/org/elasticsearch/plugin/analysis/nori/NoriTokenizerFactory.java b/plugins/analysis-nori/src/main/java/org/elasticsearch/plugin/analysis/nori/NoriTokenizerFactory.java index 8bc53fa69c9a7..ed8458bc94043 100644 --- a/plugins/analysis-nori/src/main/java/org/elasticsearch/plugin/analysis/nori/NoriTokenizerFactory.java +++ b/plugins/analysis-nori/src/main/java/org/elasticsearch/plugin/analysis/nori/NoriTokenizerFactory.java @@ -31,6 +31,7 @@ public class NoriTokenizerFactory extends AbstractTokenizerFactory { private static final String USER_DICT_PATH_OPTION = "user_dictionary"; private static final String USER_DICT_RULES_OPTION = "user_dictionary_rules"; + private static final String LENIENT = "lenient"; private final UserDictionary userDictionary; private final KoreanTokenizer.DecompoundMode decompoundMode; @@ -54,7 +55,8 @@ public static UserDictionary getUserDictionary(Environment env, Settings setting settings, USER_DICT_PATH_OPTION, USER_DICT_RULES_OPTION, - true, + LENIENT, + false, // typically don't want to remove comments as deduplication will provide better feedback isSupportDuplicateCheck(indexSettings) ); if (ruleList == null || ruleList.isEmpty()) { diff --git a/plugins/analysis-nori/src/test/java/org/elasticsearch/plugin/analysis/nori/NoriAnalysisTests.java b/plugins/analysis-nori/src/test/java/org/elasticsearch/plugin/analysis/nori/NoriAnalysisTests.java index e1123f167da99..1709d02263eea 100644 --- a/plugins/analysis-nori/src/test/java/org/elasticsearch/plugin/analysis/nori/NoriAnalysisTests.java +++ b/plugins/analysis-nori/src/test/java/org/elasticsearch/plugin/analysis/nori/NoriAnalysisTests.java @@ -127,7 +127,7 @@ public void testNoriAnalyzerDuplicateUserDictRule() throws Exception { .build(); final IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> createTestAnalysis(settings)); - assertThat(exc.getMessage(), containsString("[세종] in user dictionary at line [3]")); + assertThat(exc.getMessage(), containsString("[세종] in user dictionary at line [4]")); } public void testNoriAnalyzerDuplicateUserDictRuleWithLegacyVersion() throws IOException { @@ -144,6 +144,20 @@ public void testNoriAnalyzerDuplicateUserDictRuleWithLegacyVersion() throws IOEx } } + public void testNoriAnalyzerDuplicateUserDictRuleDeduplication() throws Exception { + Settings settings = Settings.builder() + .put("index.analysis.analyzer.my_analyzer.type", "nori") + .put("index.analysis.analyzer.my_analyzer.lenient", "true") + .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersions.NORI_DUPLICATES) + .putList("index.analysis.analyzer.my_analyzer.user_dictionary_rules", "c++", "C쁠쁠", "세종", "세종", "세종시 세종 시") + .build(); + TestAnalysis analysis = createTestAnalysis(settings); + Analyzer analyzer = analysis.indexAnalyzers.get("my_analyzer"); + try (TokenStream stream = analyzer.tokenStream("", "세종시")) { + assertTokenStreamContents(stream, new String[] { "세종", "시" }); + } + } + public void testNoriTokenizer() throws Exception { Settings settings = Settings.builder() .put("index.analysis.tokenizer.my_tokenizer.type", "nori_tokenizer") diff --git a/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java b/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java index 1a90f5f110376..462490a7fceb7 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/Analysis.java @@ -9,6 +9,8 @@ package org.elasticsearch.index.analysis; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.lucene.analysis.CharArraySet; import org.apache.lucene.analysis.ar.ArabicAnalyzer; import org.apache.lucene.analysis.bg.BulgarianAnalyzer; @@ -67,6 +69,7 @@ import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -78,6 +81,7 @@ public class Analysis { private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(Analysis.class); + private static final Logger logger = LogManager.getLogger(Analysis.class); public static void checkForDeprecatedVersion(String name, Settings settings) { String sVersion = settings.get("version"); @@ -267,12 +271,14 @@ public static List getWordList( Settings settings, String settingPath, String settingList, + String settingLenient, boolean removeComments, boolean checkDuplicate ) { + boolean deduplicateDictionary = settings.getAsBoolean(settingLenient, false); final List ruleList = getWordList(env, settings, settingPath, settingList, removeComments); if (ruleList != null && ruleList.isEmpty() == false && checkDuplicate) { - checkDuplicateRules(ruleList); + return deDuplicateRules(ruleList, deduplicateDictionary == false); } return ruleList; } @@ -288,24 +294,36 @@ public static List getWordList( * If the addition to the HashSet returns false, it means that item was already present in the set, indicating a duplicate. * In such a case, an IllegalArgumentException is thrown specifying the duplicate term and the line number in the original list. * + * Optionally the function will return the deduplicated list + * * @param ruleList The list of rules to check for duplicates. * @throws IllegalArgumentException If a duplicate rule is found. */ - private static void checkDuplicateRules(List ruleList) { - Set dup = new HashSet<>(); - int lineNum = 0; - for (String line : ruleList) { - // ignore comments + private static List deDuplicateRules(List ruleList, boolean failOnDuplicate) { + Set duplicateKeys = new HashSet<>(); + List deduplicatedList = new ArrayList<>(); + for (int lineNum = 0; lineNum < ruleList.size(); lineNum++) { + String line = ruleList.get(lineNum); + // ignore lines beginning with # as those are comments if (line.startsWith("#") == false) { String[] values = CSVUtil.parse(line); - if (dup.add(values[0]) == false) { - throw new IllegalArgumentException( - "Found duplicate term [" + values[0] + "] in user dictionary " + "at line [" + lineNum + "]" - ); + if (duplicateKeys.add(values[0]) == false) { + if (failOnDuplicate) { + throw new IllegalArgumentException( + "Found duplicate term [" + values[0] + "] in user dictionary " + "at line [" + (lineNum + 1) + "]" + ); + } else { + logger.warn("Ignoring duplicate term [" + values[0] + "] in user dictionary " + "at line [" + (lineNum + 1) + "]"); + } + } else { + deduplicatedList.add(line); } + } else { + deduplicatedList.add(line); } - ++lineNum; } + + return Collections.unmodifiableList(deduplicatedList); } private static List loadWordList(Path path, boolean removeComments) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java index 86c268dd2a092..e05b67874ddbb 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.List; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.is; public class AnalysisTests extends ESTestCase { @@ -104,4 +105,92 @@ public void testParseWordList() throws IOException { List wordList = Analysis.getWordList(env, nodeSettings, "foo.bar"); assertEquals(Arrays.asList("hello", "world"), wordList); } + + public void testParseDuplicates() throws IOException { + Path tempDir = createTempDir(); + Path dict = tempDir.resolve("foo.dict"); + Settings nodeSettings = Settings.builder() + .put("foo.path", tempDir.resolve(dict)) + .put("bar.list", "") + .put("soup.lenient", "true") + .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) + .build(); + try (BufferedWriter writer = Files.newBufferedWriter(dict, StandardCharsets.UTF_8)) { + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞,extra stuff that gets discarded"); + writer.write('\n'); + } + Environment env = TestEnvironment.newEnvironment(nodeSettings); + List wordList = Analysis.getWordList(env, nodeSettings, "foo.path", "bar.list", "soup.lenient", true, true); + assertEquals(List.of("最終契約,最終契約,最終契約,カスタム名 詞"), wordList); + } + + public void testFailOnDuplicates() throws IOException { + Path tempDir = createTempDir(); + Path dict = tempDir.resolve("foo.dict"); + Settings nodeSettings = Settings.builder() + .put("foo.path", tempDir.resolve(dict)) + .put("bar.list", "") + .put("soup.lenient", "false") + .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) + .build(); + try (BufferedWriter writer = Files.newBufferedWriter(dict, StandardCharsets.UTF_8)) { + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("最終契,最終契,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞,extra"); + writer.write('\n'); + } + Environment env = TestEnvironment.newEnvironment(nodeSettings); + IllegalArgumentException exc = expectThrows( + IllegalArgumentException.class, + () -> Analysis.getWordList(env, nodeSettings, "foo.path", "bar.list", "soup.lenient", false, true) + ); + assertThat(exc.getMessage(), containsString("[最終契約] in user dictionary at line [5]")); + } + + public void testParseDuplicatesWComments() throws IOException { + Path tempDir = createTempDir(); + Path dict = tempDir.resolve("foo.dict"); + Settings nodeSettings = Settings.builder() + .put("foo.path", tempDir.resolve(dict)) + .put("bar.list", "") + .put("soup.lenient", "true") + .put(Environment.PATH_HOME_SETTING.getKey(), tempDir) + .build(); + try (BufferedWriter writer = Files.newBufferedWriter(dict, StandardCharsets.UTF_8)) { + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞"); + writer.write('\n'); + writer.write("# This is a test of the emergency broadcast system"); + writer.write('\n'); + writer.write("最終契約,最終契約,最終契約,カスタム名 詞,extra"); + writer.write('\n'); + } + Environment env = TestEnvironment.newEnvironment(nodeSettings); + List wordList = Analysis.getWordList(env, nodeSettings, "foo.path", "bar.list", "soup.lenient", false, true); + assertEquals( + List.of( + "# This is a test of the emergency broadcast system", + "最終契約,最終契約,最終契約,カスタム名 詞", + "# This is a test of the emergency broadcast system" + ), + wordList + ); + } } From e257d93f95e928e4507b5196594129fa37586108 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 20 Sep 2024 16:04:37 -0600 Subject: [PATCH 21/46] Handle inbound exceptions on streaming request APIs (#113303) Today if we submit a request with e.g. invalid credentials to the `/_bulk` API then we throw a `ClassCastException` trying to cast the `DefaultHttpRequest` to a `FullHttpRequest` on the error path. This commit fixes the problem. --- .../http/netty4/Netty4HttpAggregator.java | 2 +- .../test/security/10_forbidden.yml | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/security/10_forbidden.yml diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpAggregator.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpAggregator.java index 3c9e684ef4279..021ce09e0ed8e 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpAggregator.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpAggregator.java @@ -46,7 +46,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception assert msg instanceof HttpObject; if (msg instanceof HttpRequest request) { var preReq = HttpHeadersAuthenticatorUtils.asHttpPreRequest(request); - aggregating = decider.test(preReq) && IGNORE_TEST.test(preReq); + aggregating = (decider.test(preReq) && IGNORE_TEST.test(preReq)) || request.decoderResult().isFailure(); } if (aggregating || msg instanceof FullHttpRequest) { super.channelRead(ctx, msg); diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/security/10_forbidden.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/security/10_forbidden.yml new file mode 100644 index 0000000000000..545b9ef1e0285 --- /dev/null +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/security/10_forbidden.yml @@ -0,0 +1,31 @@ +--- +"Test basic response with invalid credentials": + + - skip: + features: headers + + - do: + headers: + Authorization: "Basic dGVzdF91c2VyOndyb25nLXBhc3N3b3Jk" # invalid credentials + info: {} + catch: unauthorized + + - match: + error.root_cause.0.type: security_exception + +--- +"Test bulk response with invalid credentials": + + - skip: + features: headers + + - do: + headers: + Authorization: "Basic dGVzdF91c2VyOndyb25nLXBhc3N3b3Jk" # invalid credentials + bulk: + body: | + {"index": {}} + {} + catch: unauthorized + - match: + error.root_cause.0.type: security_exception From 413b23a9ea16206e8cb97bc99f5ab6ac578229c7 Mon Sep 17 00:00:00 2001 From: Oleksandr Kolomiiets Date: Fri, 20 Sep 2024 15:50:02 -0700 Subject: [PATCH 22/46] Unmute logsdb data generation tests (#113306) --- muted-tests.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 75d0242f0a921..e01d977a41c60 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -215,9 +215,6 @@ tests: - class: org.elasticsearch.xpack.sql.qa.security.JdbcSqlSpecIT method: test {case-functions.testSelectInsertWithLcaseAndLengthWithOrderBy} issue: https://github.com/elastic/elasticsearch/issues/112642 -- class: org.elasticsearch.datastreams.logsdb.qa.StandardVersusLogsIndexModeRandomDataChallengeRestIT - method: testHistogramAggregation - issue: https://github.com/elastic/elasticsearch/issues/113109 - class: org.elasticsearch.action.admin.cluster.node.stats.NodeStatsTests method: testChunking issue: https://github.com/elastic/elasticsearch/issues/113139 @@ -262,15 +259,9 @@ tests: - class: org.elasticsearch.index.mapper.DoubleRangeFieldMapperTests method: testSyntheticSourceKeepAll issue: https://github.com/elastic/elasticsearch/issues/113234 -- class: org.elasticsearch.datastreams.logsdb.qa.StandardVersusLogsIndexModeRandomDataChallengeRestIT - method: testTermsQuery - issue: https://github.com/elastic/elasticsearch/issues/113246 - class: org.elasticsearch.integration.KibanaUserRoleIntegTests method: testGetMappings issue: https://github.com/elastic/elasticsearch/issues/113260 -- class: org.elasticsearch.datastreams.logsdb.qa.StandardVersusLogsIndexModeRandomDataChallengeRestIT - method: testMatchAllQuery - issue: https://github.com/elastic/elasticsearch/issues/113265 - class: org.elasticsearch.xpack.security.authz.SecurityScrollTests method: testSearchAndClearScroll issue: https://github.com/elastic/elasticsearch/issues/113285 From 6b11af38d6ed0a0ad3dd63fa6fcef6fd9bcac33a Mon Sep 17 00:00:00 2001 From: Oleksandr Kolomiiets Date: Fri, 20 Sep 2024 16:25:26 -0700 Subject: [PATCH 23/46] Fix new synthetic source copy_to tests to pass on serverless (#113320) --- .../indices.create/20_synthetic_source.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 937c5f19ae5aa..b5a9146bc54a6 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 @@ -1815,6 +1815,8 @@ synthetic_source with copy_to pointing at dynamic field: _source: mode: synthetic properties: + name: + type: keyword k: type: keyword copy_to: c.copy @@ -1829,6 +1831,7 @@ synthetic_source with copy_to pointing at dynamic field: id: 1 refresh: true body: + name: "A" k: "hello" - do: @@ -1837,6 +1840,7 @@ synthetic_source with copy_to pointing at dynamic field: id: 2 refresh: true body: + name: "B" k: ["55", "66"] - do: @@ -1845,6 +1849,7 @@ synthetic_source with copy_to pointing at dynamic field: id: 3 refresh: true body: + name: "C" k: "hello" c: copy: "zap" @@ -1852,11 +1857,13 @@ synthetic_source with copy_to pointing at dynamic field: - do: search: index: test + sort: name body: docvalue_fields: [ "c.copy.keyword" ] - match: hits.hits.0._source: + name: "A" k: "hello" - match: hits.hits.0.fields: @@ -1864,6 +1871,7 @@ synthetic_source with copy_to pointing at dynamic field: - match: hits.hits.1._source: + name: "B" k: ["55", "66"] - match: hits.hits.1.fields: @@ -1871,6 +1879,7 @@ synthetic_source with copy_to pointing at dynamic field: - match: hits.hits.2._source: + name: "C" k: "hello" c: copy: "zap" @@ -1892,6 +1901,8 @@ synthetic_source with copy_to pointing inside dynamic object: _source: mode: synthetic properties: + name: + type: keyword k: type: keyword copy_to: c.copy @@ -1902,6 +1913,7 @@ synthetic_source with copy_to pointing inside dynamic object: id: 1 refresh: true body: + name: "A" k: "hello" - do: @@ -1910,6 +1922,7 @@ synthetic_source with copy_to pointing inside dynamic object: id: 2 refresh: true body: + name: "B" k: ["55", "66"] - do: @@ -1918,6 +1931,7 @@ synthetic_source with copy_to pointing inside dynamic object: id: 3 refresh: true body: + name: "C" k: "hello" c: copy: "zap" @@ -1925,11 +1939,13 @@ synthetic_source with copy_to pointing inside dynamic object: - do: search: index: test + sort: name body: docvalue_fields: [ "c.copy.keyword" ] - match: hits.hits.0._source: + name: "A" k: "hello" - match: hits.hits.0.fields: @@ -1937,6 +1953,7 @@ synthetic_source with copy_to pointing inside dynamic object: - match: hits.hits.1._source: + name: "B" k: ["55", "66"] - match: hits.hits.1.fields: @@ -1944,6 +1961,7 @@ synthetic_source with copy_to pointing inside dynamic object: - match: hits.hits.2._source: + name: "C" k: "hello" c: copy: "zap" From 63db96e6ed4216ad684b7d3eb20193ac65cbfd5a Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 20 Sep 2024 17:33:11 -0600 Subject: [PATCH 24/46] Ensure monitoring/_bulk route is fully aggregated (#113319) This route still exists in the code even if it is two major version old. We should still support it properly with our streaming changes. --- .../elasticsearch/http/netty4/Netty4HttpServerTransport.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java index b7b59579529f3..5ed3d81392951 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpServerTransport.java @@ -374,7 +374,9 @@ protected HttpMessage createMessage(String[] initialLine) throws Exception { // combines the HTTP message pieces into a single full HTTP request (with headers and body) final HttpObjectAggregator aggregator = new Netty4HttpAggregator( handlingSettings.maxContentLength(), - httpPreRequest -> enabled.get() == false || (httpPreRequest.rawPath().endsWith("/_bulk") == false) + httpPreRequest -> enabled.get() == false + || ((httpPreRequest.rawPath().endsWith("/_bulk") == false) + || httpPreRequest.rawPath().startsWith("/_xpack/monitoring/_bulk")) ); aggregator.setMaxCumulationBufferComponents(transport.maxCompositeBufferComponents); ch.pipeline() From 1c72e83efeb8a8cf3d8eefaee1203ccbad16a6d3 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 14:27:44 +1000 Subject: [PATCH 25/46] Mute org.elasticsearch.index.mapper.LongRangeFieldMapperTests testSyntheticSourceKeepAll #113324 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index e01d977a41c60..062fbe2474924 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -291,6 +291,9 @@ tests: - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=wildcard/10_wildcard_basic/Query_string wildcard query} issue: https://github.com/elastic/elasticsearch/issues/113316 +- class: org.elasticsearch.index.mapper.LongRangeFieldMapperTests + method: testSyntheticSourceKeepAll + issue: https://github.com/elastic/elasticsearch/issues/113324 # Examples: # From 0c822e9b5bb0aab9e5a6921dec0de02eafadd9f4 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 14:52:40 +1000 Subject: [PATCH 26/46] Mute org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT test {p0=mtermvectors/10_basic/Tests catching other exceptions per item} #113325 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 062fbe2474924..47edd4f4896f8 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -294,6 +294,9 @@ tests: - class: org.elasticsearch.index.mapper.LongRangeFieldMapperTests method: testSyntheticSourceKeepAll issue: https://github.com/elastic/elasticsearch/issues/113324 +- class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT + method: test {p0=mtermvectors/10_basic/Tests catching other exceptions per item} + issue: https://github.com/elastic/elasticsearch/issues/113325 # Examples: # From 0a493be91c524431650d4c874f08ad22d99562f0 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 14:59:57 +1000 Subject: [PATCH 27/46] Mute org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests testSyntheticSourceKeepArrays #113326 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 47edd4f4896f8..102e09c348724 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -297,6 +297,9 @@ tests: - class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT method: test {p0=mtermvectors/10_basic/Tests catching other exceptions per item} issue: https://github.com/elastic/elasticsearch/issues/113325 +- class: org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests + method: testSyntheticSourceKeepArrays + issue: https://github.com/elastic/elasticsearch/issues/113326 # Examples: # From e8cba9c8e4adf642d1c955eeaee3eef70dd6affc Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 15:01:55 +1000 Subject: [PATCH 28/46] Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=transform/transforms_force_delete/Test force deleting a running transform} #113327 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 102e09c348724..1332f792a232d 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -300,6 +300,9 @@ tests: - class: org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests method: testSyntheticSourceKeepArrays issue: https://github.com/elastic/elasticsearch/issues/113326 +- 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 # Examples: # From 069708985ea3058443f672cd49be5e77bf993410 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 15:29:10 +1000 Subject: [PATCH 29/46] Mute org.elasticsearch.integration.KibanaUserRoleIntegTests testValidateQuery #113328 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 1332f792a232d..45f7597a9f685 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -303,6 +303,9 @@ 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.integration.KibanaUserRoleIntegTests + method: testValidateQuery + issue: https://github.com/elastic/elasticsearch/issues/113328 # Examples: # From 0a3d89415a2805bb4e43cd6c09fa4e1ad97c4f7a Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 22:26:16 +1000 Subject: [PATCH 30/46] Mute org.elasticsearch.index.mapper.LongRangeFieldMapperTests testSyntheticSourceKeepArrays #113335 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 45f7597a9f685..e01146bb2b33b 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -306,6 +306,9 @@ tests: - class: org.elasticsearch.integration.KibanaUserRoleIntegTests method: testValidateQuery issue: https://github.com/elastic/elasticsearch/issues/113328 +- class: org.elasticsearch.index.mapper.LongRangeFieldMapperTests + method: testSyntheticSourceKeepArrays + issue: https://github.com/elastic/elasticsearch/issues/113335 # Examples: # From 322ed681d95318209fa230021a4f832eadc9e8a2 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 21 Sep 2024 23:31:03 +1000 Subject: [PATCH 31/46] Mute org.elasticsearch.xpack.security.support.SecurityIndexManagerIntegTests testOnIndexAvailableForSearchIndexAlreadyAvailable #113336 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index e01146bb2b33b..90a605e2ca6e9 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -309,6 +309,9 @@ tests: - class: org.elasticsearch.index.mapper.LongRangeFieldMapperTests method: testSyntheticSourceKeepArrays issue: https://github.com/elastic/elasticsearch/issues/113335 +- class: org.elasticsearch.xpack.security.support.SecurityIndexManagerIntegTests + method: testOnIndexAvailableForSearchIndexAlreadyAvailable + issue: https://github.com/elastic/elasticsearch/issues/113336 # Examples: # From 87992f5922b62612c7ed4caf0f575456b8336732 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 06:16:38 +1000 Subject: [PATCH 32/46] Mute org.elasticsearch.xpack.security.authz.SecurityScrollTests testScrollIsPerUser #113338 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 90a605e2ca6e9..0937b781a6370 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -312,6 +312,9 @@ tests: - class: org.elasticsearch.xpack.security.support.SecurityIndexManagerIntegTests method: testOnIndexAvailableForSearchIndexAlreadyAvailable issue: https://github.com/elastic/elasticsearch/issues/113336 +- class: org.elasticsearch.xpack.security.authz.SecurityScrollTests + method: testScrollIsPerUser + issue: https://github.com/elastic/elasticsearch/issues/113338 # Examples: # From ca005814f7f039ad6946773a5dea5c9889d14aef Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 06:27:12 +1000 Subject: [PATCH 33/46] Mute org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests testSyntheticSourceKeepAll #113339 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 0937b781a6370..f92e01c9ac31c 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -315,6 +315,9 @@ tests: - class: org.elasticsearch.xpack.security.authz.SecurityScrollTests method: testScrollIsPerUser issue: https://github.com/elastic/elasticsearch/issues/113338 +- class: org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests + method: testSyntheticSourceKeepAll + issue: https://github.com/elastic/elasticsearch/issues/113339 # Examples: # From 39f8a14cc1a2428fb22fd3e2f88a85eabf77b5da Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 07:12:45 +1000 Subject: [PATCH 34/46] Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=analytics/top_metrics/sort by scaled float field} #113340 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index f92e01c9ac31c..7f9f44a07f66a 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -318,6 +318,9 @@ tests: - class: org.elasticsearch.index.mapper.IntegerRangeFieldMapperTests method: testSyntheticSourceKeepAll issue: https://github.com/elastic/elasticsearch/issues/113339 +- 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 # Examples: # From 749ca1c5e7c3787c631cc51bb8f97847c945348e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Sat, 21 Sep 2024 23:26:14 +0200 Subject: [PATCH 35/46] Misc cleanup fieldata package (#113331) Just some obvious cleanup and deduplication. --- .../codec/tsdb/ES87TSDBDocValuesConsumer.java | 4 +- .../fielddata/AbstractBinaryDocValues.java | 10 ++-- .../fielddata/AbstractNumericDocValues.java | 4 +- .../fielddata/AbstractSortedDocValues.java | 4 +- .../AbstractSortedNumericDocValues.java | 2 +- .../fielddata/AbstractSortedSetDocValues.java | 4 +- .../AbstractSortingNumericDocValues.java | 4 +- .../fielddata/BinaryScriptFieldData.java | 4 -- .../fielddata/DoubleScriptFieldData.java | 2 +- .../index/fielddata/FieldData.java | 16 ++---- .../fielddata/GeoPointScriptDocValues.java | 2 +- .../fielddata/GeoPointScriptFieldData.java | 4 -- .../fielddata/IndexFieldDataService.java | 4 +- .../fielddata/IndexNumericFieldData.java | 22 ++++---- .../index/fielddata/LeafFieldData.java | 3 ++ .../index/fielddata/LongScriptFieldData.java | 4 +- .../index/fielddata/MultiGeoPointValues.java | 1 - .../index/fielddata/MultiPointValues.java | 4 -- .../fielddata/RamAccountingTermsEnum.java | 2 +- .../fielddata/SortingNumericDocValues.java | 2 +- .../SourceValueFetcherIndexFieldData.java | 5 -- ...lueFetcherMultiGeoPointIndexFieldData.java | 2 +- ...alueFetcherSortedBinaryIndexFieldData.java | 4 +- ...lueFetcherSortedBooleanIndexFieldData.java | 8 +-- ...alueFetcherSortedDoubleIndexFieldData.java | 4 +- ...lueFetcherSortedNumericIndexFieldData.java | 8 +-- .../fielddata/StoredFieldIndexFieldData.java | 3 -- ...StoredFieldSortedBinaryIndexFieldData.java | 2 +- .../FloatValuesComparatorSource.java | 2 +- .../ordinals/GlobalOrdinalsBuilder.java | 2 - .../GlobalOrdinalsIndexFieldData.java | 10 ++-- .../fielddata/ordinals/OrdinalsBuilder.java | 3 +- .../plain/AbstractBinaryDVLeafFieldData.java | 4 -- .../plain/AbstractIndexOrdinalsFieldData.java | 26 +++++----- .../plain/AbstractLeafOrdinalsFieldData.java | 3 -- .../plain/BinaryDVLeafFieldData.java | 5 -- .../fielddata/plain/BinaryIndexFieldData.java | 2 +- .../plain/BytesBinaryIndexFieldData.java | 2 +- .../plain/ConstantIndexFieldData.java | 3 -- .../FormattedSortedNumericDocValues.java | 40 +++++++++++++++ .../plain/LatLonPointDVLeafFieldData.java | 5 -- .../plain/LatLonPointIndexFieldData.java | 2 +- .../fielddata/plain/LeafDoubleFieldData.java | 3 -- .../fielddata/plain/LeafLongFieldData.java | 23 +-------- .../plain/PagedBytesLeafFieldData.java | 3 -- .../plain/SortedDoublesIndexFieldData.java | 2 +- .../plain/SortedNumericIndexFieldData.java | 23 ++------- .../plain/SortedSetBytesLeafFieldData.java | 3 -- .../plain/StringBinaryIndexFieldData.java | 2 +- .../mapper/vectors/VectorDVLeafFieldData.java | 4 -- .../elasticsearch/search/MultiValueMode.java | 51 ++++++++----------- .../KeyedFlattenedLeafFieldDataTests.java | 4 -- .../functionscore/FunctionScoreTests.java | 4 -- .../mapper/HistogramFieldMapper.java | 4 -- .../AggregateDoubleMetricFieldMapper.java | 2 - .../CountedKeywordFieldMapper.java | 4 -- .../UnsignedLongLeafFieldData.java | 19 +------ .../index/fielddata/LeafShapeFieldData.java | 3 -- .../plain/CartesianPointDVLeafFieldData.java | 5 -- .../CartesianShapeDVAtomicShapeFieldData.java | 5 -- .../plain/GeoShapeScriptFieldData.java | 4 -- .../LatLonShapeDVAtomicShapeFieldData.java | 5 -- .../support/CartesianPointValuesSource.java | 31 ----------- 63 files changed, 140 insertions(+), 311 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/fielddata/plain/FormattedSortedNumericDocValues.java diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/ES87TSDBDocValuesConsumer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/ES87TSDBDocValuesConsumer.java index 7e5494007b369..71d9768ac5ff7 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/ES87TSDBDocValuesConsumer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/ES87TSDBDocValuesConsumer.java @@ -533,7 +533,7 @@ public SortedNumericDocValues getSortedNumeric(FieldInfo field) throws IOExcepti int i, docValueCount; @Override - public long nextValue() throws IOException { + public long nextValue() { return ords[i++]; } @@ -543,7 +543,7 @@ public int docValueCount() { } @Override - public boolean advanceExact(int target) throws IOException { + public boolean advanceExact(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractBinaryDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractBinaryDocValues.java index 597fabc5418f2..d46f8a3f20a4f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractBinaryDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractBinaryDocValues.java @@ -10,13 +10,9 @@ package org.elasticsearch.index.fielddata; import org.apache.lucene.index.BinaryDocValues; -import org.apache.lucene.search.DocIdSetIterator; - -import java.io.IOException; /** - * Base implementation that throws an {@link IOException} for the - * {@link DocIdSetIterator} APIs. This impl is safe to use for sorting and + * Base implementation. This impl is safe to use for sorting and * aggregations, which only use {@link #advanceExact(int)} and * {@link #binaryValue()}. */ @@ -28,12 +24,12 @@ public int docID() { } @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException(); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractNumericDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractNumericDocValues.java index e6925dde709b8..fca1adc137b71 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractNumericDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractNumericDocValues.java @@ -23,12 +23,12 @@ public abstract class AbstractNumericDocValues extends NumericDocValues { @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException(); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedDocValues.java index e5e75cae041e4..05bb9a8fe74c4 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedDocValues.java @@ -23,12 +23,12 @@ public abstract class AbstractSortedDocValues extends SortedDocValues { @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException(); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedNumericDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedNumericDocValues.java index a1c3a054fdd27..4c17715ec55f8 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedNumericDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedNumericDocValues.java @@ -33,7 +33,7 @@ public int nextDoc() throws IOException { } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedSetDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedSetDocValues.java index 90d577c21568e..c040a2e8dc2d0 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedSetDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortedSetDocValues.java @@ -28,12 +28,12 @@ public int docID() { } @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException(); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortingNumericDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortingNumericDocValues.java index 2684b40f4e442..076b2fecf4bef 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortingNumericDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/AbstractSortingNumericDocValues.java @@ -36,12 +36,12 @@ public int docID() { } @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException(); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/BinaryScriptFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/BinaryScriptFieldData.java index b618268d9a5b0..1a612bbe724f9 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/BinaryScriptFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/BinaryScriptFieldData.java @@ -66,9 +66,5 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() { - - } } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java index 3b8cc9956442a..742e0a0f95078 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java @@ -43,7 +43,7 @@ public DoubleScriptFieldData build(IndexFieldDataCache cache, CircuitBreakerServ } private final String fieldName; - DoubleFieldScript.LeafFactory leafFactory; + final DoubleFieldScript.LeafFactory leafFactory; private final ToScriptFieldFactory toScriptFieldFactory; private DoubleScriptFieldData( diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java index b090354b08459..6c8d52ac18280 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java @@ -42,12 +42,12 @@ public static SortedBinaryDocValues emptySortedBinary() { public static NumericDoubleValues emptyNumericDouble() { return new NumericDoubleValues() { @Override - public boolean advanceExact(int doc) throws IOException { + public boolean advanceExact(int doc) { return false; } @Override - public double doubleValue() throws IOException { + public double doubleValue() { throw new UnsupportedOperationException(); } @@ -243,14 +243,6 @@ public static BinaryDocValues unwrapSingleton(SortedBinaryDocValues values) { return null; } - /** - * Returns whether the provided values *might* be multi-valued. There is no - * guarantee that this method will return {@code false} in the single-valued case. - */ - public static boolean isMultiValued(SortedSetDocValues values) { - return DocValues.unwrapSingleton(values) == null; - } - /** * Return a {@link String} representation of the provided values. That is * typically used for scripts or for the `map` execution mode of terms aggs. @@ -634,7 +626,7 @@ public boolean advanceExact(int target) throws IOException { } @Override - public long longValue() throws IOException { + public long longValue() { return value; } }; @@ -661,7 +653,7 @@ public boolean advanceExact(int target) throws IOException { } @Override - public double doubleValue() throws IOException { + public double doubleValue() { return value; } }; diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java index 25c28a99a600c..c9aed46377c3f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java @@ -67,6 +67,6 @@ public int docValueCount() { public long nextValue() { int lat = GeoEncodingUtils.encodeLatitude(script.lats()[cursor]); int lon = GeoEncodingUtils.encodeLongitude(script.lons()[cursor++]); - return Long.valueOf((((long) lat) << 32) | (lon & 0xFFFFFFFFL)); + return (((long) lat) << 32) | (lon & 0xFFFFFFFFL); } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptFieldData.java index 1ffb0deca8a3d..e6bd6c3e59656 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptFieldData.java @@ -103,10 +103,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() { - - } }; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java index 902eba0f7d5d8..65eb724549bfc 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java @@ -20,7 +20,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.Closeable; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -90,7 +89,6 @@ public synchronized void clearField(final String fieldName) { * Returns fielddata for the provided field type, given the provided fully qualified index name, while also making * a {@link SearchLookup} supplier available that is required for runtime fields. */ - @SuppressWarnings("unchecked") public > IFD getForField(MappedFieldType fieldType, FieldDataContext fieldDataContext) { return getFromBuilder(fieldType, fieldType.fielddataBuilder(fieldDataContext)); } @@ -134,7 +132,7 @@ public void setListener(IndexFieldDataCache.Listener listener) { } @Override - public void close() throws IOException { + public void close() { clear(); } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index e1e8dad62daed..289f1dd6abd25 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -198,20 +198,16 @@ private XFieldComparatorSource comparatorSource( MultiValueMode sortMode, Nested nested ) { - switch (targetNumericType) { - case HALF_FLOAT: - case FLOAT: - return new FloatValuesComparatorSource(this, missingValue, sortMode, nested); - case DOUBLE: - return new DoubleValuesComparatorSource(this, missingValue, sortMode, nested); - case DATE: - return dateComparatorSource(missingValue, sortMode, nested); - case DATE_NANOSECONDS: - return dateNanosComparatorSource(missingValue, sortMode, nested); - default: + return switch (targetNumericType) { + case HALF_FLOAT, FLOAT -> new FloatValuesComparatorSource(this, missingValue, sortMode, nested); + case DOUBLE -> new DoubleValuesComparatorSource(this, missingValue, sortMode, nested); + case DATE -> dateComparatorSource(missingValue, sortMode, nested); + case DATE_NANOSECONDS -> dateNanosComparatorSource(missingValue, sortMode, nested); + default -> { assert targetNumericType.isFloatingPoint() == false; - return new LongValuesComparatorSource(this, missingValue, sortMode, nested, targetNumericType); - } + yield new LongValuesComparatorSource(this, missingValue, sortMode, nested, targetNumericType); + } + }; } protected XFieldComparatorSource dateComparatorSource(@Nullable Object missingValue, MultiValueMode sortMode, Nested nested) { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java index 2c48eb28e7fe5..e42db267920ed 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java @@ -32,6 +32,9 @@ public interface LeafFieldData extends Accountable, Releasable { */ SortedBinaryDocValues getBytesValues(); + @Override + default void close() {} + /** * Return a formatted representation of the values */ diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/LongScriptFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/LongScriptFieldData.java index 2ad11aa771a5c..7f05ca0e50f92 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/LongScriptFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/LongScriptFieldData.java @@ -20,8 +20,6 @@ import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSourceType; -import java.io.IOException; - public final class LongScriptFieldData extends IndexNumericFieldData { public static class Builder implements IndexFieldData.Builder { @@ -79,7 +77,7 @@ public LongScriptLeafFieldData load(LeafReaderContext context) { } @Override - public LongScriptLeafFieldData loadDirect(LeafReaderContext context) throws IOException { + public LongScriptLeafFieldData loadDirect(LeafReaderContext context) { return new LongScriptLeafFieldData(new LongScriptDocValues(leafFactory.newInstance(context)), toScriptFieldFactory); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/MultiGeoPointValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/MultiGeoPointValues.java index 449b3b9ced777..06deda44dc364 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/MultiGeoPointValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/MultiGeoPointValues.java @@ -45,7 +45,6 @@ public GeoPoint nextValue() throws IOException { /** * Returns a single-valued view of the {@link MultiPointValues} if possible, otherwise null. */ - @Override protected GeoPointValues getPointValues() { final NumericDocValues singleton = DocValues.unwrapSingleton(numericValues); return singleton != null ? new GeoPointValues(singleton) : null; diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/MultiPointValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/MultiPointValues.java index f99683ebe0831..854f06d9b606a 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/MultiPointValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/MultiPointValues.java @@ -53,8 +53,4 @@ public int docValueCount() { */ public abstract T nextValue() throws IOException; - /** - * Returns a single-valued view of the {@link MultiPointValues} if possible, otherwise null. - */ - protected abstract PointValues getPointValues(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/RamAccountingTermsEnum.java b/server/src/main/java/org/elasticsearch/index/fielddata/RamAccountingTermsEnum.java index 15817ae513146..14a1f287eb500 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/RamAccountingTermsEnum.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/RamAccountingTermsEnum.java @@ -52,7 +52,7 @@ public RamAccountingTermsEnum( * Always accept the term. */ @Override - protected AcceptStatus accept(BytesRef term) throws IOException { + protected AcceptStatus accept(BytesRef term) { return AcceptStatus.YES; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SortingNumericDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/SortingNumericDocValues.java index d5933c7fab4d2..02f5de7e3c8ca 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SortingNumericDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SortingNumericDocValues.java @@ -25,7 +25,7 @@ public abstract class SortingNumericDocValues extends SortedNumericDocValues { protected long[] values; protected int valuesCursor; private final Sorter sorter; - private LongConsumer circuitBreakerConsumer; + private final LongConsumer circuitBreakerConsumer; protected SortingNumericDocValues() { this(l -> {}); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherIndexFieldData.java index d9f6cefeba38c..bb4eb3f0bebce 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherIndexFieldData.java @@ -132,11 +132,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() { - - } - @Override public SortedBinaryDocValues getBytesValues() { throw new IllegalArgumentException("not supported for source fallback"); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherMultiGeoPointIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherMultiGeoPointIndexFieldData.java index a4ff63ae1e04e..bfb5c58cac3af 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherMultiGeoPointIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherMultiGeoPointIndexFieldData.java @@ -61,7 +61,7 @@ protected SourceValueFetcherMultiGeoPointIndexFieldData( } @Override - public SourceValueFetcherMultiGeoPointLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public SourceValueFetcherMultiGeoPointLeafFieldData loadDirect(LeafReaderContext context) { return new SourceValueFetcherMultiGeoPointLeafFieldData(toScriptFieldFactory, context, valueFetcher, sourceProvider); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBinaryIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBinaryIndexFieldData.java index 3f26a6c71197d..e24a6c5251f8f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBinaryIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBinaryIndexFieldData.java @@ -62,7 +62,7 @@ protected SourceValueFetcherSortedBinaryIndexFieldData( } @Override - public SourceValueFetcherSortedBinaryLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public SourceValueFetcherSortedBinaryLeafFieldData loadDirect(LeafReaderContext context) { return new SourceValueFetcherSortedBinaryLeafFieldData(toScriptFieldFactory, context, valueFetcher, sourceProvider); } @@ -127,7 +127,7 @@ public int docValueCount() { } @Override - public BytesRef nextValue() throws IOException { + public BytesRef nextValue() { assert iterator.hasNext(); return iterator.next(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBooleanIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBooleanIndexFieldData.java index c28890cb82671..56937d47b794d 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBooleanIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedBooleanIndexFieldData.java @@ -59,7 +59,7 @@ protected SourceValueFetcherSortedBooleanIndexFieldData( } @Override - public SourceValueFetcherLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public SourceValueFetcherLeafFieldData loadDirect(LeafReaderContext context) { return new SourceValueFetcherSortedBooleanLeafFieldData(toScriptFieldFactory, context, valueFetcher, sourceProvider); } @@ -129,7 +129,7 @@ public int docValueCount() { } @Override - public long nextValue() throws IOException { + public long nextValue() { assert iteratorIndex < trueCount + falseCount; return iteratorIndex++ < falseCount ? 0L : 1L; } @@ -140,12 +140,12 @@ public int docID() { } @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException("not supported for source fallback"); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException("not supported for source fallback"); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedDoubleIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedDoubleIndexFieldData.java index 2df4917390354..f4bc2216217a7 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedDoubleIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedDoubleIndexFieldData.java @@ -61,7 +61,7 @@ protected SourceValueFetcherSortedDoubleIndexFieldData( } @Override - public SourceValueFetcherLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public SourceValueFetcherLeafFieldData loadDirect(LeafReaderContext context) { return new SourceValueFetcherSortedDoubleLeafFieldData(toScriptFieldFactory, context, valueFetcher, sourceProvider); } @@ -128,7 +128,7 @@ public int docValueCount() { } @Override - public double nextValue() throws IOException { + public double nextValue() { assert iterator.hasNext(); return iterator.next(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedNumericIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedNumericIndexFieldData.java index cf4cbf93b8e8d..ce1dff33e80ce 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedNumericIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/SourceValueFetcherSortedNumericIndexFieldData.java @@ -62,7 +62,7 @@ protected SourceValueFetcherSortedNumericIndexFieldData( } @Override - public SourceValueFetcherSortedNumericLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public SourceValueFetcherSortedNumericLeafFieldData loadDirect(LeafReaderContext context) { return new SourceValueFetcherSortedNumericLeafFieldData(toScriptFieldFactory, context, valueFetcher, sourceProvider); } @@ -129,7 +129,7 @@ public int docValueCount() { } @Override - public long nextValue() throws IOException { + public long nextValue() { assert iterator.hasNext(); return iterator.next(); } @@ -140,12 +140,12 @@ public int docID() { } @Override - public int nextDoc() throws IOException { + public int nextDoc() { throw new UnsupportedOperationException("not supported for source fallback"); } @Override - public int advance(int target) throws IOException { + public int advance(int target) { throw new UnsupportedOperationException("not supported for source fallback"); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldIndexFieldData.java index 6d3d5b988c9f9..a0c4efe9ef6c1 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldIndexFieldData.java @@ -104,9 +104,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() {} - @Override public SortedBinaryDocValues getBytesValues() { throw new IllegalArgumentException("not supported for source fallback"); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldSortedBinaryIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldSortedBinaryIndexFieldData.java index 69e348956822e..550833c396430 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldSortedBinaryIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/StoredFieldSortedBinaryIndexFieldData.java @@ -75,7 +75,7 @@ public int docValueCount() { } @Override - public BytesRef nextValue() throws IOException { + public BytesRef nextValue() { assert current < docValueCount; return sorted.get(current++); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index 459075cfcc5b1..f1e4ba95e76fe 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java @@ -54,7 +54,7 @@ public SortField.Type reducedType() { return SortField.Type.FLOAT; } - private NumericDoubleValues getNumericDocValues(LeafReaderContext context, float missingValue) throws IOException { + private NumericDoubleValues getNumericDocValues(LeafReaderContext context, double missingValue) throws IOException { final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues(); if (nested == null) { return FieldData.replaceMissing(sortMode.select(values), missingValue); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsBuilder.java b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsBuilder.java index e857a715ee657..def1c40e1e6f0 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsBuilder.java @@ -111,8 +111,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() {} }; subs[i] = atomicFD[i].getOrdinalsValues(); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsIndexFieldData.java index 2ffc8527b179a..5b3e523a10f41 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/GlobalOrdinalsIndexFieldData.java @@ -76,7 +76,7 @@ public IndexOrdinalsFieldData newConsumer(DirectoryReader source) { } @Override - public LeafOrdinalsFieldData loadDirect(LeafReaderContext context) throws Exception { + public LeafOrdinalsFieldData loadDirect(LeafReaderContext context) { throw new IllegalStateException("loadDirect(LeafReaderContext) should not be called in this context"); } @@ -86,7 +86,7 @@ public IndexOrdinalsFieldData loadGlobal(DirectoryReader indexReader) { } @Override - public IndexOrdinalsFieldData loadGlobalDirect(DirectoryReader indexReader) throws Exception { + public IndexOrdinalsFieldData loadGlobalDirect(DirectoryReader indexReader) { return this; } @@ -179,7 +179,7 @@ private TermsEnum[] getOrLoadTermsEnums() { } @Override - public LeafOrdinalsFieldData loadDirect(LeafReaderContext context) throws Exception { + public LeafOrdinalsFieldData loadDirect(LeafReaderContext context) { return load(context); } @@ -189,7 +189,7 @@ public IndexOrdinalsFieldData loadGlobal(DirectoryReader indexReader) { } @Override - public IndexOrdinalsFieldData loadGlobalDirect(DirectoryReader indexReader) throws Exception { + public IndexOrdinalsFieldData loadGlobalDirect(DirectoryReader indexReader) { return this; } @@ -258,8 +258,6 @@ public Collection getChildResources() { return segmentAfd[context.ord].getChildResources(); } - @Override - public void close() {} }; } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java index d39e811ae49f4..e1b66ec379d8e 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ordinals/OrdinalsBuilder.java @@ -16,7 +16,6 @@ import org.apache.lucene.util.packed.PagedGrowableWriter; import java.io.Closeable; -import java.io.IOException; import java.util.Arrays; /** @@ -369,7 +368,7 @@ public int maxDoc() { * Closes this builder and release all resources. */ @Override - public void close() throws IOException { + public void close() { ordinals = null; } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractBinaryDVLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractBinaryDVLeafFieldData.java index d609796d58c83..63fa0e21c6cb6 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractBinaryDVLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractBinaryDVLeafFieldData.java @@ -68,8 +68,4 @@ public BytesRef nextValue() throws IOException { }; } - @Override - public void close() { - // no-op - } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractIndexOrdinalsFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractIndexOrdinalsFieldData.java index 5227a4bed1f1c..5038117a5d554 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractIndexOrdinalsFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractIndexOrdinalsFieldData.java @@ -83,13 +83,7 @@ public LeafOrdinalsFieldData load(LeafReaderContext context) { try { return cache.load(context, this); } catch (Exception e) { - if (e instanceof ElasticsearchException) { - throw (ElasticsearchException) e; - } else if (e instanceof ExecutionException && e.getCause() instanceof ElasticsearchException) { - throw (ElasticsearchException) e.getCause(); - } else { - throw new ElasticsearchException(e); - } + throw handleCacheLoadException(e); } } @@ -131,13 +125,7 @@ private IndexOrdinalsFieldData loadGlobalInternal(DirectoryReader indexReader) { try { return cache.load(indexReader, this); } catch (Exception e) { - if (e instanceof ElasticsearchException) { - throw (ElasticsearchException) e; - } else if (e instanceof ExecutionException && e.getCause() instanceof ElasticsearchException) { - throw (ElasticsearchException) e.getCause(); - } else { - throw new ElasticsearchException(e); - } + throw handleCacheLoadException(e); } } @@ -157,6 +145,16 @@ public boolean supportsGlobalOrdinalsMapping() { return false; } + private static ElasticsearchException handleCacheLoadException(Exception e) { + if (e instanceof ElasticsearchException ese) { + return ese; + } + if (e instanceof ExecutionException && e.getCause() instanceof ElasticsearchException ese) { + throw ese; + } + throw new ElasticsearchException(e); + } + /** * A {@code PerValueEstimator} is a sub-class that can be used to estimate * the memory overhead for loading the data. Each field data diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafOrdinalsFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafOrdinalsFieldData.java index 8dc30f2646a8a..d1280040f085f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafOrdinalsFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafOrdinalsFieldData.java @@ -43,9 +43,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() {} - @Override public SortedSetDocValues getOrdinalsValues() { return DocValues.emptySortedSet(); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryDVLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryDVLeafFieldData.java index 22bceca37fb6c..54f1ecac89f8e 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryDVLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryDVLeafFieldData.java @@ -47,11 +47,6 @@ public DocValuesScriptFieldFactory getScriptFieldFactory(String name) { return new DelegateDocValuesField(new ScriptDocValues.Strings(new ScriptDocValues.StringsSupplier(getBytesValues())), name); } - @Override - public void close() { - // no-op - } - @Override public long ramBytesUsed() { return 0; // unknown diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryIndexFieldData.java index f414f800bf809..0d90ef9a41071 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryIndexFieldData.java @@ -65,7 +65,7 @@ public BinaryDVLeafFieldData load(LeafReaderContext context) { } @Override - public BinaryDVLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public BinaryDVLeafFieldData loadDirect(LeafReaderContext context) { return load(context); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BytesBinaryIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BytesBinaryIndexFieldData.java index 2f0f7254f1265..f5d4bf837f062 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/BytesBinaryIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/BytesBinaryIndexFieldData.java @@ -75,7 +75,7 @@ public BytesBinaryDVLeafFieldData load(LeafReaderContext context) { } @Override - public BytesBinaryDVLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public BytesBinaryDVLeafFieldData loadDirect(LeafReaderContext context) { return load(context); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/ConstantIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/ConstantIndexFieldData.java index 85adc2b5c5012..8e15e6fc8e7f1 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/ConstantIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/ConstantIndexFieldData.java @@ -113,9 +113,6 @@ public int docID() { return DocValues.singleton(sortedValues); } - @Override - public void close() {} - } private final ConstantLeafFieldData atomicFieldData; diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/FormattedSortedNumericDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/FormattedSortedNumericDocValues.java new file mode 100644 index 0000000000000..da64520c7c86c --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/FormattedSortedNumericDocValues.java @@ -0,0 +1,40 @@ +/* + * 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.index.fielddata.plain; + +import org.apache.lucene.index.SortedNumericDocValues; +import org.elasticsearch.index.fielddata.FormattedDocValues; +import org.elasticsearch.search.DocValueFormat; + +import java.io.IOException; + +public final class FormattedSortedNumericDocValues implements FormattedDocValues { + private final SortedNumericDocValues values; + private final DocValueFormat format; + + public FormattedSortedNumericDocValues(SortedNumericDocValues values, DocValueFormat format) { + this.values = values; + this.format = format; + } + + @Override + public boolean advanceExact(int docId) throws IOException { + return values.advanceExact(docId); + } + + @Override + public int docValueCount() { + return values.docValueCount(); + } + + @Override + public Object nextValue() throws IOException { + return format.format(values.nextValue()); + } +} diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointDVLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointDVLeafFieldData.java index f303c61c86b6e..64e6dd223caf4 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointDVLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointDVLeafFieldData.java @@ -31,11 +31,6 @@ public long ramBytesUsed() { return 0; // not exposed by lucene } - @Override - public void close() { - // noop - } - @Override public SortedNumericDocValues getSortedNumericDocValues() { try { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointIndexFieldData.java index 40f934746126a..cbf7d76f24137 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LatLonPointIndexFieldData.java @@ -43,7 +43,7 @@ public LeafPointFieldData load(LeafReaderContext context) { } @Override - public LeafPointFieldData loadDirect(LeafReaderContext context) throws Exception { + public LeafPointFieldData loadDirect(LeafReaderContext context) { return load(context); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafDoubleFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafDoubleFieldData.java index 144dc9220110c..e870f7af22562 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafDoubleFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafDoubleFieldData.java @@ -60,7 +60,4 @@ public Object nextValue() throws IOException { }; } - @Override - public void close() {} - } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java index 5c849fa1efaa6..bad40d2c51943 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java @@ -9,7 +9,6 @@ package org.elasticsearch.index.fielddata.plain; -import org.apache.lucene.index.SortedNumericDocValues; import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.FormattedDocValues; import org.elasticsearch.index.fielddata.LeafNumericFieldData; @@ -17,8 +16,6 @@ import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; import org.elasticsearch.search.DocValueFormat; -import java.io.IOException; - /** * Specialization of {@link LeafNumericFieldData} for integers. */ @@ -47,25 +44,7 @@ public final SortedNumericDoubleValues getDoubleValues() { @Override public FormattedDocValues getFormattedValues(DocValueFormat format) { - SortedNumericDocValues values = getLongValues(); - return new FormattedDocValues() { - @Override - public boolean advanceExact(int docId) throws IOException { - return values.advanceExact(docId); - } - - @Override - public int docValueCount() throws IOException { - return values.docValueCount(); - } - - @Override - public Object nextValue() throws IOException { - return format.format(values.nextValue()); - } - }; + return new FormattedSortedNumericDocValues(getLongValues(), format); } - @Override - public void close() {} } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/PagedBytesLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/PagedBytesLeafFieldData.java index 1dc2203e04c7d..d0b77c70eb0cf 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/PagedBytesLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/PagedBytesLeafFieldData.java @@ -38,9 +38,6 @@ public PagedBytesLeafFieldData( this.ordinals = ordinals; } - @Override - public void close() {} - @Override public long ramBytesUsed() { long size = ordinals.ramBytesUsed(); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedDoublesIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedDoublesIndexFieldData.java index 151ecb7d376e3..ae20100f4c1f7 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedDoublesIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedDoublesIndexFieldData.java @@ -112,7 +112,7 @@ public NumericType getNumericType() { } @Override - public LeafNumericFieldData loadDirect(LeafReaderContext context) throws Exception { + public LeafNumericFieldData loadDirect(LeafReaderContext context) { return load(context); } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java index d72ca60bb9786..98fd4e27f9422 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericIndexFieldData.java @@ -153,7 +153,7 @@ public NumericType getNumericType() { } @Override - public LeafNumericFieldData loadDirect(LeafReaderContext context) throws Exception { + public LeafNumericFieldData loadDirect(LeafReaderContext context) { return load(context); } @@ -206,25 +206,9 @@ public DocValuesScriptFieldFactory getScriptFieldFactory(String name) { @Override public FormattedDocValues getFormattedValues(DocValueFormat format) { - DocValueFormat nanosFormat = DocValueFormat.withNanosecondResolution(format); - SortedNumericDocValues values = getLongValuesAsNanos(); - return new FormattedDocValues() { - @Override - public boolean advanceExact(int docId) throws IOException { - return values.advanceExact(docId); - } - - @Override - public int docValueCount() throws IOException { - return values.docValueCount(); - } - - @Override - public Object nextValue() throws IOException { - return nanosFormat.format(values.nextValue()); - } - }; + return new FormattedSortedNumericDocValues(getLongValuesAsNanos(), DocValueFormat.withNanosecondResolution(format)); } + } /** @@ -264,4 +248,5 @@ public DocValuesScriptFieldFactory getScriptFieldFactory(String name) { return toScriptFieldFactory.getScriptFieldFactory(getLongValues(), name); } } + } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedSetBytesLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedSetBytesLeafFieldData.java index 9b08c319ecd98..d4682e680a54a 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedSetBytesLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/SortedSetBytesLeafFieldData.java @@ -40,9 +40,6 @@ public SortedSetDocValues getOrdinalsValues() { } } - @Override - public void close() {} - @Override public long ramBytesUsed() { return 0; // unknown diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/StringBinaryIndexFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/StringBinaryIndexFieldData.java index 954f2db34f4ba..7c8b058cb01f1 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/StringBinaryIndexFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/StringBinaryIndexFieldData.java @@ -82,7 +82,7 @@ public BucketedSort newBucketedSort( } @Override - public StringBinaryDVLeafFieldData loadDirect(LeafReaderContext context) throws Exception { + public StringBinaryDVLeafFieldData loadDirect(LeafReaderContext context) { return load(context); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorDVLeafFieldData.java b/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorDVLeafFieldData.java index 3f9ba85bee1bb..23d2c4b554d85 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorDVLeafFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorDVLeafFieldData.java @@ -76,8 +76,4 @@ public DocValuesScriptFieldFactory getScriptFieldFactory(String name) { } } - @Override - public void close() { - // no-op - } } diff --git a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java index 7f499da9c0fc8..49480816bbbb1 100644 --- a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java +++ b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java @@ -593,12 +593,7 @@ public boolean advanceExact(int parentDoc) throws IOException { return true; } final int prevParentDoc = parentDocs.prevSetBit(parentDoc - 1); - final int firstChildDoc; - if (childDocs.docID() > prevParentDoc) { - firstChildDoc = childDocs.docID(); - } else { - firstChildDoc = childDocs.advance(prevParentDoc + 1); - } + final int firstChildDoc = getFirstChildDoc(prevParentDoc, childDocs); lastSeenParentDoc = parentDoc; lastEmittedValue = pick(values, missingValue, childDocs, firstChildDoc, parentDoc, maxChildren); @@ -700,12 +695,7 @@ public boolean advanceExact(int parentDoc) throws IOException { return true; } final int prevParentDoc = parentDocs.prevSetBit(parentDoc - 1); - final int firstChildDoc; - if (childDocs.docID() > prevParentDoc) { - firstChildDoc = childDocs.docID(); - } else { - firstChildDoc = childDocs.advance(prevParentDoc + 1); - } + final int firstChildDoc = getFirstChildDoc(prevParentDoc, childDocs); lastSeenParentDoc = parentDoc; lastEmittedValue = pick(values, missingValue, childDocs, firstChildDoc, parentDoc, maxChildren); @@ -713,12 +703,22 @@ public boolean advanceExact(int parentDoc) throws IOException { } @Override - public double doubleValue() throws IOException { + public double doubleValue() { return lastEmittedValue; } }; } + private static int getFirstChildDoc(int prevParentDoc, DocIdSetIterator childDocs) throws IOException { + final int firstChildDoc; + if (childDocs.docID() > prevParentDoc) { + firstChildDoc = childDocs.docID(); + } else { + firstChildDoc = childDocs.advance(prevParentDoc + 1); + } + return firstChildDoc; + } + protected double pick( SortedNumericDoubleValues values, double missingValue, @@ -754,7 +754,7 @@ public boolean advanceExact(int target) throws IOException { } @Override - public BytesRef binaryValue() throws IOException { + public BytesRef binaryValue() { return this.value; } }; @@ -768,14 +768,13 @@ public boolean advanceExact(int target) throws IOException { if (values.advanceExact(target)) { value = pick(values); return true; - } else { - value = missingValue; - return missingValue != null; } + value = missingValue; + return missingValue != null; } @Override - public BytesRef binaryValue() throws IOException { + public BytesRef binaryValue() { return value; } }; @@ -825,12 +824,7 @@ public boolean advanceExact(int parentDoc) throws IOException { } final int prevParentDoc = parentDocs.prevSetBit(parentDoc - 1); - final int firstChildDoc; - if (childDocs.docID() > prevParentDoc) { - firstChildDoc = childDocs.docID(); - } else { - firstChildDoc = childDocs.advance(prevParentDoc + 1); - } + final int firstChildDoc = getFirstChildDoc(prevParentDoc, childDocs); lastSeenParentDoc = parentDoc; lastEmittedValue = pick(selectedValues, builder, childDocs, firstChildDoc, parentDoc, maxChildren); @@ -841,7 +835,7 @@ public boolean advanceExact(int parentDoc) throws IOException { } @Override - public BytesRef binaryValue() throws IOException { + public BytesRef binaryValue() { return lastEmittedValue; } }; @@ -964,12 +958,7 @@ public boolean advanceExact(int parentDoc) throws IOException { } final int prevParentDoc = parentDocs.prevSetBit(parentDoc - 1); - final int firstChildDoc; - if (childDocs.docID() > prevParentDoc) { - firstChildDoc = childDocs.docID(); - } else { - firstChildDoc = childDocs.advance(prevParentDoc + 1); - } + final int firstChildDoc = getFirstChildDoc(prevParentDoc, childDocs); docID = lastSeenParentDoc = parentDoc; lastEmittedOrd = pick(selectedValues, childDocs, firstChildDoc, parentDoc, maxChildren); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedLeafFieldDataTests.java b/server/src/test/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedLeafFieldDataTests.java index cadc836f7c4ea..f494af259c504 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedLeafFieldDataTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedLeafFieldDataTests.java @@ -165,10 +165,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() { - // Nothing to do. - } } private static final ToScriptFieldFactory MOCK_TO_SCRIPT_FIELD = (dv, n) -> new DelegateDocValuesField( diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java index e9a47668faca5..946dbea45b4ba 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java @@ -126,8 +126,6 @@ public Collection getChildResources() { throw new UnsupportedOperationException(UNSUPPORTED); } - @Override - public void close() {} }; } @@ -229,8 +227,6 @@ public Collection getChildResources() { throw new UnsupportedOperationException(UNSUPPORTED); } - @Override - public void close() {} }; } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 0940b3ef96ddd..d597d7b59f240 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -233,10 +233,6 @@ public long ramBytesUsed() { return 0; // Unknown } - @Override - public void close() { - - } }; } diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java index b1f38191ba7b3..6944f91042311 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java @@ -469,8 +469,6 @@ public long ramBytesUsed() { return 0; // Unknown } - @Override - public void close() {} }; } diff --git a/x-pack/plugin/mapper-counted-keyword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java b/x-pack/plugin/mapper-counted-keyword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java index e728a74955f30..e52237f4d507e 100644 --- a/x-pack/plugin/mapper-counted-keyword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java +++ b/x-pack/plugin/mapper-counted-keyword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java @@ -155,10 +155,6 @@ public long ramBytesUsed() { return 0; // Unknown } - @Override - public void close() { - // nothing to close - } }; } diff --git a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java index 0c98c5b8bd5a7..00c60987f439d 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java +++ b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java @@ -16,6 +16,7 @@ import org.elasticsearch.index.fielddata.NumericDoubleValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; +import org.elasticsearch.index.fielddata.plain.FormattedSortedNumericDocValues; import org.elasticsearch.script.field.DocValuesScriptFieldFactory; import org.elasticsearch.script.field.ToScriptFieldFactory; import org.elasticsearch.search.DocValueFormat; @@ -97,23 +98,7 @@ public void close() { @Override public FormattedDocValues getFormattedValues(DocValueFormat format) { - SortedNumericDocValues values = getLongValues(); - return new FormattedDocValues() { - @Override - public boolean advanceExact(int docId) throws IOException { - return values.advanceExact(docId); - } - - @Override - public int docValueCount() { - return values.docValueCount(); - } - - @Override - public Object nextValue() throws IOException { - return format.format(values.nextValue()); - } - }; + return new FormattedSortedNumericDocValues(getLongValues(), format); } static double convertUnsignedLongToDouble(long value) { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/LeafShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/LeafShapeFieldData.java index f3a3b4d2f3f27..8be44242886c9 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/LeafShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/LeafShapeFieldData.java @@ -38,9 +38,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() {} - @Override public T getShapeValues() { return emptyValues; diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianPointDVLeafFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianPointDVLeafFieldData.java index 9cbbe04f56cd5..b8bd5e82fef3b 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianPointDVLeafFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianPointDVLeafFieldData.java @@ -34,11 +34,6 @@ public long ramBytesUsed() { return 0; // not exposed by lucene } - @Override - public void close() { - // noop - } - @Override public SortedNumericDocValues getSortedNumericDocValues() { try { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianShapeDVAtomicShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianShapeDVAtomicShapeFieldData.java index 34e6c569a4e68..42eb1925601cd 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianShapeDVAtomicShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/CartesianShapeDVAtomicShapeFieldData.java @@ -37,11 +37,6 @@ public long ramBytesUsed() { return 0; // not exposed by lucene } - @Override - public void close() { - // noop - } - @Override public CartesianShapeValues getShapeValues() { try { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java index 4790924412cd7..bf4202466eb41 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java @@ -70,10 +70,6 @@ public long ramBytesUsed() { return 0; } - @Override - public void close() { - - } }; } } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java index 8319da1306be6..a1aedb07825f3 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java @@ -33,11 +33,6 @@ public long ramBytesUsed() { return 0; // not exposed by lucene } - @Override - public void close() { - // noop - } - @Override public GeoShapeValues getShapeValues() { try { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/CartesianPointValuesSource.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/CartesianPointValuesSource.java index 5d916b6c5cead..68d5e3bd04421 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/CartesianPointValuesSource.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/CartesianPointValuesSource.java @@ -9,18 +9,14 @@ import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.index.SortedNumericDocValues; import org.elasticsearch.common.Rounding; import org.elasticsearch.index.fielddata.DocValueBits; import org.elasticsearch.index.fielddata.MultiPointValues; -import org.elasticsearch.index.fielddata.PointValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.search.aggregations.AggregationErrors; import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.xcontent.ToXContentFragment; -import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xpack.spatial.common.CartesianPoint; import org.elasticsearch.xpack.spatial.index.fielddata.IndexCartesianPointFieldData; @@ -73,33 +69,6 @@ public CartesianPoint nextValue() throws IOException { return point.resetFromEncoded(numericValues.nextValue()); } - @Override - protected PointValues getPointValues() { - final NumericDocValues singleton = DocValues.unwrapSingleton(numericValues); - return singleton != null ? new CartesianPointValues(singleton) : null; - } - } - - public static final class CartesianPointValues extends PointValues { - - private final CartesianPoint point = new CartesianPoint(); - - CartesianPointValues(NumericDocValues values) { - super(values); - } - - @Override - public CartesianPoint pointValue() throws IOException { - return point.resetFromEncoded(values.longValue()); - } - } - - public static final class CartesianPointValue implements ToXContentFragment { - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return null; - } } /** From 6e79b832d40887a5135d5be7a955183063a0d1fc Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 07:26:15 +1000 Subject: [PATCH 36/46] Mute org.elasticsearch.integration.KibanaUserRoleIntegTests testFieldMappings #113341 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 7f9f44a07f66a..d10d8c1e1a27b 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -321,6 +321,9 @@ tests: - 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 +- class: org.elasticsearch.integration.KibanaUserRoleIntegTests + method: testFieldMappings + issue: https://github.com/elastic/elasticsearch/issues/113341 # Examples: # From 114880af8acf484d8548b72b47789e4e1a74f4d5 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 14:44:48 +1000 Subject: [PATCH 37/46] Mute org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT test {yaml=reference/ccr/apis/follow/post-resume-follow/line_84} #113343 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index d10d8c1e1a27b..c0cabf9552bbb 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -324,6 +324,9 @@ tests: - class: org.elasticsearch.integration.KibanaUserRoleIntegTests method: testFieldMappings issue: https://github.com/elastic/elasticsearch/issues/113341 +- class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT + method: test {yaml=reference/ccr/apis/follow/post-resume-follow/line_84} + issue: https://github.com/elastic/elasticsearch/issues/113343 # Examples: # From 3d1d495a329757d3696a824128c046b53e03e9b3 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sun, 22 Sep 2024 15:36:20 +1000 Subject: [PATCH 38/46] Mute org.elasticsearch.integration.KibanaUserRoleIntegTests testSearchAndMSearch #113345 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index c0cabf9552bbb..a592a493423c0 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -327,6 +327,9 @@ tests: - class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT method: test {yaml=reference/ccr/apis/follow/post-resume-follow/line_84} issue: https://github.com/elastic/elasticsearch/issues/113343 +- class: org.elasticsearch.integration.KibanaUserRoleIntegTests + method: testSearchAndMSearch + issue: https://github.com/elastic/elasticsearch/issues/113345 # Examples: # From c21342a499c562c7badae51ac4542537d75c9c6d Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 23 Sep 2024 07:06:55 +0200 Subject: [PATCH 39/46] Added known issue entry for synthetic source bug. (#113269) Added known issue entry for synthetic source bug. Co-authored-by: Oleksandr Kolomiiets --- docs/reference/release-notes/8.15.0.asciidoc | 8 ++++++++ docs/reference/release-notes/8.15.1.asciidoc | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/reference/release-notes/8.15.0.asciidoc b/docs/reference/release-notes/8.15.0.asciidoc index 1496d7846a080..80c86c7079f0c 100644 --- a/docs/reference/release-notes/8.15.0.asciidoc +++ b/docs/reference/release-notes/8.15.0.asciidoc @@ -44,6 +44,14 @@ To work around this issue, you have a number of options: <> ** Change the default data view in Discover to a smaller set of indices and/or one with fewer mapping conflicts. +* Synthetic source bug. Synthetic source may fail generating the _source at runtime, causing failures in get APIs or +partial failures in the search APIs. The result is that for the affected documents the _source can't be retrieved. +There is no workaround and the only option to is to upgrade to 8.15.2 when released. ++ +If you use synthetic source then you may be affected by this bug if the following is true: +** If you have more fields then the `index.mapping.total_fields.limit` setting allows. +** If you use dynamic mappings and the `index.mapping.total_fields.ignore_dynamic_beyond_limit` setting is enabled. + [[breaking-8.15.0]] [float] === Breaking changes diff --git a/docs/reference/release-notes/8.15.1.asciidoc b/docs/reference/release-notes/8.15.1.asciidoc index e3bfaa18b6986..7c48f457e3b4e 100644 --- a/docs/reference/release-notes/8.15.1.asciidoc +++ b/docs/reference/release-notes/8.15.1.asciidoc @@ -24,13 +24,20 @@ To work around this issue, you have a number of options: <> ** Change the default data view in Discover to a smaller set of indices and/or one with fewer mapping conflicts. -* Index Stats, Node Stats and Cluster Stats API can return a null pointer exception if an index contains a `dense_vector` field +* Index Stats, Node Stats and Cluster Stats API can return a null pointer exception if an index contains a `dense_vector` field but there is an index segment that does not contain any documents with a dense vector field ({es-pull}112720[#112720]). Workarounds: ** If the affected index already contains documents with a dense vector field, force merge the index to a single segment. ** If the affected index does not already contain documents with a dense vector field, index a document with a dense vector field and then force merge to a single segment. ** If the affected index's `dense_vector` fields are unused, reindex without the `dense_vector` fields. +* Synthetic source bug. Synthetic source may fail generating the _source at runtime, causing failures in get APIs or +partial failures in the search APIs. The result is that for the affected documents the _source can't be retrieved. +There is no workaround and the only option to is to upgrade to 8.15.2 when released. ++ +If you use synthetic source then you may be affected by this bug if the following is true: +** If you have more fields then the `index.mapping.total_fields.limit` setting allows. +** If you use dynamic mappings and the `index.mapping.total_fields.ignore_dynamic_beyond_limit` setting is enabled. [[bug-8.15.1]] [float] From a20b5973d423ae0b145ae40cea18b2ade7fb692d Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 Sep 2024 07:33:02 +0100 Subject: [PATCH 40/46] Replace `DeleteIndexClusterStateUpdateRequest` with private class (#113288) No need to extend `IndicesClusterStateUpdateRequest` here, nor to attach the listener to the request. Instead we can encapsulate the cluster state update task directly instead. --- .../snapshots/ConcurrentSnapshotsIT.java | 21 ++--- .../DeleteIndexClusterStateUpdateRequest.java | 56 ------------- .../delete/TransportDeleteIndexAction.java | 15 ++-- .../metadata/MetadataDeleteIndexService.java | 81 +++++++++++++++---- .../MetadataDeleteIndexServiceTests.java | 7 +- 5 files changed, 92 insertions(+), 88 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexClusterStateUpdateRequest.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/ConcurrentSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/ConcurrentSnapshotsIT.java index 61a1168b128b9..de62c0152817a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/ConcurrentSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/ConcurrentSnapshotsIT.java @@ -18,7 +18,6 @@ import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexClusterStateUpdateRequest; import org.elasticsearch.action.support.ActionTestUtils; import org.elasticsearch.action.support.GroupedActionListener; import org.elasticsearch.action.support.PlainActionFuture; @@ -33,9 +32,7 @@ import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.common.util.concurrent.UncategorizedExecutionException; import org.elasticsearch.core.PathUtils; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.discovery.AbstractDisruptionTestCase; -import org.elasticsearch.index.Index; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoryConflictException; @@ -59,6 +56,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -2214,12 +2212,17 @@ public void testDeleteIndexWithOutOfOrderFinalization() { .anyMatch(e -> e.snapshot().getSnapshotId().getName().equals("snapshot-with-index-1") && e.state().completed()) ) // execute the index deletion _directly on the master_ so it happens before the snapshot finalization executes - .andThen(l -> masterDeleteIndexService.deleteIndices(new DeleteIndexClusterStateUpdateRequest(l.map(r -> { - assertTrue(r.isAcknowledged()); - return null; - })).indices(new Index[] { internalCluster().clusterService().state().metadata().index(indexToDelete).getIndex() }) - .ackTimeout(TimeValue.timeValueSeconds(10)) - .masterNodeTimeout(TimeValue.timeValueSeconds(10)))) + .andThen( + l -> masterDeleteIndexService.deleteIndices( + TEST_REQUEST_TIMEOUT, + TEST_REQUEST_TIMEOUT, + Set.of(internalCluster().clusterService().state().metadata().index(indexToDelete).getIndex()), + l.map(r -> { + assertTrue(r.isAcknowledged()); + return null; + }) + ) + ) // ultimately create the index again so that taking a full snapshot will pick up any missing shard gen blob, and deleting that // full snapshot will clean up all dangling shard-level blobs .andThen((l, ignored) -> prepareCreate(indexToDelete, indexSettingsNoReplicas(1)).execute(l.map(r -> { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexClusterStateUpdateRequest.java deleted file mode 100644 index cc6971a0f584b..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexClusterStateUpdateRequest.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", 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.action.admin.indices.delete; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.cluster.ClusterStateAckListener; -import org.elasticsearch.cluster.ClusterStateTaskListener; -import org.elasticsearch.cluster.ack.IndicesClusterStateUpdateRequest; -import org.elasticsearch.cluster.node.DiscoveryNode; - -/** - * Cluster state update request that allows to close one or more indices - */ -public class DeleteIndexClusterStateUpdateRequest extends IndicesClusterStateUpdateRequest - implements - ClusterStateAckListener, - ClusterStateTaskListener { - - private final ActionListener listener; - - public DeleteIndexClusterStateUpdateRequest(ActionListener listener) { - this.listener = listener; - } - - @Override - public void onFailure(Exception e) { - listener.onFailure(e); - } - - @Override - public boolean mustAck(DiscoveryNode discoveryNode) { - return true; - } - - @Override - public void onAllNodesAcked() { - listener.onResponse(AcknowledgedResponse.TRUE); - } - - @Override - public void onAckFailure(Exception e) { - listener.onResponse(AcknowledgedResponse.FALSE); - } - - @Override - public void onAckTimeout() { - listener.onResponse(AcknowledgedResponse.FALSE); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java index 24c152c1a1947..fbf95699dd03a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java @@ -92,11 +92,14 @@ protected void masterOperation( return; } - DeleteIndexClusterStateUpdateRequest deleteRequest = new DeleteIndexClusterStateUpdateRequest(listener.delegateResponse((l, e) -> { - logger.debug(() -> "failed to delete indices [" + concreteIndices + "]", e); - listener.onFailure(e); - })).ackTimeout(request.ackTimeout()).masterNodeTimeout(request.masterNodeTimeout()).indices(concreteIndices.toArray(new Index[0])); - - deleteIndexService.deleteIndices(deleteRequest); + deleteIndexService.deleteIndices( + request.masterNodeTimeout(), + request.ackTimeout(), + concreteIndices, + listener.delegateResponse((l, e) -> { + logger.debug(() -> "failed to delete indices [" + concreteIndices + "]", e); + listener.onFailure(e); + }) + ); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java index ced5d4e490478..5d1a037d6bc3e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java @@ -11,13 +11,16 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexClusterStateUpdateRequest; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateAckListener; import org.elasticsearch.cluster.ClusterStateTaskExecutor; +import org.elasticsearch.cluster.ClusterStateTaskListener; import org.elasticsearch.cluster.RestoreInProgress; import org.elasticsearch.cluster.SimpleBatchedAckListenerTaskExecutor; import org.elasticsearch.cluster.block.ClusterBlocks; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.allocation.AllocationService; import org.elasticsearch.cluster.service.ClusterService; @@ -25,7 +28,7 @@ import org.elasticsearch.common.Priority; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.Index; import org.elasticsearch.injection.guice.Inject; @@ -33,10 +36,10 @@ import org.elasticsearch.snapshots.SnapshotInProgressException; import org.elasticsearch.snapshots.SnapshotsService; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import static org.elasticsearch.cluster.routing.allocation.allocator.AllocationActionListener.rerouteCompletionIsNotRequired; @@ -48,22 +51,19 @@ public class MetadataDeleteIndexService { private static final Logger logger = LogManager.getLogger(MetadataDeleteIndexService.class); - private final Settings settings; - // package private for tests - final ClusterStateTaskExecutor executor; - private final MasterServiceTaskQueue taskQueue; + final ClusterStateTaskExecutor executor; + private final MasterServiceTaskQueue taskQueue; @Inject public MetadataDeleteIndexService(Settings settings, ClusterService clusterService, AllocationService allocationService) { - this.settings = settings; executor = new SimpleBatchedAckListenerTaskExecutor<>() { @Override public Tuple executeTask( - DeleteIndexClusterStateUpdateRequest task, + DeleteIndicesClusterStateUpdateTask task, ClusterState clusterState ) { - return Tuple.tuple(MetadataDeleteIndexService.deleteIndices(clusterState, Sets.newHashSet(task.indices()), settings), task); + return Tuple.tuple(MetadataDeleteIndexService.deleteIndices(clusterState, task.indices, settings), task); } @Override @@ -81,11 +81,64 @@ public ClusterState afterBatchExecution(ClusterState clusterState, boolean clust taskQueue = clusterService.createTaskQueue("delete-index", Priority.URGENT, executor); } - public void deleteIndices(final DeleteIndexClusterStateUpdateRequest request) { - if (request.indices() == null || request.indices().length == 0) { - throw new IllegalArgumentException("Index name is required"); + public void deleteIndices( + TimeValue masterNodeTimeout, + TimeValue ackTimeout, + Set indices, + ActionListener listener + ) { + if (indices == null || indices.isEmpty()) { + throw new IllegalArgumentException("Indices are required"); + } + taskQueue.submitTask( + "delete-index " + indices, + new DeleteIndicesClusterStateUpdateTask(indices, ackTimeout, listener), + masterNodeTimeout + ); + } + + // package private for tests + static class DeleteIndicesClusterStateUpdateTask implements ClusterStateTaskListener, ClusterStateAckListener { + + private final Set indices; + private final TimeValue ackTimeout; + private final ActionListener listener; + + DeleteIndicesClusterStateUpdateTask(Set indices, TimeValue ackTimeout, ActionListener listener) { + this.indices = Objects.requireNonNull(indices); + this.ackTimeout = Objects.requireNonNull(ackTimeout); + this.listener = Objects.requireNonNull(listener); + } + + @Override + public boolean mustAck(DiscoveryNode discoveryNode) { + return true; + } + + @Override + public void onAllNodesAcked() { + listener.onResponse(AcknowledgedResponse.TRUE); + } + + @Override + public void onAckFailure(Exception e) { + listener.onResponse(AcknowledgedResponse.FALSE); + } + + @Override + public void onAckTimeout() { + listener.onResponse(AcknowledgedResponse.FALSE); + } + + @Override + public TimeValue ackTimeout() { + return ackTimeout; + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); } - taskQueue.submitTask("delete-index " + Arrays.toString(request.indices()), request, request.masterNodeTimeout()); } /** diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java index b0b229f19dd7d..0354b6f0bcea8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java @@ -9,7 +9,6 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexClusterStateUpdateRequest; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.SnapshotsInProgress; @@ -132,8 +131,10 @@ public void testDeleteUnassigned() throws Exception { before, service.executor, List.of( - new DeleteIndexClusterStateUpdateRequest(ActionListener.noop()).indices( - new Index[] { before.metadata().getIndices().get(index).getIndex() } + new MetadataDeleteIndexService.DeleteIndicesClusterStateUpdateTask( + Set.of(before.metadata().getIndices().get(index).getIndex()), + TEST_REQUEST_TIMEOUT, + ActionListener.noop() ) ) ); From 8fe7d74e425895dd5b398da5f04a2f4aeb2494fa Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 Sep 2024 07:33:13 +0100 Subject: [PATCH 41/46] Make `IndicesAliasesClusterStateUpdateRequest` a record (#113281) No need to extend `ClusterStateUpdateRequest` here. --- ...dicesAliasesClusterStateUpdateRequest.java | 33 ++++++++----------- .../alias/TransportIndicesAliasesAction.java | 4 ++- .../metadata/MetadataIndexAliasesService.java | 2 +- .../MetadataIndexAliasesServiceTests.java | 4 +++ 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java index e98091beb88a5..c6d60f34eddeb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java @@ -9,32 +9,25 @@ package org.elasticsearch.action.admin.indices.alias; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse.AliasActionResult; -import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.metadata.AliasAction; +import org.elasticsearch.core.TimeValue; import java.util.List; +import java.util.Objects; /** * Cluster state update request that allows to add or remove aliases */ -public class IndicesAliasesClusterStateUpdateRequest extends ClusterStateUpdateRequest { - private final List actions; - - private final List actionResults; - - public IndicesAliasesClusterStateUpdateRequest(List actions, List actionResults) { - this.actions = actions; - this.actionResults = actionResults; - } - - /** - * Returns the alias actions to be performed - */ - public List actions() { - return actions; - } - - public List getActionResults() { - return actionResults; +public record IndicesAliasesClusterStateUpdateRequest( + TimeValue masterNodeTimeout, + TimeValue ackTimeout, + List actions, + List actionResults +) { + public IndicesAliasesClusterStateUpdateRequest { + Objects.requireNonNull(masterNodeTimeout); + Objects.requireNonNull(ackTimeout); + Objects.requireNonNull(actions); + Objects.requireNonNull(actionResults); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index 129e853ec638c..c73c44e9a23e7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -255,9 +255,11 @@ protected void masterOperation( } request.aliasActions().clear(); IndicesAliasesClusterStateUpdateRequest updateRequest = new IndicesAliasesClusterStateUpdateRequest( + request.masterNodeTimeout(), + request.ackTimeout(), unmodifiableList(finalActions), unmodifiableList(actionResults) - ).ackTimeout(request.ackTimeout()).masterNodeTimeout(request.masterNodeTimeout()); + ); indexAliasesService.indicesAliases(updateRequest, listener.delegateResponse((l, e) -> { logger.debug("failed to perform aliases", e); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java index 130a38f39b11a..f926e2b6ebf35 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java @@ -282,7 +282,7 @@ public boolean mustAck(DiscoveryNode discoveryNode) { @Override public void onAllNodesAcked() { - listener.onResponse(IndicesAliasesResponse.build(request.getActionResults())); + listener.onResponse(IndicesAliasesResponse.build(request.actionResults())); } @Override diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java index 796d0892184f5..4f2c84d76b5a4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -693,10 +693,14 @@ public void testAddAndRemoveAliasClusterStateUpdate() throws Exception { String index = randomAlphaOfLength(5); ClusterState before = createIndex(ClusterState.builder(ClusterName.DEFAULT).build(), index); IndicesAliasesClusterStateUpdateRequest addAliasRequest = new IndicesAliasesClusterStateUpdateRequest( + TEST_REQUEST_TIMEOUT, + TEST_REQUEST_TIMEOUT, List.of(new AliasAction.Add(index, "test", null, null, null, null, null)), List.of(AliasActionResult.buildSuccess(List.of(index), AliasActions.add().aliases("test").indices(index))) ); IndicesAliasesClusterStateUpdateRequest removeAliasRequest = new IndicesAliasesClusterStateUpdateRequest( + TEST_REQUEST_TIMEOUT, + TEST_REQUEST_TIMEOUT, List.of(new AliasAction.Remove(index, "test", true)), List.of(AliasActionResult.buildSuccess(List.of(index), AliasActions.remove().aliases("test").indices(index))) ); From 8edfa059ec091be900e4e98f22ace00ef185ec85 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 Sep 2024 07:33:23 +0100 Subject: [PATCH 42/46] Make `MigrateToDataStreamClusterStateUpdateRequest` a record (#113277) No need to extend `ClusterStateUpdateRequest` here. --- .../MetadataMigrateToDataStreamService.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataMigrateToDataStreamService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataMigrateToDataStreamService.java index 7ab68abb99ca5..aee60c3eda57f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataMigrateToDataStreamService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataMigrateToDataStreamService.java @@ -18,7 +18,6 @@ import org.elasticsearch.cluster.AckedClusterStateUpdateTask; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateUpdateTask; -import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.metadata.MetadataCreateDataStreamService.CreateDataStreamClusterStateUpdateRequest; import org.elasticsearch.cluster.routing.allocation.allocator.AllocationActionListener; import org.elasticsearch.cluster.service.ClusterService; @@ -40,6 +39,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -283,15 +283,11 @@ static void validateBackingIndices(ClusterState currentState, String dataStreamN } } - @SuppressWarnings("rawtypes") - public static final class MigrateToDataStreamClusterStateUpdateRequest extends ClusterStateUpdateRequest { - - private final String aliasName; - - public MigrateToDataStreamClusterStateUpdateRequest(String aliasName, TimeValue masterNodeTimeout, TimeValue timeout) { - this.aliasName = aliasName; - masterNodeTimeout(masterNodeTimeout); - ackTimeout(timeout); + public record MigrateToDataStreamClusterStateUpdateRequest(String aliasName, TimeValue masterNodeTimeout, TimeValue ackTimeout) { + public MigrateToDataStreamClusterStateUpdateRequest { + Objects.requireNonNull(aliasName); + Objects.requireNonNull(masterNodeTimeout); + Objects.requireNonNull(ackTimeout); } } From 71eb158b367409d2d44fa0b5016465fea3678a9a Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:16:30 +1000 Subject: [PATCH 43/46] Mute org.elasticsearch.action.bulk.IncrementalBulkIT testBulkLevelBulkFailureAfterFirstIncrementalRequest #113365 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index a592a493423c0..b5d0aa8f2fe63 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -330,6 +330,9 @@ tests: - class: org.elasticsearch.integration.KibanaUserRoleIntegTests method: testSearchAndMSearch issue: https://github.com/elastic/elasticsearch/issues/113345 +- class: org.elasticsearch.action.bulk.IncrementalBulkIT + method: testBulkLevelBulkFailureAfterFirstIncrementalRequest + issue: https://github.com/elastic/elasticsearch/issues/113365 # Examples: # From 8d223cbf7a097765e9e18c80d9abafd17eb93336 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 23 Sep 2024 09:31:18 +0200 Subject: [PATCH 44/46] Add support for multi-value dimensions (#112645) Closes https://github.com/elastic/elasticsearch/issues/110387 Having this in now affords us not having to introduce version checks in the ES exporter later. We can simply use the same serialization logic for metric attributes as we do for other signals. This also enables us to properly map `*.ip` fields to the ip field type as ip fields containing a list of IPs are not converted to a comma-separated list. --- docs/changelog/112645.yaml | 6 ++ docs/reference/mapping/types/keyword.asciidoc | 1 - .../test/data_stream/150_tsdb.yml | 80 +++++++++++++++++++ rest-api-spec/build.gradle | 4 + .../test/tsdb/140_routing_path.yml | 31 +++---- .../cluster/routing/IndexRouting.java | 58 +++++++++----- .../cluster/routing/RoutingFeatures.java | 2 +- .../common/xcontent/XContentParserUtils.java | 14 ++++ .../index/mapper/TimeSeriesIdFieldMapper.java | 58 +++++++++----- .../cluster/routing/IndexRoutingTests.java | 31 ++++++- .../index/mapper/BooleanFieldMapperTests.java | 7 +- .../index/mapper/IpFieldMapperTests.java | 10 +-- .../index/mapper/KeywordFieldMapperTests.java | 7 +- .../mapper/TimeSeriesIdFieldMapperTests.java | 47 +++++++++++ .../flattened/FlattenedFieldMapperTests.java | 11 +-- .../mapper/WholeNumberFieldMapperTests.java | 10 +-- .../UnsignedLongFieldMapperTests.java | 11 +-- .../metrics-otel@template.yaml | 5 ++ .../rest-api-spec/test/20_metrics_tests.yml | 39 ++++++++- 19 files changed, 349 insertions(+), 83 deletions(-) create mode 100644 docs/changelog/112645.yaml diff --git a/docs/changelog/112645.yaml b/docs/changelog/112645.yaml new file mode 100644 index 0000000000000..cf4ef4609a1f3 --- /dev/null +++ b/docs/changelog/112645.yaml @@ -0,0 +1,6 @@ +pr: 112645 +summary: Add support for multi-value dimensions +area: Mapping +type: enhancement +issues: + - 110387 diff --git a/docs/reference/mapping/types/keyword.asciidoc b/docs/reference/mapping/types/keyword.asciidoc index 59d307c4df0ad..a4be7026dffcd 100644 --- a/docs/reference/mapping/types/keyword.asciidoc +++ b/docs/reference/mapping/types/keyword.asciidoc @@ -163,7 +163,6 @@ index setting limits the number of dimensions in an index. Dimension fields have the following constraints: * The `doc_values` and `index` mapping parameters must be `true`. -* Field values cannot be an <>. // end::dimension[] * Dimension values are used to identify a document’s time series. If dimension values are altered in any way during indexing, the document will be stored as belonging to different from intended time series. As a result there are additional constraints: ** The field cannot use a <>. diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml index d20231a6d6cf2..56f387c016261 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml @@ -1230,3 +1230,83 @@ non string dimension fields: - match: { .$idx0name.mappings.properties.attributes.properties.double.time_series_dimension: true } - match: { .$idx0name.mappings.properties.attributes.properties.host\.ip.type: 'ip' } - match: { .$idx0name.mappings.properties.attributes.properties.host\.ip.time_series_dimension: true } + +--- +multi value dimensions: + - requires: + cluster_features: ["routing.multi_value_routing_path"] + reason: support for multi-value dimensions + + - do: + allowed_warnings: + - "index template [my-dynamic-template] has index patterns [k9s*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [my-dynamic-template] will take precedence during new index creation" + indices.put_index_template: + name: my-dynamic-template + body: + index_patterns: [k9s*] + data_stream: {} + template: + settings: + index: + number_of_shards: 1 + mode: time_series + time_series: + start_time: 2023-08-31T13:03:08.138Z + + mappings: + properties: + attributes: + type: passthrough + dynamic: true + time_series_dimension: true + priority: 1 + dynamic_templates: + - counter_metric: + mapping: + type: integer + time_series_metric: counter + + - do: + bulk: + index: k9s + refresh: true + body: + - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }' + - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "10", "attributes": { "dim1": ["a" , "b"], "dim2": [1, 2] } }' + - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }' + - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "20", "attributes": { "dim1": ["b" , "a"], "dim2": [1, 2] } }' + - '{ "create": { "dynamic_templates": { "data": "counter_metric" } } }' + - '{ "@timestamp": "2023-09-01T13:03:08.138Z","data": "20", "attributes": { "dim1": ["c" , "b"], "dim2": [1, 2] } }' + - is_false: errors + + - do: + search: + index: k9s + body: + size: 0 + aggs: + tsids: + terms: + field: _tsid + + - length: { aggregations.tsids.buckets: 3 } # only the order of the dim1 attribute is different, yet we expect to have two distinct time series + + - do: + search: + index: k9s + body: + size: 0 + aggs: + dims: + terms: + field: dim1 + order: + _key: asc + + - length: { aggregations.dims.buckets: 3 } + - match: { aggregations.dims.buckets.0.key: a } + - match: { aggregations.dims.buckets.0.doc_count: 2 } + - match: { aggregations.dims.buckets.1.key: b } + - match: { aggregations.dims.buckets.1.doc_count: 3 } + - match: { aggregations.dims.buckets.2.key: c } + - match: { aggregations.dims.buckets.2.doc_count: 1 } diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 015c9c4b812c6..eb3d66ece50d6 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -53,3 +53,7 @@ tasks.register('enforceYamlTestConvention').configure { 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") +}) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/140_routing_path.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/140_routing_path.yml index 6eb7a8dcad7aa..616afd3cf67ad 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/140_routing_path.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/140_routing_path.yml @@ -119,11 +119,11 @@ missing dimension on routing path field: type: keyword --- -multi-value routing path field: +multi-value routing path field succeeds: - requires: test_runner_features: close_to - cluster_features: ["gte_v8.13.0"] - reason: _tsid hashing introduced in 8.13 + cluster_features: ["routing.multi_value_routing_path"] + reason: support for multi-value dimensions - do: indices.create: @@ -172,12 +172,7 @@ multi-value routing path field: - '{"index": {}}' - '{"@timestamp": "2021-04-28T18:35:54.467Z", "uid": "df3145b3-0563-4d3b-a0f7-897eb2876ea9", "voltage": 6.8, "unmapped_field": 40, "tag": [ "one", "three" ] }' - - is_true: errors - - - match: {items.1.index.error.reason: "Error extracting routing: Routing values must be strings but found [START_ARRAY]" } - - match: {items.3.index.error.reason: "Error extracting routing: Routing values must be strings but found [START_ARRAY]" } - - match: {items.4.index.error.reason: "Error extracting routing: Routing values must be strings but found [START_ARRAY]" } - - match: {items.7.index.error.reason: "Error extracting routing: Routing values must be strings but found [START_ARRAY]" } + - is_false: errors - do: search: @@ -195,13 +190,21 @@ multi-value routing path field: avg: field: voltage - - match: {hits.total.value: 4} - - length: {aggregations.tsids.buckets: 2} + - match: {hits.total.value: 8} + - length: {aggregations.tsids.buckets: 4} - - match: {aggregations.tsids.buckets.0.key: "KDODRmbj7vu4rLWvjrJbpUuaET_vOYoRw6ImzKEcF4sEaGKnXSaKfM0" } + - match: {aggregations.tsids.buckets.0.key: "KDODRmbj7vu4rLWvjrJbpUtt0uPSOYoRw_LI4DD7DFEGEJ3NR3eQkMY" } - match: {aggregations.tsids.buckets.0.doc_count: 2 } - close_to: {aggregations.tsids.buckets.0.voltage.value: { value: 6.70, error: 0.01 }} - - match: { aggregations.tsids.buckets.1.key: "KDODRmbj7vu4rLWvjrJbpUvcUWJEddqA4Seo8jbBBBFxwC0lrefCb6A" } + - match: { aggregations.tsids.buckets.1.key: "KDODRmbj7vu4rLWvjrJbpUtt0uPSddqA4WYKglGPR_C0cJe8QGaiC2c" } - match: {aggregations.tsids.buckets.1.doc_count: 2 } - - close_to: {aggregations.tsids.buckets.1.voltage.value: { value: 7.30, error: 0.01 }} + - close_to: {aggregations.tsids.buckets.1.voltage.value: { value: 7.15, error: 0.01 }} + + - match: { aggregations.tsids.buckets.2.key: "KDODRmbj7vu4rLWvjrJbpUuaET_vOYoRw6ImzKEcF4sEaGKnXSaKfM0" } + - match: {aggregations.tsids.buckets.2.doc_count: 2 } + - close_to: {aggregations.tsids.buckets.2.voltage.value: { value: 6.70, error: 0.01 }} + + - match: { aggregations.tsids.buckets.3.key: "KDODRmbj7vu4rLWvjrJbpUvcUWJEddqA4Seo8jbBBBFxwC0lrefCb6A" } + - match: {aggregations.tsids.buckets.3.doc_count: 2 } + - close_to: {aggregations.tsids.buckets.3.voltage.value: { value: 7.30, error: 0.01 }} diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java index 8a69c66bf8375..3fb3c182f89cd 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java @@ -36,7 +36,6 @@ import java.util.ArrayList; import java.util.Base64; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -46,6 +45,7 @@ import java.util.function.Predicate; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.elasticsearch.common.xcontent.XContentParserUtils.expectValueToken; /** * Generates the shard id for {@code (id, routing)} pairs. @@ -53,6 +53,7 @@ public abstract class IndexRouting { static final NodeFeature BOOLEAN_ROUTING_PATH = new NodeFeature("routing.boolean_routing_path"); + static final NodeFeature MULTI_VALUE_ROUTING_PATH = new NodeFeature("routing.multi_value_routing_path"); /** * Build the routing from {@link IndexMetadata}. @@ -302,7 +303,13 @@ public String createId(Map flat, byte[] suffix) { Builder b = builder(); for (Map.Entry e : flat.entrySet()) { if (isRoutingPath.test(e.getKey())) { - b.hashes.add(new NameAndHash(new BytesRef(e.getKey()), hash(new BytesRef(e.getValue().toString())))); + if (e.getValue() instanceof List listValue) { + for (Object v : listValue) { + b.addHash(e.getKey(), new BytesRef(v.toString())); + } + } else { + b.addHash(e.getKey(), new BytesRef(e.getValue().toString())); + } } } return b.createId(suffix, IndexRouting.ExtractFromSource::defaultOnEmpty); @@ -337,7 +344,7 @@ public class Builder { public void addMatching(String fieldName, BytesRef string) { if (isRoutingPath.test(fieldName)) { - hashes.add(new NameAndHash(new BytesRef(fieldName), hash(string))); + addHash(fieldName, string); } } @@ -358,6 +365,13 @@ private void extractObject(@Nullable String path, XContentParser source) throws } } + private void extractArray(@Nullable String path, XContentParser source) throws IOException { + while (source.currentToken() != Token.END_ARRAY) { + expectValueToken(source.currentToken(), source); + extractItem(path, source); + } + } + private void extractItem(String path, XContentParser source) throws IOException { switch (source.currentToken()) { case START_OBJECT: @@ -368,7 +382,12 @@ private void extractItem(String path, XContentParser source) throws IOException case VALUE_STRING: case VALUE_NUMBER: case VALUE_BOOLEAN: - hashes.add(new NameAndHash(new BytesRef(path), hash(new BytesRef(source.text())))); + addHash(path, new BytesRef(source.text())); + source.nextToken(); + break; + case START_ARRAY: + source.nextToken(); + extractArray(path, source); source.nextToken(); break; case VALUE_NULL: @@ -377,28 +396,24 @@ private void extractItem(String path, XContentParser source) throws IOException default: throw new ParsingException( source.getTokenLocation(), - "Routing values must be strings but found [{}]", + "Cannot extract routing path due to unexpected token [{}]", source.currentToken() ); } } + private void addHash(String path, BytesRef value) { + hashes.add(new NameAndHash(new BytesRef(path), hash(value), hashes.size())); + } + private int buildHash(IntSupplier onEmpty) { - Collections.sort(hashes); - Iterator itr = hashes.iterator(); - if (itr.hasNext() == false) { + if (hashes.isEmpty()) { return onEmpty.getAsInt(); } - NameAndHash prev = itr.next(); - int hash = hash(prev.name) ^ prev.hash; - while (itr.hasNext()) { - NameAndHash next = itr.next(); - if (prev.name.equals(next.name)) { - throw new IllegalArgumentException("Duplicate routing dimension for [" + next.name + "]"); - } - int thisHash = hash(next.name) ^ next.hash; - hash = 31 * hash + thisHash; - prev = next; + Collections.sort(hashes); + int hash = 0; + for (NameAndHash nah : hashes) { + hash = 31 * hash + (hash(nah.name) ^ nah.hash); } return hash; } @@ -459,10 +474,13 @@ private String error(String operation) { } } - private record NameAndHash(BytesRef name, int hash) implements Comparable { + private record NameAndHash(BytesRef name, int hash, int order) implements Comparable { @Override public int compareTo(NameAndHash o) { - return name.compareTo(o.name); + int i = name.compareTo(o.name); + if (i != 0) return i; + // ensures array values are in the order as they appear in the source + return Integer.compare(order, o.order); } } } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingFeatures.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingFeatures.java index 8885410bd0530..f8028ce7f9d68 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingFeatures.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingFeatures.java @@ -18,6 +18,6 @@ public class RoutingFeatures implements FeatureSpecification { @Override public Set getFeatures() { - return Set.of(IndexRouting.BOOLEAN_ROUTING_PATH); + return Set.of(IndexRouting.BOOLEAN_ROUTING_PATH, IndexRouting.MULTI_VALUE_ROUTING_PATH); } } diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java index cd7cf5ddbd893..6390e62f9758f 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java @@ -72,6 +72,20 @@ public static void ensureExpectedToken(Token expected, Token actual, XContentPar } } + /** + * Makes sure the provided token {@linkplain Token#isValue() is a value type} + * + * @throws ParsingException if the token is not a value type + */ + public static void expectValueToken(Token actual, XContentParser parser) { + if (actual.isValue() == false) { + throw new ParsingException( + parser.getTokenLocation(), + String.format(Locale.ROOT, "Failed to parse object: expecting value token but found [%s]", actual) + ); + } + } + private static ParsingException parsingException(XContentParser parser, Token expected, Token actual) { return new ParsingException( parser.getTokenLocation(), diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java index 6ba0d4568e713..a6b2ad265decf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java @@ -42,12 +42,13 @@ import java.io.IOException; import java.net.InetAddress; import java.time.ZoneId; +import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; -import java.util.SortedSet; -import java.util.TreeSet; +import java.util.SortedMap; +import java.util.TreeMap; /** * Mapper for {@code _tsid} field included generated when the index is @@ -175,16 +176,14 @@ public static class TimeSeriesIdBuilder implements DocumentDimensions { public static final int MAX_DIMENSIONS = 512; - private record Dimension(BytesRef name, BytesReference value) {} - private final Murmur3Hasher tsidHasher = new Murmur3Hasher(0); /** - * A sorted set of the serialized values of dimension fields that will be used + * A map of the serialized values of dimension fields that will be used * for generating the _tsid field. The map will be used by {@link TimeSeriesIdFieldMapper} * to build the _tsid field for the document. */ - private final SortedSet dimensions = new TreeSet<>(Comparator.comparing(o -> o.name)); + private final SortedMap> dimensions = new TreeMap<>(); /** * Builds the routing. Used for building {@code _id}. If null then skipped. */ @@ -202,9 +201,17 @@ public BytesReference buildLegacyTsid() throws IOException { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(dimensions.size()); - for (Dimension entry : dimensions) { - out.writeBytesRef(entry.name); - entry.value.writeTo(out); + for (Map.Entry> entry : dimensions.entrySet()) { + out.writeBytesRef(entry.getKey()); + List value = entry.getValue(); + if (value.size() > 1) { + // multi-value dimensions are only supported for newer indices that use buildTsidHash + throw new IllegalArgumentException( + "Dimension field [" + entry.getKey().utf8ToString() + "] cannot be a multi-valued field." + ); + } + assert value.isEmpty() == false : "dimension value is empty"; + value.get(0).writeTo(out); } return out.bytes(); } @@ -236,18 +243,19 @@ public BytesReference buildTsidHash() { int tsidHashIndex = StreamOutput.putVInt(tsidHash, len, 0); tsidHasher.reset(); - for (final Dimension dimension : dimensions) { - tsidHasher.update(dimension.name.bytes); + for (final BytesRef name : dimensions.keySet()) { + tsidHasher.update(name.bytes); } tsidHashIndex = writeHash128(tsidHasher.digestHash(), tsidHash, tsidHashIndex); // NOTE: concatenate all dimension value hashes up to a certain number of dimensions int tsidHashStartIndex = tsidHashIndex; - for (final Dimension dimension : dimensions) { + for (final List values : dimensions.values()) { if ((tsidHashIndex - tsidHashStartIndex) >= 4 * numberOfDimensions) { break; } - final BytesRef dimensionValueBytesRef = dimension.value.toBytesRef(); + assert values.isEmpty() == false : "dimension values are empty"; + final BytesRef dimensionValueBytesRef = values.get(0).toBytesRef(); ByteUtils.writeIntLE( StringHelper.murmurhash3_x86_32( dimensionValueBytesRef.bytes, @@ -263,8 +271,10 @@ public BytesReference buildTsidHash() { // NOTE: hash all dimension field allValues tsidHasher.reset(); - for (final Dimension dimension : dimensions) { - tsidHasher.update(dimension.value.toBytesRef().bytes); + for (final List values : dimensions.values()) { + for (BytesReference v : values) { + tsidHasher.update(v.toBytesRef().bytes); + } } tsidHashIndex = writeHash128(tsidHasher.digestHash(), tsidHash, tsidHashIndex); @@ -367,8 +377,20 @@ public DocumentDimensions validate(final IndexSettings settings) { } private void add(String fieldName, BytesReference encoded) throws IOException { - if (dimensions.add(new Dimension(new BytesRef(fieldName), encoded)) == false) { - throw new IllegalArgumentException("Dimension field [" + fieldName + "] cannot be a multi-valued field."); + BytesRef name = new BytesRef(fieldName); + List values = dimensions.get(name); + if (values == null) { + // optimize for the common case where dimensions are not multi-valued + dimensions.put(name, List.of(encoded)); + } else { + if (values.size() == 1) { + // converts the immutable list that's optimized for the common case of having only one value to a mutable list + BytesReference previousValue = values.get(0); + values = new ArrayList<>(4); + values.add(previousValue); + dimensions.put(name, values); + } + values.add(encoded); } } } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java index 6ac8e6a853f27..e39ccdf7af5e2 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTests.java @@ -595,7 +595,32 @@ public void testRoutingPathBooleansInSource() throws IOException { IndexRouting routing = indexRoutingForPath(shards, "foo"); assertIndexShard(routing, Map.of("foo", true), Math.floorMod(hash(List.of("foo", "true")), shards)); assertIndexShard(routing, Map.of("foo", false), Math.floorMod(hash(List.of("foo", "false")), shards)); + } + public void testRoutingPathArraysInSource() throws IOException { + int shards = between(2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "a,b,c,d"); + assertIndexShard( + routing, + Map.of("c", List.of(true), "d", List.of(), "a", List.of("foo", "bar", "foo"), "b", List.of(21, 42)), + // Note that the fields are sorted + Math.floorMod(hash(List.of("a", "foo", "a", "bar", "a", "foo", "b", "21", "b", "42", "c", "true")), shards) + ); + } + + public void testRoutingPathObjectArraysInSource() throws IOException { + int shards = between(2, 1000); + IndexRouting routing = indexRoutingForPath(shards, "a"); + + BytesReference source = source(Map.of("a", List.of("foo", Map.of("foo", "bar")))); + Exception e = expectThrows( + IllegalArgumentException.class, + () -> routing.indexShard(randomAlphaOfLength(5), null, XContentType.JSON, source, s -> {}) + ); + assertThat( + e.getMessage(), + equalTo("Error extracting routing: Failed to parse object: expecting value token but found [START_OBJECT]") + ); } public void testRoutingPathBwc() throws IOException { @@ -668,7 +693,11 @@ private void assertIndexShard(IndexRouting routing, Map source, IndexRouting.ExtractFromSource.Builder b = r.builder(); for (Map.Entry e : flattened.entrySet()) { - b.addMatching(e.getKey(), new BytesRef(e.getValue().toString())); + if (e.getValue() instanceof List listValue) { + listValue.forEach(v -> b.addMatching(e.getKey(), new BytesRef(v.toString()))); + } else { + b.addMatching(e.getKey(), new BytesRef(e.getValue().toString())); + } } String idFromBuilder = b.createId(suffix, () -> { throw new AssertionError(); }); assertThat(idFromBuilder, equalTo(idFromSource)); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java index b0a9a9e9fe7dc..83553503c3c5e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java @@ -268,8 +268,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimension", true); }), IndexMode.TIME_SERIES); - Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", true, false)))); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field", true, false); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 2d190f77f60f7..86c1157259790 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -259,11 +259,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimension", true); }), IndexMode.TIME_SERIES); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1"))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field", "192.168.1.1", "192.168.1.1"); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 2d2fad23b3831..5b218fb077d32 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -384,8 +384,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimension", true); }), IndexMode.TIME_SERIES); - Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field", "1234", "45678"); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java index b27b8bd9766f3..9d56938f185de 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java @@ -28,10 +28,13 @@ import org.elasticsearch.xcontent.XContentType; import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -63,13 +66,19 @@ protected IndexVersion getVersion() { } private DocumentMapper createDocumentMapper(String routingPath, XContentBuilder mappings) throws IOException { + return createDocumentMapper(getVersion(), routingPath, mappings); + } + + private DocumentMapper createDocumentMapper(IndexVersion version, String routingPath, XContentBuilder mappings) throws IOException { return createMapperService( + version, getIndexSettingsBuilder().put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES.name()) .put(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING.getKey(), 200) // Allow tests that use many dimensions .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), routingPath) .put(IndexSettings.TIME_SERIES_START_TIME.getKey(), "2021-04-28T00:00:00Z") .put(IndexSettings.TIME_SERIES_END_TIME.getKey(), "2021-10-29T00:00:00Z") .build(), + () -> true, mappings ).documentMapper(); } @@ -644,6 +653,44 @@ public void testDifferentDimensions() throws IOException { assertThat(doc1.rootDoc().getBinaryValue("_tsid").bytes, not(doc2.rootDoc().getBinaryValue("_tsid").bytes)); } + public void testMultiValueDimensionsNotSupportedBeforeTsidHashing() throws IOException { + IndexVersion priorToTsidHashing = IndexVersionUtils.getPreviousVersion(IndexVersions.TIME_SERIES_ID_HASHING); + DocumentMapper docMapper = createDocumentMapper( + priorToTsidHashing, + "a", + mapping(b -> b.startObject("a").field("type", "keyword").field("time_series_dimension", true).endObject()) + ); + + String a1 = randomAlphaOfLength(10); + String a2 = randomAlphaOfLength(10); + CheckedConsumer fields = d -> d.field("a", new String[] { a1, a2 }); + DocumentParsingException exception = assertThrows(DocumentParsingException.class, () -> parseDocument(docMapper, fields)); + assertThat(exception.getMessage(), containsString("Dimension field [a] cannot be a multi-valued field")); + } + + public void testMultiValueDimensions() throws IOException { + DocumentMapper docMapper = createDocumentMapper( + IndexVersions.TIME_SERIES_ID_HASHING, + "a", + mapping(b -> b.startObject("a").field("type", "keyword").field("time_series_dimension", true).endObject()) + ); + + String a1 = randomAlphaOfLength(10); + String a2 = randomAlphaOfLength(10); + List docs = List.of( + parseDocument(docMapper, d -> d.field("a", new String[] { a1 })), + parseDocument(docMapper, d -> d.field("a", new String[] { a1, a2 })), + parseDocument(docMapper, d -> d.field("a", new String[] { a2, a1 })), + parseDocument(docMapper, d -> d.field("a", new String[] { a1, a2, a1 })), + parseDocument(docMapper, d -> d.field("a", new String[] { a2, a1, a2 })) + ); + List tsids = docs.stream() + .map(doc -> doc.rootDoc().getBinaryValue("_tsid").toString()) + .distinct() + .collect(Collectors.toList()); + assertThat(tsids, hasSize(docs.size())); + } + /** * Documents with fewer dimensions have a different value. */ diff --git a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java index 2f245a319f8cc..285431b881add 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.index.mapper.MapperTestCase; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; +import org.elasticsearch.index.mapper.TimeSeriesRoutingHashFieldMapper; import org.elasticsearch.index.mapper.flattened.FlattenedFieldMapper.KeyedFlattenedFieldType; import org.elasticsearch.index.mapper.flattened.FlattenedFieldMapper.RootFlattenedFieldType; import org.elasticsearch.xcontent.XContentBuilder; @@ -200,11 +201,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimensions", List.of("key1", "key2", "field3.key3")); }), IndexMode.TIME_SERIES); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field.key1", "value1", "value2"))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field.key1] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field.key1", "value1", "value2"); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java index bb17983264072..a297f5d13254b 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java @@ -80,11 +80,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimension", true); }), IndexMode.TIME_SERIES); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber()))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field", randomNumber(), randomNumber(), randomNumber()); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index cdb25fbe995b2..f554a84048fde 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.index.mapper.NumberTypeOutOfRangeSpec; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.TimeSeriesParams; +import org.elasticsearch.index.mapper.TimeSeriesRoutingHashFieldMapper; import org.elasticsearch.index.mapper.WholeNumberFieldMapperTests; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.xcontent.XContentBuilder; @@ -268,11 +269,11 @@ public void testDimensionMultiValuedFieldTSDB() throws IOException { b.field("time_series_dimension", true); }), IndexMode.TIME_SERIES); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ParsedDocument doc = mapper.parse(source(null, b -> { + b.array("field", randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); + b.field("@timestamp", Instant.now()); + }, TimeSeriesRoutingHashFieldMapper.encode(randomInt()))); + assertThat(doc.docs().get(0).getFields("field"), hasSize(greaterThan(1))); } public void testDimensionMultiValuedFieldNonTSDB() throws IOException { diff --git a/x-pack/plugin/otel-data/src/main/resources/index-templates/metrics-otel@template.yaml b/x-pack/plugin/otel-data/src/main/resources/index-templates/metrics-otel@template.yaml index 89ff28249aabb..a4413d266181d 100644 --- a/x-pack/plugin/otel-data/src/main/resources/index-templates/metrics-otel@template.yaml +++ b/x-pack/plugin/otel-data/src/main/resources/index-templates/metrics-otel@template.yaml @@ -28,6 +28,11 @@ template: type: constant_keyword value: metrics dynamic_templates: + - ecs_ip: + mapping: + type: ip + path_match: [ "ip", "*.ip", "*_ip" ] + match_mapping_type: string - all_strings_to_keywords: mapping: ignore_above: 1024 diff --git a/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_metrics_tests.yml b/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_metrics_tests.yml index a6591d6c32210..1823dfab7e716 100644 --- a/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_metrics_tests.yml +++ b/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_metrics_tests.yml @@ -123,11 +123,10 @@ setup: start_time: 2024-07-01T13:03:08.138Z mappings: dynamic_templates: - - ip_fields: + - no_ip_fields: mapping: - type: ip + type: keyword match_mapping_type: string - path_match: "*.ip" - do: bulk: index: metrics-generic.otel-default @@ -145,5 +144,37 @@ setup: indices.get_mapping: index: $idx0name expand_wildcards: hidden - - match: { .$idx0name.mappings.properties.attributes.properties.host\.ip.type: 'ip' } + - match: { .$idx0name.mappings.properties.attributes.properties.host\.ip.type: 'keyword' } - match: { .$idx0name.mappings.properties.attributes.properties.foo.type: "keyword" } +--- +IP dimensions: + - requires: + cluster_features: ["routing.multi_value_routing_path"] + reason: support for multi-value dimensions + - do: + bulk: + index: metrics-generic.otel-default + refresh: true + body: + - create: {"dynamic_templates":{"metrics.foo.bar":"counter_long"}} + - "@timestamp": 2024-07-18T14:48:33.467654000Z + resource: + attributes: + host.ip: [ "127.0.0.1", "0.0.0.0" ] + attributes: + philip: [ a, b, c ] + metrics: + foo.bar: 42 + - is_false: errors + + - do: + indices.get_data_stream: + name: metrics-generic.otel-default + - set: { data_streams.0.indices.0.index_name: idx0name } + + - do: + indices.get_mapping: + index: $idx0name + expand_wildcards: hidden + - match: { .$idx0name.mappings.properties.resource.properties.attributes.properties.host\.ip.type: 'ip' } + - match: { .$idx0name.mappings.properties.attributes.properties.philip.type: "keyword" } From c28df8e538282022ae45c355fdaefeb711b5d135 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 Sep 2024 08:42:10 +0100 Subject: [PATCH 45/46] Parallelize master stats in `TransportNodeStatsAction` (#113236) Today we reach out to the master for its stats after collecting all the node-level stats. With #113140 we can do these things in parallel very simply. This commit does so. --- .../node/stats/TransportNodesStatsAction.java | 62 ++++++++++++------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java index 2a628466ea396..a4fda469da3a2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java @@ -16,7 +16,9 @@ import org.elasticsearch.action.admin.cluster.allocation.TransportGetAllocationStatsAction; import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsRequestParameters.Metric; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.SubscribableListener; import org.elasticsearch.action.support.nodes.TransportNodesAction; +import org.elasticsearch.client.internal.ParentTaskAssigningClient; import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings; @@ -47,7 +49,7 @@ public class TransportNodesStatsAction extends TransportNodesAction< NodesStatsResponse, TransportNodesStatsAction.NodeStatsRequest, NodeStats, - Void> { + SubscribableListener> { public static final ActionType TYPE = new ActionType<>("cluster:monitor/nodes/stats"); @@ -77,37 +79,55 @@ public TransportNodesStatsAction( @Override protected NodesStatsResponse newResponse(NodesStatsRequest request, List responses, List failures) { - return new NodesStatsResponse(clusterService.getClusterName(), responses, failures); + assert false; + throw new UnsupportedOperationException("use newResponseAsync instead"); + } + + @Override + protected SubscribableListener createActionContext(Task task, NodesStatsRequest request) { + return SubscribableListener.newForked(l -> { + var metrics = request.getNodesStatsRequestParameters().requestedMetrics(); + if (metrics.contains(Metric.FS) || metrics.contains(Metric.ALLOCATIONS)) { + new ParentTaskAssigningClient(client, clusterService.localNode(), task).execute( + TransportGetAllocationStatsAction.TYPE, + new TransportGetAllocationStatsAction.Request( + Objects.requireNonNullElse(request.timeout(), RestUtils.REST_MASTER_TIMEOUT_DEFAULT), + new TaskId(clusterService.localNode().getId(), task.getId()), + metrics + ), + l + ); + } else { + l.onResponse(null); + } + }); } @Override protected void newResponseAsync( Task task, NodesStatsRequest request, - Void actionContext, + SubscribableListener actionContext, List responses, List failures, ActionListener listener ) { - var metrics = request.getNodesStatsRequestParameters().requestedMetrics(); - if (metrics.contains(Metric.FS) || metrics.contains(Metric.ALLOCATIONS)) { - client.execute( - TransportGetAllocationStatsAction.TYPE, - new TransportGetAllocationStatsAction.Request( - Objects.requireNonNullElse(request.timeout(), RestUtils.REST_MASTER_TIMEOUT_DEFAULT), - new TaskId(clusterService.localNode().getId(), task.getId()), - metrics - ), - listener.delegateFailure( - (l, r) -> ActionListener.respondAndRelease( - l, - newResponse(request, merge(responses, r.getNodeAllocationStats(), r.getDiskThresholdSettings()), failures) - ) + actionContext + // merge in the stats from the master, if available + .andThenApply( + getAllocationStatsResponse -> new NodesStatsResponse( + clusterService.getClusterName(), + getAllocationStatsResponse == null + ? responses + : merge( + responses, + getAllocationStatsResponse.getNodeAllocationStats(), + getAllocationStatsResponse.getDiskThresholdSettings() + ), + failures ) - ); - } else { - ActionListener.run(listener, l -> ActionListener.respondAndRelease(l, newResponse(request, responses, failures))); - } + ) + .addListener(listener); } private static List merge( From ce79fa484780147f30ee7fd12d70457917175d20 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Mon, 23 Sep 2024 10:19:00 +0200 Subject: [PATCH 46/46] Add view_index_matadata connector permission for fleet-server account (#113262) * Add view_index_matadata to fleet-server for elastic_connetors package * Fix typo --- .../rest-api/security/get-service-accounts.asciidoc | 6 ++++-- .../xpack/security/authc/service/ServiceAccountIT.java | 6 ++++-- .../security/authc/service/ElasticServiceAccounts.java | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/reference/rest-api/security/get-service-accounts.asciidoc b/docs/reference/rest-api/security/get-service-accounts.asciidoc index 3a14278fb4cfb..74f98f2602e34 100644 --- a/docs/reference/rest-api/security/get-service-accounts.asciidoc +++ b/docs/reference/rest-api/security/get-service-accounts.asciidoc @@ -250,7 +250,8 @@ GET /_security/service/elastic/fleet-server "monitor", "create_index", "auto_configure", - "maintenance" + "maintenance", + "view_index_metadata" ], "allow_restricted_indices": false }, @@ -265,7 +266,8 @@ GET /_security/service/elastic/fleet-server "monitor", "create_index", "auto_configure", - "maintenance" + "maintenance", + "view_index_metadata" ], "allow_restricted_indices": false } diff --git a/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java b/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java index 595d48ea92a44..b7fb8c37f4c45 100644 --- a/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java +++ b/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java @@ -296,7 +296,8 @@ public class ServiceAccountIT extends ESRestTestCase { "monitor", "create_index", "auto_configure", - "maintenance" + "maintenance", + "view_index_metadata" ], "allow_restricted_indices": false }, @@ -311,7 +312,8 @@ public class ServiceAccountIT extends ESRestTestCase { "monitor", "create_index", "auto_configure", - "maintenance" + "maintenance", + "view_index_metadata" ], "allow_restricted_indices": false } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java index baa920eee275b..67211ec6135be 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java @@ -160,12 +160,12 @@ final class ElasticServiceAccounts { // Custom permissions required for running Elastic connectors integration RoleDescriptor.IndicesPrivileges.builder() .indices(".elastic-connectors*") - .privileges("read", "write", "monitor", "create_index", "auto_configure", "maintenance") + .privileges("read", "write", "monitor", "create_index", "auto_configure", "maintenance", "view_index_metadata") .build(), // Permissions for data indices and access control filters used by Elastic connectors integration RoleDescriptor.IndicesPrivileges.builder() .indices("content-*", ".search-acl-filter-*") - .privileges("read", "write", "monitor", "create_index", "auto_configure", "maintenance") + .privileges("read", "write", "monitor", "create_index", "auto_configure", "maintenance", "view_index_metadata") .build(), }, new RoleDescriptor.ApplicationResourcePrivileges[] { RoleDescriptor.ApplicationResourcePrivileges.builder()