From e7d198128e9266b18344aeb5dd90aaef9988645f Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Mon, 26 Sep 2022 08:30:06 +0200 Subject: [PATCH 01/30] EQL: fix spec test exampleWithTwoStagesAndKeyFromTheDocs (#90053) --- .../assembler/SequenceSpecTests.java | 11 +++++++++-- .../src/test/resources/sequences.series-spec | 19 +++++++++---------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/assembler/SequenceSpecTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/assembler/SequenceSpecTests.java index cf9c1bbc3eaef..336c4a5a18543 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/assembler/SequenceSpecTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/execution/assembler/SequenceSpecTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.search.SearchResponse.Clusters; import org.elasticsearch.action.search.SearchResponseSections; import org.elasticsearch.common.breaker.NoopCircuitBreaker; +import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; @@ -38,6 +39,8 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -57,6 +60,8 @@ public class SequenceSpecTests extends ESTestCase { private static final String PARAM_FORMATTING = "%1$s"; private static final String QUERIES_FILENAME = "sequences.series-spec"; + private static final String KEY_FIELD_NAME = "key"; + private final List>> events; private final List> matches; private final Map allEvents; @@ -95,7 +100,7 @@ public Timestamp extract(SearchHit hit) { static class KeyExtractor extends EmptyHitExtractor { @Override public String extract(SearchHit hit) { - return hit.getId(); + return hit.getFields().get(KEY_FIELD_NAME).getValues().get(0).toString(); } } @@ -180,8 +185,10 @@ static class EventsAsHits { for (Entry> entry : events.entrySet()) { Tuple value = entry.getValue(); + Map documentFields = new HashMap<>(); + documentFields.put(KEY_FIELD_NAME, new DocumentField(KEY_FIELD_NAME, Collections.singletonList(value.v1()))); // save the timestamp both as docId (int) and as id (string) - SearchHit searchHit = new SearchHit(entry.getKey(), entry.getKey().toString(), null, null); + SearchHit searchHit = new SearchHit(entry.getKey(), entry.getKey().toString(), documentFields, null); hits.add(searchHit); } } diff --git a/x-pack/plugin/eql/src/test/resources/sequences.series-spec b/x-pack/plugin/eql/src/test/resources/sequences.series-spec index 0b251bfffbc2e..a05f2c40cb689 100644 --- a/x-pack/plugin/eql/src/test/resources/sequences.series-spec +++ b/x-pack/plugin/eql/src/test/resources/sequences.series-spec @@ -248,13 +248,12 @@ A4 B5 C7 D8 // https://eql.readthedocs.io/en/0.8/query-guide/implementation.html#sequences -// AwaitsFix: https://github.com/elastic/elasticsearch/issues/67102 -//exampleWithTwoStagesAndKeyFromTheDocs -// -//r|W1 r|W2 u|W6 r|W7 x|X99 -// u|H3 r|H4 r|H5 u|H8 -// r|I9 u|I10 r|I11 -//; -//r|W2 H4 I9 -//u|W6 H8 I10 -//; +exampleWithTwoStagesAndKeyFromTheDocs + +r|W1 r|W2 u|W6 r|W7 x|X99 + u|H3 r|H4 r|H5 u|H8 + r|I9 u|I10 r|I11 +; +r|W2 H4 I9 +u|W6 H8 I10 +; From dd82fe4fbcd78f32968d8ed1665815f88e2a8752 Mon Sep 17 00:00:00 2001 From: weizijun Date: Mon, 26 Sep 2022 15:00:27 +0800 Subject: [PATCH 02/30] [CI] Fix `DownsampleActionSingleNodeTests#testCannotRollupWhileOtherRollupInProgress` (#90209) Fixes #90197 The reason why this test some times fails is that, the second rollup call is so close to the first rollup call. Add a check, that the first rollup has already created the rollup index, then call the next rollup. --- .../xpack/downsample/DownsampleActionSingleNodeTests.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java index 5a82b40c90510..7a9d7d1c7be45 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java @@ -496,7 +496,6 @@ public void testCannotRollupMissingIndex() { assertThat(exception.getMessage(), containsString("no such index [missing-index]")); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/90197") public void testCannotRollupWhileOtherRollupInProgress() throws Exception { DownsampleConfig config = new DownsampleConfig(randomInterval()); SourceSupplier sourceSupplier = () -> XContentFactory.jsonBuilder() @@ -525,6 +524,13 @@ public void onFailure(Exception e) { } }; client().execute(DownsampleAction.INSTANCE, new DownsampleAction.Request(sourceIndex, rollupIndex, config), rollupListener); + assertBusy(() -> { + try { + assertEquals(client().admin().indices().prepareGetIndex().addIndices(rollupIndex).get().getIndices().length, 1); + } catch (IndexNotFoundException e) { + fail("rollup index has not been created"); + } + }); ResourceAlreadyExistsException exception = expectThrows( ResourceAlreadyExistsException.class, () -> rollup(sourceIndex, rollupIndex, config) From fdfb3b53ac96f865d9964d4147ffb1555109d5cf Mon Sep 17 00:00:00 2001 From: Ievgen Degtiarenko Date: Mon, 26 Sep 2022 10:02:57 +0200 Subject: [PATCH 03/30] Introduce equals and hashCode in ClusterInfo to simplify its testing (#90221) --- .../elasticsearch/cluster/ClusterInfo.java | 50 ++++++++++++++++--- .../cluster/ClusterInfoTests.java | 30 +++++------ 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java b/server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java index 4d1b9be2c4d90..cdce668b1d11b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java @@ -186,17 +186,17 @@ public Map getNodeMostAvailableDiskUsages() { } /** - * Returns the shard size for the given shard routing or null it that metric is not available. + * Returns the shard size for the given shardId or null if that metric is not available. */ - public Long getShardSize(ShardRouting shardRouting) { - return shardSizes.get(shardIdentifierFromRouting(shardRouting)); + public Long getShardSize(ShardId shardId, boolean primary) { + return shardSizes.get(shardIdentifierFromRouting(shardId, primary)); } /** - * Returns the nodes absolute data-path the given shard is allocated on or null if the information is not available. + * Returns the shard size for the given shard routing or null if that metric is not available. */ - public String getDataPath(ShardRouting shardRouting) { - return routingToDataPath.get(shardRouting); + public Long getShardSize(ShardRouting shardRouting) { + return getShardSize(shardRouting.shardId(), shardRouting.primary()); } /** @@ -207,6 +207,13 @@ public long getShardSize(ShardRouting shardRouting, long defaultValue) { return shardSize == null ? defaultValue : shardSize; } + /** + * Returns the nodes absolute data-path the given shard is allocated on or null if the information is not available. + */ + public String getDataPath(ShardRouting shardRouting) { + return routingToDataPath.get(shardRouting); + } + public Optional getShardDataSetSize(ShardId shardId) { return Optional.ofNullable(shardDataSetSizes.get(shardId)); } @@ -224,7 +231,36 @@ public ReservedSpace getReservedSpace(String nodeId, String dataPath) { * includes a 'p' or 'r' depending on whether the shard is a primary. */ public static String shardIdentifierFromRouting(ShardRouting shardRouting) { - return shardRouting.shardId().toString() + "[" + (shardRouting.primary() ? "p" : "r") + "]"; + return shardIdentifierFromRouting(shardRouting.shardId(), shardRouting.primary()); + } + + public static String shardIdentifierFromRouting(ShardId shardId, boolean primary) { + return shardId.toString() + "[" + (primary ? "p" : "r") + "]"; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ClusterInfo that = (ClusterInfo) o; + return leastAvailableSpaceUsage.equals(that.leastAvailableSpaceUsage) + && mostAvailableSpaceUsage.equals(that.mostAvailableSpaceUsage) + && shardSizes.equals(that.shardSizes) + && shardDataSetSizes.equals(that.shardDataSetSizes) + && routingToDataPath.equals(that.routingToDataPath) + && reservedSpace.equals(that.reservedSpace); + } + + @Override + public int hashCode() { + return Objects.hash( + leastAvailableSpaceUsage, + mostAvailableSpaceUsage, + shardSizes, + shardDataSetSizes, + routingToDataPath, + reservedSpace + ); } @Override diff --git a/server/src/test/java/org/elasticsearch/cluster/ClusterInfoTests.java b/server/src/test/java/org/elasticsearch/cluster/ClusterInfoTests.java index 682b9424a0ad2..889d65444af50 100644 --- a/server/src/test/java/org/elasticsearch/cluster/ClusterInfoTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/ClusterInfoTests.java @@ -10,17 +10,24 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.routing.TestShardRouting; -import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.AbstractWireSerializingTestCase; +import java.io.IOException; import java.util.HashMap; import java.util.Map; -public class ClusterInfoTests extends ESTestCase { +public class ClusterInfoTests extends AbstractWireSerializingTestCase { - public void testSerialization() throws Exception { - ClusterInfo clusterInfo = new ClusterInfo( + @Override + protected Writeable.Reader instanceReader() { + return ClusterInfo::new; + } + + @Override + protected ClusterInfo createTestInstance() { + return new ClusterInfo( randomDiskUsage(), randomDiskUsage(), randomShardSizes(), @@ -28,16 +35,11 @@ public void testSerialization() throws Exception { randomRoutingToDataPath(), randomReservedSpace() ); - BytesStreamOutput output = new BytesStreamOutput(); - clusterInfo.writeTo(output); + } - ClusterInfo result = new ClusterInfo(output.bytes().streamInput()); - assertEquals(clusterInfo.getNodeLeastAvailableDiskUsages(), result.getNodeLeastAvailableDiskUsages()); - assertEquals(clusterInfo.getNodeMostAvailableDiskUsages(), result.getNodeMostAvailableDiskUsages()); - assertEquals(clusterInfo.shardSizes, result.shardSizes); - assertEquals(clusterInfo.shardDataSetSizes, result.shardDataSetSizes); - assertEquals(clusterInfo.routingToDataPath, result.routingToDataPath); - assertEquals(clusterInfo.reservedSpace, result.reservedSpace); + @Override + protected ClusterInfo mutateInstance(ClusterInfo instance) throws IOException { + return createTestInstance(); } private static Map randomDiskUsage() { From 2a701bae19d9f63ef3ef25b69db658fa3f576ff4 Mon Sep 17 00:00:00 2001 From: Iraklis Psaroudakis Date: Mon, 26 Sep 2022 11:03:11 +0300 Subject: [PATCH 04/30] Nodes test to use 4 shards (#90297) Fixes #90228 --- .../rest-api-spec/test/nodes.stats/11_indices_metrics.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/nodes.stats/11_indices_metrics.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/nodes.stats/11_indices_metrics.yml index 2c6879e5a695e..b119a1a1d94f3 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/nodes.stats/11_indices_metrics.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/nodes.stats/11_indices_metrics.yml @@ -425,8 +425,8 @@ index: index1 body: settings: - number_of_shards: "3" - number_of_replicas: "1" + number_of_shards: 4 + number_of_replicas: 0 mappings: runtime: a_source_field: @@ -519,8 +519,8 @@ index: index1 body: settings: - number_of_shards: "3" - number_of_replicas: "1" + number_of_shards: 4 + number_of_replicas: 0 mappings: properties: prop1: From 3178a787612913ee071258243c535eddb654fa1d Mon Sep 17 00:00:00 2001 From: Ievgen Degtiarenko Date: Mon, 26 Sep 2022 10:03:24 +0200 Subject: [PATCH 05/30] Remove ShardRouting assertion that is always true (#90254) --- .../java/org/elasticsearch/cluster/routing/ShardRouting.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java index 6c36490af2b5e..8301125c8e947 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java @@ -74,8 +74,6 @@ public final class ShardRouting implements Writeable, ToXContentObject { this.allocationId = allocationId; this.expectedShardSize = expectedShardSize; this.targetRelocatingShard = initializeTargetRelocatingShard(); - assert expectedShardSize >= 0 || state != ShardRoutingState.INITIALIZING || state != ShardRoutingState.RELOCATING - : expectedShardSize + " state: " + state; assert (state == ShardRoutingState.UNASSIGNED && unassignedInfo == null) == false : "unassigned shard must be created with meta"; assert (state == ShardRoutingState.UNASSIGNED || state == ShardRoutingState.INITIALIZING) == (recoverySource != null) : "recovery source only available on unassigned or initializing shard but was " + state; From 17ad718c9a4fc846c8188397846b3f9711f97695 Mon Sep 17 00:00:00 2001 From: Ievgen Degtiarenko Date: Mon, 26 Sep 2022 10:03:44 +0200 Subject: [PATCH 06/30] Replace intermediate abstract class with default methods (#90304) --- .../routing/RoutingChangesObserver.java | 70 +++---------------- .../allocation/IndexMetadataUpdater.java | 2 +- .../snapshots/RestoreService.java | 2 +- .../NodeVersionAllocationDeciderTests.java | 3 +- 4 files changed, 13 insertions(+), 64 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingChangesObserver.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingChangesObserver.java index bc5c41c014896..acdcba613c776 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingChangesObserver.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingChangesObserver.java @@ -15,101 +15,49 @@ public interface RoutingChangesObserver { /** * Called when unassigned shard is initialized. Does not include initializing relocation target shards. */ - void shardInitialized(ShardRouting unassignedShard, ShardRouting initializedShard); + default void shardInitialized(ShardRouting unassignedShard, ShardRouting initializedShard) {} /** * Called when an initializing shard is started. */ - void shardStarted(ShardRouting initializingShard, ShardRouting startedShard); + default void shardStarted(ShardRouting initializingShard, ShardRouting startedShard) {} /** * Called when relocation of a started shard is initiated. */ - void relocationStarted(ShardRouting startedShard, ShardRouting targetRelocatingShard); + default void relocationStarted(ShardRouting startedShard, ShardRouting targetRelocatingShard) {} /** * Called when an unassigned shard's unassigned information was updated */ - void unassignedInfoUpdated(ShardRouting unassignedShard, UnassignedInfo newUnassignedInfo); + default void unassignedInfoUpdated(ShardRouting unassignedShard, UnassignedInfo newUnassignedInfo) {} /** * Called when a shard is failed or cancelled. */ - void shardFailed(ShardRouting failedShard, UnassignedInfo unassignedInfo); + default void shardFailed(ShardRouting failedShard, UnassignedInfo unassignedInfo) {} /** * Called on relocation source when relocation completes after relocation target is started. */ - void relocationCompleted(ShardRouting removedRelocationSource); + default void relocationCompleted(ShardRouting removedRelocationSource) {} /** * Called on replica relocation target when replica relocation source fails. Promotes the replica relocation target to ordinary * initializing shard. */ - void relocationSourceRemoved(ShardRouting removedReplicaRelocationSource); + default void relocationSourceRemoved(ShardRouting removedReplicaRelocationSource) {} /** * Called when started replica is promoted to primary. */ - void replicaPromoted(ShardRouting replicaShard); + default void replicaPromoted(ShardRouting replicaShard) {} /** * Called when an initializing replica is reinitialized. This happens when a primary relocation completes, which * reinitializes all currently initializing replicas as their recovery source node changes */ - void initializedReplicaReinitialized(ShardRouting oldReplica, ShardRouting reinitializedReplica); - - /** - * Abstract implementation of {@link RoutingChangesObserver} that does not take any action. Useful for subclasses that only override - * certain methods. - */ - class AbstractRoutingChangesObserver implements RoutingChangesObserver { - - @Override - public void shardInitialized(ShardRouting unassignedShard, ShardRouting initializedShard) { - - } - - @Override - public void shardStarted(ShardRouting initializingShard, ShardRouting startedShard) { - - } - - @Override - public void relocationStarted(ShardRouting startedShard, ShardRouting targetRelocatingShard) { - - } - - @Override - public void unassignedInfoUpdated(ShardRouting unassignedShard, UnassignedInfo newUnassignedInfo) { - - } - - @Override - public void shardFailed(ShardRouting activeShard, UnassignedInfo unassignedInfo) { - - } - - @Override - public void relocationCompleted(ShardRouting removedRelocationSource) { - - } - - @Override - public void relocationSourceRemoved(ShardRouting removedReplicaRelocationSource) { - - } - - @Override - public void replicaPromoted(ShardRouting replicaShard) { - - } - - @Override - public void initializedReplicaReinitialized(ShardRouting oldReplica, ShardRouting reinitializedReplica) { - - } - } + default void initializedReplicaReinitialized(ShardRouting oldReplica, ShardRouting reinitializedReplica) {} class DelegatingRoutingChangesObserver implements RoutingChangesObserver { diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetadataUpdater.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetadataUpdater.java index 980324c1611b9..469e7f7efe36c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetadataUpdater.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetadataUpdater.java @@ -40,7 +40,7 @@ * * Allocation ids are added for shards that become active and removed for shards that stop being active. */ -public class IndexMetadataUpdater extends RoutingChangesObserver.AbstractRoutingChangesObserver { +public class IndexMetadataUpdater implements RoutingChangesObserver { private final Map shardChanges = new HashMap<>(); @Override diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index bb0587b756ca4..7d9f859b6ce82 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -781,7 +781,7 @@ public RestoreInfo getRestoreInfo() { } } - public static class RestoreInProgressUpdater extends RoutingChangesObserver.AbstractRoutingChangesObserver { + public static class RestoreInProgressUpdater implements RoutingChangesObserver { // Map of RestoreUUID to a of changes to the shards' restore statuses private final Map> shardChanges = new HashMap<>(); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/NodeVersionAllocationDeciderTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/NodeVersionAllocationDeciderTests.java index bbf624d2cb2da..edf6ca103598e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/NodeVersionAllocationDeciderTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/NodeVersionAllocationDeciderTests.java @@ -674,7 +674,8 @@ public void testMessages() { ) ); - final RoutingChangesObserver routingChangesObserver = new RoutingChangesObserver.AbstractRoutingChangesObserver(); + final RoutingChangesObserver routingChangesObserver = new RoutingChangesObserver() { + }; final RoutingNodes routingNodes = clusterState.mutableRoutingNodes(); final ShardRouting startedPrimary = routingNodes.startShard( logger, From 81a8b7dc6b8ffaad5644b988d1efe82bc80b157a Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Mon, 26 Sep 2022 11:18:50 +0300 Subject: [PATCH 07/30] [ML] Do not make autoscaling decision when memory is undetermined (#90259) When a processor autoscaling decider was added in #89645, an unwanted change of behaviour sneaked in. In particular, if we cannot determine required memory capacity, we previously returned `new AutoscalingDeciderResult(null)` where as we now return an autoscaling result with no memory capacity and whatever the result of the processor decider is. Previously, if we returned a result with null capacity, the cluster would remain as-is. Now, it is possible to cause unwanted scaling. This commit fixes this by checking if the memory decider result was undetermined and returns an empty result if so. Also, some logging warnings have been added to pop up scenarios that shouldn't happen like when the memory tracker is not called by the master node or it has no memory estimate for anomaly detection or analytics jobs. --- .../MlAutoscalingDeciderService.java | 18 +++++-- .../MlMemoryAutoscalingCapacity.java | 5 ++ .../MlMemoryAutoscalingDecider.java | 20 +++++-- .../xpack/ml/process/MlMemoryTracker.java | 14 +++-- .../MlAutoscalingDeciderServiceTests.java | 52 +++++++++++++++++++ 5 files changed, 97 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderService.java index 87c7cdaeb0de1..9ac2513f9ed02 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderService.java @@ -25,9 +25,10 @@ import java.time.Instant; import java.util.List; -import java.util.Locale; import java.util.function.LongSupplier; +import static org.elasticsearch.core.Strings.format; + public class MlAutoscalingDeciderService implements AutoscalingDeciderService, LocalNodeMasterListener { private static final Logger logger = LogManager.getLogger(MlAutoscalingDeciderService.class); @@ -120,9 +121,19 @@ public AutoscalingDeciderResult scale(Settings configuration, AutoscalingDecider } MlMemoryAutoscalingCapacity memoryCapacity = memoryDecider.scale(configuration, context, mlContext); + if (memoryCapacity.isUndetermined()) { + // If we cannot determine memory capacity we shouldn't make any autoscaling decision + // as it could lead to undesired capacity. For example, it could be that the processor decider decides + // to scale down the cluster but as soon as we can determine memory requirements again we need to scale + // back up. + return new AutoscalingDeciderResult( + null, + reasonBuilder.setSimpleReason(format("[memory_decider] %s", memoryCapacity.reason())).build() + ); + } MlProcessorAutoscalingCapacity processorCapacity = processorDecider.scale(configuration, context, mlContext); reasonBuilder.setSimpleReason( - String.format(Locale.ROOT, "[memory_decider] %s; [processor_decider] %s", memoryCapacity.reason(), processorCapacity.reason()) + format("[memory_decider] %s; [processor_decider] %s", memoryCapacity.reason(), processorCapacity.reason()) ); return new AutoscalingDeciderResult( @@ -153,8 +164,7 @@ private AutoscalingDeciderResult downscaleToZero( return new AutoscalingDeciderResult( context.currentCapacity(), reasonBuilder.setSimpleReason( - String.format( - Locale.ROOT, + format( "Passing currently perceived capacity as down scale delay has not been satisfied; configured delay [%s] " + "last detected scale down event [%s]. Will request scale down in approximately [%s]", DOWN_SCALE_DELAY.get(configuration).getStringRep(), diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingCapacity.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingCapacity.java index 3fbbd4b1e92c8..bab7bb52f928f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingCapacity.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingCapacity.java @@ -25,6 +25,10 @@ public String toString() { return "MlMemoryAutoscalingCapacity{" + "nodeSize=" + nodeSize + ", tierSize=" + tierSize + ", reason='" + reason + '\'' + '}'; } + public boolean isUndetermined() { + return nodeSize == null && tierSize == null; + } + public static class Builder { private ByteSizeValue nodeSize; @@ -32,6 +36,7 @@ public static class Builder { private String reason; public Builder(ByteSizeValue nodeSize, ByteSizeValue tierSize) { + assert (nodeSize == null) == (tierSize == null) : "nodeSize " + nodeSize + " tierSize " + tierSize; this.nodeSize = nodeSize; this.tierSize = tierSize; } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingDecider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingDecider.java index 03fc96ad70204..5a5c89c985f01 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingDecider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/MlMemoryAutoscalingDecider.java @@ -212,8 +212,9 @@ public MlMemoryAutoscalingCapacity scale(Settings configuration, AutoscalingDeci long maxTaskMemoryBytes = maxMemoryBytes(mlContext); - // This state is invalid, but may occur due to complex bugs that have slipped through testing. - // We could have tasks where the required job memory is 0, which should be impossible. + // This should rarely happen, it could imply a bug. However, it is possible to happen + // if there are persistent tasks that do not have matching configs stored. + // Also, it could be that we have tasks where the required job memory is 0, which should be impossible. // This can also happen if a job that is awaiting assignment ceases to have the AWAITING_LAZY_ASSIGNMENT // assignment explanation, for example because some other explanation overrides it. (This second situation // arises because, for example, anomalyDetectionTasks contains a task that is waiting but waitingAnomalyJobs @@ -346,8 +347,11 @@ private long maxMemoryBytes(MlAutoscalingContext mlContext) { // Memory SHOULD be recently refreshed, so in our current state, we should at least have an idea of the memory used .mapToLong(t -> { Long mem = getAnomalyMemoryRequirement(t); + if (mem == null) { + logger.warn("unexpected null for anomaly detection memory requirement for [{}]", MlTasks.jobId(t.getId())); + } assert mem != null : "unexpected null for anomaly memory requirement after recent stale check"; - return mem; + return mem == null ? 0 : mem; }) .max() .orElse(0L), @@ -356,8 +360,11 @@ private long maxMemoryBytes(MlAutoscalingContext mlContext) { // Memory SHOULD be recently refreshed, so in our current state, we should at least have an idea of the memory used .mapToLong(t -> { Long mem = getAnomalyMemoryRequirement(t); + if (mem == null) { + logger.warn("unexpected null for snapshot upgrade memory requirement for [{}]", MlTasks.jobId(t.getId())); + } assert mem != null : "unexpected null for anomaly memory requirement after recent stale check"; - return mem; + return mem == null ? 0 : mem; }) .max() .orElse(0L) @@ -369,8 +376,11 @@ private long maxMemoryBytes(MlAutoscalingContext mlContext) { // Memory SHOULD be recently refreshed, so in our current state, we should at least have an idea of the memory used .mapToLong(t -> { Long mem = this.getAnalyticsMemoryRequirement(t); + if (mem == null) { + logger.warn("unexpected null for analytics memory requirement for [{}]", MlTasks.dataFrameAnalyticsId(t.getId())); + } assert mem != null : "unexpected null for analytics memory requirement after recent stale check"; - return mem; + return mem == null ? 0 : mem; }) .max() .orElse(0L) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/MlMemoryTracker.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/MlMemoryTracker.java index 588edd859750b..664283c825523 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/MlMemoryTracker.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/MlMemoryTracker.java @@ -262,6 +262,7 @@ public Long getDataFrameAnalyticsJobMemoryRequirement(String id) { */ public Long getTrainedModelAssignmentMemoryRequirement(String modelId) { if (isMaster == false) { + logger.warn("Request to get trained model assignment memory not on master node; modelId was [{}]", modelId); return null; } @@ -282,6 +283,7 @@ public Long getTrainedModelAssignmentMemoryRequirement(String modelId) { public Long getJobMemoryRequirement(String taskName, String id) { if (isMaster == false) { + logger.warn("Request to get job memory not on master node; taskName [{}], id [{}]", taskName, id); return null; } @@ -353,7 +355,9 @@ public boolean asyncRefresh() { public void refreshAnomalyDetectorJobMemoryAndAllOthers(String jobId, ActionListener listener) { if (isMaster == false) { - listener.onFailure(new NotMasterException("Request to refresh anomaly detector memory requirements on non-master node")); + String msg = "Request to refresh anomaly detector memory requirements on non-master node"; + logger.warn(msg); + listener.onFailure(new NotMasterException(msg)); return; } @@ -377,7 +381,9 @@ public void refreshAnomalyDetectorJobMemoryAndAllOthers(String jobId, ActionList public void addDataFrameAnalyticsJobMemoryAndRefreshAllOthers(String id, long mem, ActionListener listener) { if (isMaster == false) { - listener.onFailure(new NotMasterException("Request to put data frame analytics memory requirement on non-master node")); + String msg = "Request to put data frame analytics memory requirement on non-master node"; + logger.warn(msg); + listener.onFailure(new NotMasterException(msg)); return; } @@ -517,7 +523,9 @@ private void refreshAllDataFrameAnalyticsJobTasks( */ public void refreshAnomalyDetectorJobMemory(String jobId, ActionListener listener) { if (isMaster == false) { - listener.onFailure(new NotMasterException("Request to refresh anomaly detector memory requirement on non-master node")); + String msg = "Request to refresh anomaly detector memory requirement on non-master node"; + logger.warn(msg); + listener.onFailure(new NotMasterException(msg)); return; } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderServiceTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderServiceTests.java index e2758319fe27e..da5332b32a819 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderServiceTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/autoscaling/MlAutoscalingDeciderServiceTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.cluster.ClusterInfo; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -21,14 +22,17 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; +import org.elasticsearch.persistent.PersistentTasksCustomMetadata; import org.elasticsearch.snapshots.SnapshotShardSizeInfo; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingCapacity; import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderContext; import org.elasticsearch.xpack.autoscaling.capacity.AutoscalingDeciderResult; +import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.job.NodeLoad; import org.elasticsearch.xpack.ml.job.NodeLoadDetector; +import org.elasticsearch.xpack.ml.job.task.OpenJobPersistentTasksExecutorTests; import org.elasticsearch.xpack.ml.process.MlMemoryTracker; import org.junit.Before; @@ -43,6 +47,8 @@ import static org.elasticsearch.xpack.ml.utils.NativeMemoryCalculator.STATIC_JVM_UPPER_THRESHOLD; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -208,6 +214,52 @@ public void testScale_GivenNoML_AndNoMLNodes() { assertThat(result.reason().summary(), containsString("Passing currently perceived capacity as no scaling changes are necessary")); } + public void testScale_GivenUndeterminedMemory_ShouldReturnNullCapacity() { + MlAutoscalingDeciderService service = buildService(); + service.onMaster(); + + String jobId = "a_job"; + PersistentTasksCustomMetadata.Builder tasksBuilder = PersistentTasksCustomMetadata.builder(); + OpenJobPersistentTasksExecutorTests.addJobTask(jobId, randomFrom("ml-1", "ml-2"), JobState.OPENED, tasksBuilder); + Metadata.Builder metadata = Metadata.builder(); + metadata.putCustom(PersistentTasksCustomMetadata.TYPE, tasksBuilder.build()); + + ClusterState clusterState = ClusterState.builder(new ClusterName("test")) + .nodes( + DiscoveryNodes.builder() + .add(buildNode("ml-1", ByteSizeValue.ofGb(4), 8)) + .add(buildNode("ml-2", ByteSizeValue.ofGb(4), 8)) + .build() + ) + .metadata(metadata) + .build(); + + // Making the memory tracker return 0 for an AD job memory results to the decider return + // undetermined capacity. + when(mlMemoryTracker.getAnomalyDetectorJobMemoryRequirement(jobId)).thenReturn(0L); + + AutoscalingDeciderResult result = service.scale( + Settings.EMPTY, + new DeciderContext( + clusterState, + new AutoscalingCapacity( + new AutoscalingCapacity.AutoscalingResources(null, ByteSizeValue.ofGb(8), null), + new AutoscalingCapacity.AutoscalingResources(null, ByteSizeValue.ofGb(4), null) + ) + ) + ); + + assertThat( + result.reason().summary(), + containsString( + "[memory_decider] Passing currently perceived capacity as there are running analytics " + + "and anomaly jobs or deployed models, but their assignment explanations are unexpected or their " + + "memory usage estimates are inaccurate." + ) + ); + assertThat(result.requiredCapacity(), is(nullValue())); + } + private DiscoveryNode buildNode(String id, ByteSizeValue machineMemory, int allocatedProcessors) { return new DiscoveryNode( id, From ad6b4137cb687247201febc3e6ac8e2bc88782c0 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Mon, 26 Sep 2022 11:15:55 +0200 Subject: [PATCH 08/30] upgrade lucene to 9.4.0-snapshot-f5d0646daa5 (#90230) --- build-tools-internal/version.properties | 2 +- build.gradle | 4 +- gradle/verification-metadata.xml | 120 ++++++++++++++++++ .../index/engine/TranslogDirectoryReader.java | 4 + .../BytesRefFieldComparatorSource.java | 4 +- .../index/mapper/DocumentLeafReader.java | 2 + .../lookup/LeafStoredFieldsLookupTests.java | 2 + .../sourceonly/SourceOnlySnapshot.java | 2 + .../xpack/lucene/bwc/codecs/BWCCodec.java | 1 + .../lucene50/Lucene50FieldInfosFormat.java | 2 + 10 files changed, 137 insertions(+), 6 deletions(-) diff --git a/build-tools-internal/version.properties b/build-tools-internal/version.properties index 5b2a2ddf4f616..efb78f1ffdd5b 100644 --- a/build-tools-internal/version.properties +++ b/build-tools-internal/version.properties @@ -1,5 +1,5 @@ elasticsearch = 8.6.0 -lucene = 9.4.0-snapshot-923a9f800ae +lucene = 9.4.0-snapshot-f5d0646daa5 bundled_jdk_vendor = openjdk bundled_jdk = 18.0.2.1+1@db379da656dc47308e138f21b33976fa diff --git a/build.gradle b/build.gradle index e1e11e60e110e..0b4d9302f5dd6 100644 --- a/build.gradle +++ b/build.gradle @@ -137,9 +137,9 @@ tasks.register("verifyVersions") { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = true +boolean bwc_tests_enabled = false // place a PR link here when committing bwc changes: -String bwc_tests_disabled_issue = "" +String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/90230" if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index a5b3cb5bfbc9e..6f6567e428780 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -2481,6 +2481,11 @@ + + + + + @@ -2491,6 +2496,11 @@ + + + + + @@ -2501,6 +2511,11 @@ + + + + + @@ -2511,6 +2526,11 @@ + + + + + @@ -2521,6 +2541,11 @@ + + + + + @@ -2531,6 +2556,11 @@ + + + + + @@ -2541,6 +2571,11 @@ + + + + + @@ -2551,6 +2586,11 @@ + + + + + @@ -2561,6 +2601,11 @@ + + + + + @@ -2571,6 +2616,11 @@ + + + + + @@ -2581,6 +2631,11 @@ + + + + + @@ -2591,6 +2646,11 @@ + + + + + @@ -2601,6 +2661,11 @@ + + + + + @@ -2611,6 +2676,11 @@ + + + + + @@ -2621,6 +2691,11 @@ + + + + + @@ -2631,6 +2706,11 @@ + + + + + @@ -2641,6 +2721,11 @@ + + + + + @@ -2651,6 +2736,11 @@ + + + + + @@ -2661,6 +2751,11 @@ + + + + + @@ -2671,6 +2766,11 @@ + + + + + @@ -2681,6 +2781,11 @@ + + + + + @@ -2691,6 +2796,11 @@ + + + + + @@ -2701,6 +2811,11 @@ + + + + + @@ -2711,6 +2826,11 @@ + + + + + diff --git a/server/src/main/java/org/elasticsearch/index/engine/TranslogDirectoryReader.java b/server/src/main/java/org/elasticsearch/index/engine/TranslogDirectoryReader.java index eb76f3eee96f4..c377fdd73f3d8 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/TranslogDirectoryReader.java +++ b/server/src/main/java/org/elasticsearch/index/engine/TranslogDirectoryReader.java @@ -32,6 +32,7 @@ import org.apache.lucene.index.StoredFieldVisitor; import org.apache.lucene.index.Terms; import org.apache.lucene.index.TermsEnum; +import org.apache.lucene.index.VectorEncoding; import org.apache.lucene.index.VectorSimilarityFunction; import org.apache.lucene.index.VectorValues; import org.apache.lucene.search.DocIdSetIterator; @@ -155,6 +156,7 @@ private static class TranslogLeafReader extends LeafReader { 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); @@ -172,6 +174,7 @@ private static class TranslogLeafReader extends LeafReader { 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); @@ -189,6 +192,7 @@ private static class TranslogLeafReader extends LeafReader { 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java index bdb299a993c1b..addc6f33c9eba 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java @@ -74,7 +74,7 @@ public FieldComparator newComparator(String fieldname, int numHits, boolean e final boolean sortMissingLast = sortMissingLast(missingValue) ^ reversed; final BytesRef missingBytes = (BytesRef) missingObject(missingValue, reversed); if (indexFieldData instanceof IndexOrdinalsFieldData) { - TermOrdValComparator comparator = new TermOrdValComparator(numHits, null, sortMissingLast, reversed) { + return new TermOrdValComparator(numHits, null, sortMissingLast, reversed, false) { @Override protected SortedDocValues getSortedDocValues(LeafReaderContext context, String field) throws IOException { @@ -98,8 +98,6 @@ protected SortedDocValues getSortedDocValues(LeafReaderContext context, String f } }; - comparator.disableSkipping(); - return comparator; } return new FieldComparator.TermValComparator(numHits, null, sortMissingLast) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java index 51c18ef3c659d..e2a8feb30fa7c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java @@ -25,6 +25,7 @@ import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.index.StoredFieldVisitor; import org.apache.lucene.index.Terms; +import org.apache.lucene.index.VectorEncoding; import org.apache.lucene.index.VectorSimilarityFunction; import org.apache.lucene.index.VectorValues; import org.apache.lucene.index.memory.MemoryIndex; @@ -261,6 +262,7 @@ private static FieldInfo fieldInfo(String name) { 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); diff --git a/server/src/test/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookupTests.java b/server/src/test/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookupTests.java index a9111aa8e8e28..e0a1b55914327 100644 --- a/server/src/test/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookupTests.java +++ b/server/src/test/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookupTests.java @@ -10,6 +10,7 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.VectorEncoding; import org.apache.lucene.index.VectorSimilarityFunction; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.test.ESTestCase; @@ -48,6 +49,7 @@ public void setUp() throws Exception { 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java index ae9a53da0b01f..cb22b11a5be98 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.SegmentInfos; import org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper; import org.apache.lucene.index.StandardDirectoryReader; +import org.apache.lucene.index.VectorEncoding; import org.apache.lucene.index.VectorSimilarityFunction; import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.IndexSearcher; @@ -258,6 +259,7 @@ private SegmentCommitInfo syncSegment( 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, fieldInfo.isSoftDeletesField() ) diff --git a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodec.java b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodec.java index 7faf5a9829ace..5d834e0303a37 100644 --- a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodec.java +++ b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/BWCCodec.java @@ -107,6 +107,7 @@ private static FieldInfos filterFields(FieldInfos fieldInfos) { fieldInfo.getPointIndexDimensionCount(), fieldInfo.getPointNumBytes(), 0, + fieldInfo.getVectorEncoding(), fieldInfo.getVectorSimilarityFunction(), fieldInfo.isSoftDeletesField() ) diff --git a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene50/Lucene50FieldInfosFormat.java b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene50/Lucene50FieldInfosFormat.java index 11966fb396724..9cef274aa753e 100644 --- a/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene50/Lucene50FieldInfosFormat.java +++ b/x-pack/plugin/old-lucene-versions/src/main/java/org/elasticsearch/xpack/lucene/bwc/codecs/lucene50/Lucene50FieldInfosFormat.java @@ -29,6 +29,7 @@ import org.apache.lucene.index.IndexFileNames; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.SegmentInfo; +import org.apache.lucene.index.VectorEncoding; import org.apache.lucene.index.VectorSimilarityFunction; import org.apache.lucene.store.ChecksumIndexInput; import org.apache.lucene.store.Directory; @@ -108,6 +109,7 @@ public FieldInfos read(Directory directory, SegmentInfo segmentInfo, String segm 0, 0, 0, + VectorEncoding.FLOAT32, VectorSimilarityFunction.EUCLIDEAN, false ); From 4134eee9e0c5ce0b9ef9308ab7502c148c3a184e Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Mon, 26 Sep 2022 12:34:17 +0300 Subject: [PATCH 09/30] Mute testNoBlockedIndicesAndRedAllRoleNodes (#90338) Test is in DiskHealthIndicatorServiceTests. Relates #90305 --- .../health/node/DiskHealthIndicatorServiceTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceTests.java b/server/src/test/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceTests.java index f0ac18cf76395..f67e1d239f9ef 100644 --- a/server/src/test/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceTests.java +++ b/server/src/test/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceTests.java @@ -131,6 +131,7 @@ public void testGreen() throws IOException { * This method tests that we get the expected behavior when there are all role nodes with indices that report RED status and there are * no blocks in the cluster state. We expect 3 impacts and 1 diagnosis. */ + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/90305") public void testNoBlockedIndicesAndRedAllRoleNodes() throws IOException { Set discoveryNodes = createNodesWithAllRoles(); Map> indexNameToNodeIdsMap = new HashMap<>(); From dae717ae8d40509e8105e78ede8f84f887078b50 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 26 Sep 2022 11:38:04 +0200 Subject: [PATCH 10/30] Check object depth limit at parse time (#90285) Helps avoiding stack overflow errors when the total field limit is set too high. Relates #87926 --- .../index/mapper/DynamicMappingIT.java | 39 +++++++++++++++---- .../index/mapper/DocumentParserContext.java | 4 ++ .../index/mapper/MappingLookup.java | 26 +++++++------ .../index/mapper/DocumentParserTests.java | 28 +++++++++++++ 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java index d31475a172056..c5d262a088e53 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Randomness; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; @@ -45,7 +46,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; -import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; +import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING; import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; @@ -137,12 +138,34 @@ public void run() { } } - public void testPreflightCheckAvoidsMaster() throws InterruptedException { - // can't use INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING as a check here, as that is already checked at parse time, - // see testTotalFieldsLimitForDynamicMappingsUpdateCheckedAtDocumentParseTime - createIndex("index", Settings.builder().put(INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey(), 2).build()); + public void testPreflightCheckAvoidsMaster() throws InterruptedException, IOException { + // can't use INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING nor INDEX_MAPPING_DEPTH_LIMIT_SETTING as a check here, as that is already + // checked at parse time, see testTotalFieldsLimitForDynamicMappingsUpdateCheckedAtDocumentParseTime + createIndex("index", Settings.builder().put(INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING.getKey(), 2).build()); ensureGreen("index"); - client().prepareIndex("index").setId("1").setSource("field1", Map.of("field2", "value1")).get(); + client().admin() + .indices() + .preparePutMapping("index") + .setSource( + Strings.toString( + XContentFactory.jsonBuilder() + .startObject() + .startArray("dynamic_templates") + .startObject() + .startObject("test") + .field("match", "nested*") + .startObject("mapping") + .field("type", "nested") + .endObject() + .endObject() + .endObject() + .endArray() + .endObject() + ), + XContentType.JSON + ) + .get(); + client().prepareIndex("index").setId("1").setSource("nested1", Map.of("foo", "bar"), "nested2", Map.of("foo", "bar")).get(); final CountDownLatch masterBlockedLatch = new CountDownLatch(1); final CountDownLatch indexingCompletedLatch = new CountDownLatch(1); @@ -165,11 +188,11 @@ public void onFailure(Exception e) { masterBlockedLatch.await(); final IndexRequestBuilder indexRequestBuilder = client().prepareIndex("index") .setId("2") - .setSource("field1", Map.of("field3", Map.of("field4", "value2"))); + .setSource("nested3", Map.of("foo", "bar")); try { assertThat( expectThrows(IllegalArgumentException.class, () -> indexRequestBuilder.get(TimeValue.timeValueSeconds(10))).getMessage(), - Matchers.containsString("Limit of mapping depth [2] has been exceeded due to object field [field1.field3]") + Matchers.containsString("Limit of nested fields [2] has been exceeded") ); } finally { indexingCompletedLatch.countDown(); 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 a79bc9003ef75..b2e8990144170 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java @@ -226,6 +226,10 @@ public final String documentDescription() { * Add a new mapper dynamically created while parsing. */ public final void addDynamicMapper(Mapper mapper) { + // eagerly check object depth limit here to avoid stack overflow errors + if (mapper instanceof ObjectMapper) { + MappingLookup.checkObjectDepthLimit(indexSettings().getMappingDepthLimit(), mapper.name()); + } // eagerly check field name limit here to avoid OOM errors // only check fields that are not already mapped or tracked in order to avoid hitting field limit too early via double-counting // note that existing fields can also receive dynamic mapping updates (e.g. constant_keyword to fix the value) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index 6cbb903a6251f..e112a676ad38d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -294,19 +294,23 @@ private void checkDimensionFieldLimit(long limit) { private void checkObjectDepthLimit(long limit) { for (String objectPath : objectMappers.keySet()) { - int numDots = 0; - for (int i = 0; i < objectPath.length(); ++i) { - if (objectPath.charAt(i) == '.') { - numDots += 1; - } - } - final int depth = numDots + 2; - if (depth > limit) { - throw new IllegalArgumentException( - "Limit of mapping depth [" + limit + "] has been exceeded due to object field [" + objectPath + "]" - ); + checkObjectDepthLimit(limit, objectPath); + } + } + + static void checkObjectDepthLimit(long limit, String objectPath) { + int numDots = 0; + for (int i = 0; i < objectPath.length(); ++i) { + if (objectPath.charAt(i) == '.') { + numDots += 1; } } + final int depth = numDots + 2; + if (depth > limit) { + throw new IllegalArgumentException( + "Limit of mapping depth [" + limit + "] has been exceeded due to object field [" + objectPath + "]" + ); + } } private void checkFieldNameLengthLimit(long limit) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index e72add443eafc..17ebe2d153cc3 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -2407,6 +2407,34 @@ same name need to be part of the same mappings (hence the same document). If th assertArrayEquals(new long[] { 10, 10 }, longs2); } + public void testDeeplyNestedDocument() throws Exception { + int depth = 10000; + + DocumentMapper docMapper = createMapperService(Settings.builder().put(getIndexSettings()).build(), mapping(b -> {})) + .documentMapper(); + // hits the mapping object depth limit (defaults to 20) + MapperParsingException mpe = expectThrows(MapperParsingException.class, () -> docMapper.parse(source(b -> { + for (int i = 0; i < depth; i++) { + b.startObject("obj"); + } + b.field("foo", 10); + for (int i = 0; i < depth; i++) { + b.endObject(); + } + }))); + assertThat(mpe.getCause().getMessage(), containsString("Limit of mapping depth [20] has been exceeded due to object field")); + + // check that multiple-dotted field name underneath an object mapper with subobjects=false does not trigger this + DocumentMapper docMapper2 = createMapperService(topMapping(b -> { b.field("subobjects", false); })).documentMapper(); + + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < depth; i++) { + sb.append("obj."); + } + sb.append("foo"); + docMapper2.parse(source(b -> { b.field(sb.toString(), 10); })); + } + /** * Mapper plugin providing a mock metadata field mapper implementation that supports setting its value */ From 99d51df238252ee40b7da71225179677d2626e0f Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Mon, 26 Sep 2022 11:38:44 +0200 Subject: [PATCH 11/30] Use IndexOrDocValues query for IP range queries (#90303) Closes #83658 --- docs/changelog/90303.yaml | 6 +++ .../index/mapper/IpFieldMapper.java | 7 ++- .../index/mapper/IpFieldTypeTests.java | 44 ++++++++++++++----- 3 files changed, 44 insertions(+), 13 deletions(-) create mode 100644 docs/changelog/90303.yaml diff --git a/docs/changelog/90303.yaml b/docs/changelog/90303.yaml new file mode 100644 index 0000000000000..21292e424ad38 --- /dev/null +++ b/docs/changelog/90303.yaml @@ -0,0 +1,6 @@ +pr: 90303 +summary: Use `IndexOrDocValues` query for IP range queries +area: Search +type: enhancement +issues: + - 83658 diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index d99120221ed6d..1b0821799597a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -13,6 +13,7 @@ import org.apache.lucene.document.SortedSetDocValuesField; import org.apache.lucene.document.StoredField; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.PointRangeQuery; import org.apache.lucene.search.Query; @@ -343,7 +344,11 @@ public Query rangeQuery( return rangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (lower, upper) -> { Query query = InetAddressPoint.newRangeQuery(name(), lower, upper); if (isIndexed()) { - return query; + if (hasDocValues()) { + return new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)); + } else { + return query; + } } else { return convertToDocValuesQuery(query); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldTypeTests.java index ec0348d23146f..a39efded005ec 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldTypeTests.java @@ -11,7 +11,9 @@ import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.common.network.InetAddresses; @@ -134,43 +136,51 @@ public void testTermsQuery() { public void testRangeQuery() { MappedFieldType ft = new IpFieldMapper.IpFieldType("field"); + Query query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddressPoint.MAX_VALUE); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddressPoint.MAX_VALUE), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery(null, null, randomBoolean(), randomBoolean(), null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("192.168.2.0")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("192.168.2.0")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery(null, "192.168.2.0", randomBoolean(), true, null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("192.168.1.255")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("192.168.1.255")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery(null, "192.168.2.0", randomBoolean(), false, null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::"), InetAddressPoint.MAX_VALUE); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::"), InetAddressPoint.MAX_VALUE), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery("2001:db8::", null, true, randomBoolean(), null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::1"), InetAddressPoint.MAX_VALUE); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::1"), InetAddressPoint.MAX_VALUE), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery("2001:db8::", null, false, randomBoolean(), null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::"), InetAddresses.forString("2001:db8::ffff")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::"), InetAddresses.forString("2001:db8::ffff")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery("2001:db8::", "2001:db8::ffff", true, true, null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::1"), InetAddresses.forString("2001:db8::fffe")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::1"), InetAddresses.forString("2001:db8::fffe")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery("2001:db8::", "2001:db8::ffff", false, false, null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::2"), InetAddresses.forString("2001:db8::")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("2001:db8::2"), InetAddresses.forString("2001:db8::")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), // same lo/hi values but inclusive=false so this won't match anything ft.rangeQuery("2001:db8::1", "2001:db8::1", false, false, null, null, null, MOCK_CONTEXT) ); @@ -193,24 +203,34 @@ public void testRangeQuery() { ) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("::fffe:ffff:ffff")); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddresses.forString("::fffe:ffff:ffff")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), // same lo/hi values but inclusive=false so this won't match anything ft.rangeQuery("::", "0.0.0.0", true, false, null, null, null, MOCK_CONTEXT) ); + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::1:0:0:0"), InetAddressPoint.MAX_VALUE); assertEquals( - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::1:0:0:0"), InetAddressPoint.MAX_VALUE), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), // same lo/hi values but inclusive=false so this won't match anything ft.rangeQuery("255.255.255.255", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false, true, null, null, null, MOCK_CONTEXT) ); + // lower bound is ipv4, upper bound is ipv6 + query = InetAddressPoint.newRangeQuery("field", InetAddresses.forString("192.168.1.7"), InetAddresses.forString("2001:db8::")); assertEquals( - // lower bound is ipv4, upper bound is ipv6 - InetAddressPoint.newRangeQuery("field", InetAddresses.forString("192.168.1.7"), InetAddresses.forString("2001:db8::")), + new IndexOrDocValuesQuery(query, convertToDocValuesQuery(query)), ft.rangeQuery("::ffff:c0a8:107", "2001:db8::", true, true, null, null, null, MOCK_CONTEXT) ); + ft = new IpFieldMapper.IpFieldType("field", true, false); + + assertEquals( + InetAddressPoint.newRangeQuery("field", InetAddresses.forString("::"), InetAddressPoint.MAX_VALUE), + ft.rangeQuery(null, null, randomBoolean(), randomBoolean(), null, null, null, MOCK_CONTEXT) + ); + ft = new IpFieldMapper.IpFieldType("field", false); assertEquals( From eb0856393ce4f84f33a9e54a381c03d5007613a4 Mon Sep 17 00:00:00 2001 From: Christos Soulios <1561376+csoulios@users.noreply.github.com> Date: Mon, 26 Sep 2022 15:10:44 +0300 Subject: [PATCH 12/30] [TSDB] Add validations for the downsampling ILM action (#90295) Add validations on downsampling intervals when there are more than phases with downsampling actions (rollups of rollups case). The rules that we enforce are the following: The latter interval must be greater than the previous interval The latter interval must be a multiple of the previous interval --- docs/changelog/90295.yaml | 5 + .../core/ilm/TimeseriesLifecycleType.java | 63 ++++++++++++ .../ilm/TimeseriesLifecycleTypeTests.java | 95 +++++++++++++++++++ .../test/ilm/70_downsampling.yml | 64 +++++++++++++ .../rest-api-spec/test/rollup/10_basic.yml | 4 +- .../downsample/TransportRollupAction.java | 8 +- .../DownsampleActionSingleNodeTests.java | 3 +- 7 files changed, 234 insertions(+), 8 deletions(-) create mode 100644 docs/changelog/90295.yaml create mode 100644 x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/70_downsampling.yml diff --git a/docs/changelog/90295.yaml b/docs/changelog/90295.yaml new file mode 100644 index 0000000000000..02dbbd9290ed0 --- /dev/null +++ b/docs/changelog/90295.yaml @@ -0,0 +1,5 @@ +pr: 90295 +summary: "Add validations for the downsampling ILM action" +area: "ILM+SLM" +type: enhancement +issues: [] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java index 20e980d9bcc4a..88ff518f04272 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.core.Tuple; import java.io.IOException; import java.util.ArrayList; @@ -25,6 +26,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -322,6 +324,7 @@ && definesAllocationRules((AllocateAction) phase.getActions().get(AllocateAction validateActionsFollowingSearchableSnapshot(phases); validateAllSearchableSnapshotActionsUseSameRepository(phases); validateFrozenPhaseHasSearchableSnapshotAction(phases); + validateDownsamplingIntervals(phases); } static void validateActionsFollowingSearchableSnapshot(Collection phases) { @@ -489,6 +492,66 @@ static void validateFrozenPhaseHasSearchableSnapshotAction(Collection pha }); } + /** + * Add validations if there are multiple downsample actions on different phases. The rules that we + * enforce are the following: + * - The latter interval must be greater than the previous interval + * - The latter interval must be a multiple of the previous interval + */ + static void validateDownsamplingIntervals(Collection phases) { + Map phasesWithDownsamplingActions = phases.stream() + .filter(phase -> phase.getActions().containsKey(DownsampleAction.NAME)) + .collect(Collectors.toMap(Phase::getName, Function.identity())); + + if (phasesWithDownsamplingActions.size() < 2) { + // Interval validations must be executed when there are at least two downsample actions, otherwise return + return; + } + + // Order phases and extract the downsample action instances per phase + List orderedPhases = INSTANCE.getOrderedPhases(phasesWithDownsamplingActions); + var downsampleActions = orderedPhases.stream() + .map(phase -> Tuple.tuple(phase.getName(), (DownsampleAction) phase.getActions().get(DownsampleAction.NAME))) + .toList(); // Returns a list of tuples (phase name, downsample action) + + var firstDownsample = downsampleActions.get(0); + for (int i = 1; i < downsampleActions.size(); i++) { + var secondDownsample = downsampleActions.get(i); + var firstInterval = firstDownsample.v2().fixedInterval(); + var secondInterval = secondDownsample.v2().fixedInterval(); + long firstMillis = firstInterval.estimateMillis(); + long secondMillis = secondInterval.estimateMillis(); + if (firstMillis >= secondMillis) { + // The later interval must be greater than the previous interval + throw new IllegalArgumentException( + "Downsampling interval [" + + secondInterval + + "] for phase [" + + secondDownsample.v1() + + "] must be greater than the interval [" + + firstInterval + + "] for phase [" + + firstDownsample.v1() + + "]" + ); + } else if (secondMillis % firstMillis != 0) { + // Downsampling interval must be a multiple of the source interval + throw new IllegalArgumentException( + "Downsampling interval [" + + secondInterval + + "] for phase [" + + secondDownsample.v1() + + "] must be a multiple of the interval [" + + firstInterval + + "] for phase [" + + firstDownsample.v1() + + "]" + ); + } + firstDownsample = secondDownsample; + } + } + private static boolean definesAllocationRules(AllocateAction action) { return action.getRequire().isEmpty() == false || action.getInclude().isEmpty() == false || action.getExclude().isEmpty() == false; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java index 9d1168c122b29..423c16dc81d2c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java @@ -360,6 +360,101 @@ public void testValidateActionsFollowingSearchableSnapshot() { } } + public void testValidateDownsamplingAction() { + { + Phase hotPhase = new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, TEST_ROLLOVER_ACTION)); + Phase warmPhase = new Phase( + "warm", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.hours(1))) + ); + Phase coldPhase = new Phase( + "cold", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.hours(1))) + ); + + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateDownsamplingIntervals(List.of(warmPhase, coldPhase, hotPhase)) + ); + assertThat( + e.getMessage(), + is("Downsampling interval [1h] for phase [cold] must be greater than the interval [1h] for phase [warm]") + ); + } + + { + Phase warmPhase = new Phase( + "warm", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.hours(1))) + ); + Phase coldPhase = new Phase( + "cold", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.minutes(30))) + ); + + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateDownsamplingIntervals(List.of(coldPhase, warmPhase)) + ); + assertThat( + e.getMessage(), + is("Downsampling interval [30m] for phase [cold] must be greater than the interval [1h] for phase [warm]") + ); + } + + { + Phase warmPhase = new Phase( + "warm", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.hours(1))) + ); + Phase coldPhase = new Phase( + "cold", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.minutes(130))) + ); + + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> TimeseriesLifecycleType.validateDownsamplingIntervals(List.of(coldPhase, warmPhase)) + ); + assertThat( + e.getMessage(), + is("Downsampling interval [130m] for phase [cold] must be a multiple of the interval [1h] for phase [warm]") + ); + } + + { + Phase hotPhase = new Phase( + "hot", + TimeValue.ZERO, + Map.of( + RolloverAction.NAME, + TEST_ROLLOVER_ACTION, + DownsampleAction.NAME, + new DownsampleAction(DateHistogramInterval.minutes(10)) + ) + ); + Phase warmPhase = new Phase( + "warm", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.minutes(30))) + ); + Phase coldPhase = new Phase( + "cold", + TimeValue.ZERO, + Map.of(DownsampleAction.NAME, new DownsampleAction(DateHistogramInterval.hours(2))) + ); + + // This is a valid interval combination + TimeseriesLifecycleType.validateDownsamplingIntervals(List.of(coldPhase, warmPhase, hotPhase)); + } + } + public void testGetOrderedPhases() { Map phaseMap = new HashMap<>(); for (String phaseName : randomSubsetOf(randomIntBetween(0, ORDERED_VALID_PHASES.size()), ORDERED_VALID_PHASES)) { diff --git a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/70_downsampling.yml b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/70_downsampling.yml new file mode 100644 index 0000000000000..49521f2e3d929 --- /dev/null +++ b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/70_downsampling.yml @@ -0,0 +1,64 @@ +setup: + - skip: + version: " - 8.4.99" + reason: "Downsample ILM validations added in 8.5.0" + + - do: + cluster.health: + wait_for_status: yellow + +--- +"Test downsample in hot phase without rollover": + - skip: + version: " - 8.4.99" + reason: "Downsample ILM validations added in 8.5.0" + + - do: + catch: /the \[downsample\] action\(s\) may not be used in the \[hot\] phase without an accompanying \[rollover\] action/ + ilm.put_lifecycle: + policy: "bad_policy" + body: | + { + "policy": { + "phases": { + "hot": { + "min_age": "0s", + "actions": { + "downsample": { + "fixed_interval": "3h" + } + } + } + } + } + } + +--- +"Test downsampling in multiple phases with the same interval": + - do: + catch: /Downsampling interval \[3h\] for phase \[cold\] must be greater than the interval \[3h\] for phase \[warm\]/ + ilm.put_lifecycle: + policy: "bad_policy" + body: | + { + "policy": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + "downsample": { + "fixed_interval": "3h" + } + } + }, + "cold": { + "min_age": "30s", + "actions": { + "downsample": { + "fixed_interval": "3h" + } + } + } + } + } + } diff --git a/x-pack/plugin/rollup/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/rollup/10_basic.yml b/x-pack/plugin/rollup/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/rollup/10_basic.yml index 8a17dccbf1441..fbbb41efb452a 100644 --- a/x-pack/plugin/rollup/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/rollup/10_basic.yml +++ b/x-pack/plugin/rollup/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/rollup/10_basic.yml @@ -383,7 +383,7 @@ setup: - is_true: acknowledged - do: - catch: /Downsampling interval \[1h\] must be greater than the the source index interval \[1h\]/ + catch: /Downsampling interval \[1h\] must be greater than the source index interval \[1h\]/ indices.downsample: index: rollup-test target_index: rollup-test-2 @@ -393,7 +393,7 @@ setup: } - do: - catch: /Downsampling interval \[30m\] must be greater than the the source index interval \[1h\]/ + catch: /Downsampling interval \[30m\] must be greater than the source index interval \[1h\]/ indices.downsample: index: rollup-test target_index: rollup-test-2 diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/TransportRollupAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/TransportRollupAction.java index e6b11a9dadfb5..f9192772ea1e5 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/TransportRollupAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/TransportRollupAction.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.cluster.stats.MappingVisitor; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; @@ -65,7 +66,6 @@ import org.elasticsearch.xpack.core.downsample.DownsampleAction; import org.elasticsearch.xpack.core.downsample.DownsampleConfig; import org.elasticsearch.xpack.core.downsample.RollupIndexerAction; -import org.elasticsearch.xpack.core.rollup.action.RollupActionRequestValidationException; import java.io.IOException; import java.util.ArrayList; @@ -231,7 +231,7 @@ protected void masterOperation( } }); - RollupActionRequestValidationException validationException = new RollupActionRequestValidationException(); + ActionRequestValidationException validationException = new ActionRequestValidationException(); if (dimensionFields.isEmpty()) { validationException.addValidationError("Index [" + sourceIndexName + "] does not contain any dimension fields"); } @@ -480,7 +480,7 @@ private static void addMetricFieldMapping(final XContentBuilder builder, final S private static void validateDownsamplingInterval(MapperService mapperService, DownsampleConfig config) { MappedFieldType timestampFieldType = mapperService.fieldType(config.getTimestampField()); assert timestampFieldType != null : "Cannot find timestamp field [" + config.getTimestampField() + "] in the mapping"; - RollupActionRequestValidationException e = new RollupActionRequestValidationException(); + ActionRequestValidationException e = new ActionRequestValidationException(); Map meta = timestampFieldType.meta(); if (meta.isEmpty() == false) { @@ -495,7 +495,7 @@ private static void validateDownsamplingInterval(MapperService mapperService, Do e.addValidationError( "Source index is a downsampled index. Downsampling interval [" + targetIndexInterval - + "] must be greater than the the source index interval [" + + "] must be greater than the source index interval [" + sourceIndexInterval + "]" ); diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java index 7a9d7d1c7be45..f7078c5fba367 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java @@ -75,7 +75,6 @@ import org.elasticsearch.xpack.core.ilm.LifecycleSettings; import org.elasticsearch.xpack.core.ilm.RolloverAction; import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; -import org.elasticsearch.xpack.core.rollup.action.RollupActionRequestValidationException; import org.elasticsearch.xpack.ilm.IndexLifecycle; import org.elasticsearch.xpack.rollup.Rollup; import org.junit.Before; @@ -478,7 +477,7 @@ public void testCannotRollupIndexWithNoMetrics() { DownsampleConfig config = new DownsampleConfig(randomInterval()); prepareSourceIndex(sourceIndex); - Exception exception = expectThrows(RollupActionRequestValidationException.class, () -> rollup(sourceIndex, rollupIndex, config)); + Exception exception = expectThrows(ActionRequestValidationException.class, () -> rollup(sourceIndex, rollupIndex, config)); assertThat(exception.getMessage(), containsString("does not contain any metric fields")); } From 3446853aa3ddb1fa4cb5cf182f0922e463a279e8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 26 Sep 2022 14:29:20 +0200 Subject: [PATCH 13/30] Speed up snapshot shards service by avoiding looping all shards needlessly (#90057) Store a flag to indicate whether or not we have INIT stage shards in a snapshot to avoid looping over those snapshots that don't needlessly in the snapshot shards service. We loop all shards when constructing the snapshot entry anyway so this is almost free when constructing the entry, but saves considerable CPU time when there's many queued up snapshots during CS application. --- .../cluster/SnapshotsInProgress.java | 51 ++++++++++++++++--- .../snapshots/SnapshotShardsService.java | 2 +- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java index eaff0a7d3eb39..324379afe11aa 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java @@ -686,6 +686,13 @@ public static class Entry implements Writeable, ToXContent, RepositoryOperation, @Nullable private final String failure; + /** + * Flag set to true in case any of the shard snapshots in {@link #shards} are in state {@link ShardState#INIT}. + * This is used by data nodes to determine if there is any work to be done on a snapshot by them without having to iterate + * the full {@link #shards} map. + */ + private final boolean hasShardsInInitState; + // visible for testing, use #startedEntry and copy constructors in production code public static Entry snapshot( Snapshot snapshot, @@ -704,13 +711,16 @@ public static Entry snapshot( ) { final Map res = Maps.newMapWithExpectedSize(indices.size()); final Map byRepoShardIdBuilder = Maps.newHashMapWithExpectedSize(shards.size()); + boolean hasInitStateShards = false; for (Map.Entry entry : shards.entrySet()) { final ShardId shardId = entry.getKey(); final IndexId indexId = indices.get(shardId.getIndexName()); final Index index = shardId.getIndex(); final Index existing = res.put(indexId.getName(), index); assert existing == null || existing.equals(index) : "Conflicting indices [" + existing + "] and [" + index + "]"; - byRepoShardIdBuilder.put(new RepositoryShardId(indexId, shardId.id()), entry.getValue()); + final var shardSnapshotStatus = entry.getValue(); + hasInitStateShards |= shardSnapshotStatus.state() == ShardState.INIT; + byRepoShardIdBuilder.put(new RepositoryShardId(indexId, shardId.id()), shardSnapshotStatus); } return new Entry( snapshot, @@ -728,7 +738,8 @@ public static Entry snapshot( version, null, byRepoShardIdBuilder, - res + res, + hasInitStateShards ); } @@ -759,7 +770,8 @@ private static Entry createClone( version, source, shardStatusByRepoShardId, - Map.of() + Map.of(), + false ); } @@ -779,7 +791,8 @@ private Entry( Version version, @Nullable SnapshotId source, Map shardStatusByRepoShardId, - Map snapshotIndices + Map snapshotIndices, + boolean hasShardsInInitState ) { this.state = state; this.snapshot = snapshot; @@ -797,7 +810,15 @@ private Entry( this.source = source; this.shardStatusByRepoShardId = Map.copyOf(shardStatusByRepoShardId); this.snapshotIndices = snapshotIndices; - assert assertShardsConsistent(this.source, this.state, this.indices, this.shards, this.shardStatusByRepoShardId); + this.hasShardsInInitState = hasShardsInInitState; + assert assertShardsConsistent( + this.source, + this.state, + this.indices, + this.shards, + this.shardStatusByRepoShardId, + this.hasShardsInInitState + ); } private static Entry readFrom(StreamInput in) throws IOException { @@ -856,7 +877,8 @@ private static boolean assertShardsConsistent( State state, Map indices, Map shards, - Map statusByRepoShardId + Map statusByRepoShardId, + boolean hasInitStateShards ) { if ((state == State.INIT || state == State.ABORTED) && shards.isEmpty()) { return true; @@ -883,12 +905,15 @@ assert hasFailures(statusByRepoShardId) == false || state == State.FAILED } if (source == null) { assert shards.size() == statusByRepoShardId.size(); + boolean foundInitStateShard = false; for (Map.Entry entry : shards.entrySet()) { + foundInitStateShard |= entry.getValue().state() == ShardState.INIT; final ShardId routingShardId = entry.getKey(); assert statusByRepoShardId.get( new RepositoryShardId(indices.get(routingShardId.getIndexName()), routingShardId.id()) ) == entry.getValue() : "found inconsistent values tracked by routing- and repository shard id"; } + assert foundInitStateShard == hasInitStateShards : "init shard state flag does not match shard states"; } return true; } @@ -912,7 +937,8 @@ public Entry withRepoGen(long newRepoGen) { version, source, shardStatusByRepoShardId, - snapshotIndices + snapshotIndices, + hasShardsInInitState ); } @@ -1130,6 +1156,14 @@ public Map userMetadata() { return userMetadata; } + /** + * See {@link #hasShardsInInitState}. + * @return true if this entry can contain shard snapshots that have yet to be started on a data node. + */ + public boolean hasShardsInInitState() { + return hasShardsInInitState; + } + public boolean partial() { return partial; } @@ -1514,7 +1548,8 @@ public Entry apply(Entry part) { part.version, null, part.shardStatusByRepoShardId, - part.snapshotIndices + part.snapshotIndices, + part.hasShardsInInitState ); } if (part.isClone()) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java index 7e98eaa2c7119..5c8a2668d1d59 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java @@ -206,7 +206,7 @@ private void startNewSnapshots(List snapshotsInProgre // This is a snapshot clone, it will be executed on the current master continue; } - if (entryState == State.STARTED) { + if (entryState == State.STARTED && entry.hasShardsInInitState()) { Map startedShards = null; final Snapshot snapshot = entry.snapshot(); Map snapshotShards = shardSnapshots.getOrDefault(snapshot, emptyMap()); From 5a9a4571b29f5a05655c844fa9a74c39f8547107 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Mon, 26 Sep 2022 15:11:18 +0200 Subject: [PATCH 14/30] re-enable bwc after upgrade lucene to 9.4.0-snapshot-f5d0646daa5 (#90344) --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 0b4d9302f5dd6..e1e11e60e110e 100644 --- a/build.gradle +++ b/build.gradle @@ -137,9 +137,9 @@ tasks.register("verifyVersions") { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = false +boolean bwc_tests_enabled = true // place a PR link here when committing bwc changes: -String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/90230" +String bwc_tests_disabled_issue = "" if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") From d0cf9f50345b23d05bf8d6cc19e019e652aa97e4 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Sep 2022 09:28:55 -0400 Subject: [PATCH 15/30] Synthetic `_source`: `ignore_malformed` for `ip` (#90038) This adds synthetic `_source` support for `ip` fields with `ignore_malfored` set to `true`. We save the field values in hidden stored field, just like we do for `ignore_above` keyword fields. Then we load them at load time. --- docs/changelog/90038.yaml | 5 + docs/reference/mapping/types/ip.asciidoc | 3 +- .../test/get/100_synthetic_source.yml | 96 +++++++++ .../test/get/110_ignore_malformed.yml | 93 +++++++++ .../mapper/IgnoreMalformedStoredValues.java | 197 ++++++++++++++++++ .../index/mapper/IpFieldMapper.java | 11 +- .../index/mapper/KeywordFieldMapper.java | 3 +- ...ortedSetDocValuesSyntheticFieldLoader.java | 46 +++- .../index/mapper/SourceLoader.java | 3 + .../IgnoreMalformedStoredValuesTests.java | 131 ++++++++++++ .../index/mapper/IpFieldMapperTests.java | 43 ++-- .../VersionStringFieldMapper.java | 2 +- 12 files changed, 602 insertions(+), 31 deletions(-) create mode 100644 docs/changelog/90038.yaml create mode 100644 rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/110_ignore_malformed.yml create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValues.java create mode 100644 server/src/test/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValuesTests.java diff --git a/docs/changelog/90038.yaml b/docs/changelog/90038.yaml new file mode 100644 index 0000000000000..826b6f9284fe0 --- /dev/null +++ b/docs/changelog/90038.yaml @@ -0,0 +1,5 @@ +pr: 90038 +summary: "Synthetic `_source`: `ignore_malformed` for `ip`" +area: TSDB +type: enhancement +issues: [] diff --git a/docs/reference/mapping/types/ip.asciidoc b/docs/reference/mapping/types/ip.asciidoc index 00a8d9a050847..abcf084b54a12 100644 --- a/docs/reference/mapping/types/ip.asciidoc +++ b/docs/reference/mapping/types/ip.asciidoc @@ -155,8 +155,7 @@ GET my-index-000001/_search ==== Synthetic source `ip` fields support <> in their default configuration. Synthetic `_source` cannot be used together with -<>, <>, or with -<> disabled. +<> or with <> disabled. Synthetic source always sorts `ip` fields and removes duplicates. For example: [source,console,id=synthetic-source-ip-example] diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml index 8358beb0a4c1b..77edf3d78874f 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml @@ -583,3 +583,99 @@ _source filtering: _source: kwd: foo - is_false: fields + +--- +ip with ignore_malformed: + - skip: + version: " - 8.5.99" + reason: introduced in 8.6.0 + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + ip: + type: ip + ignore_malformed: true + + - do: + index: + index: test + id: 1 + refresh: true + body: + ip: 192.168.0.1 + - do: + index: + index: test + id: 2 + refresh: true + body: + ip: garbage + - do: + index: + index: test + id: 3 + refresh: true + body: + ip: + - 10.10.1.1 + - 192.8.1.2 + - hot garbage + - 7 + - do: + catch: "/failed to parse field \\[ip\\] of type \\[ip\\] in document with id '4'. Preview of field's value: '\\{object=wow\\}'/" + index: + index: test + id: 4 + refresh: true + body: + ip: + object: wow + + - do: + get: + index: test + id: 1 + - match: {_index: "test"} + - match: {_id: "1"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: 192.168.0.1 + - is_false: fields + + - do: + get: + index: test + id: 2 + - match: {_index: "test"} + - match: {_id: "2"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: garbage + - is_false: fields + + - do: + get: + index: test + id: 3 + - match: {_index: "test"} + - match: {_id: "3"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: + - 10.10.1.1 + - 192.8.1.2 + - hot garbage # fields saved by ignore_malformed are sorted after doc values + - 7 + - is_false: fields diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/110_ignore_malformed.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/110_ignore_malformed.yml new file mode 100644 index 0000000000000..ffb1e3861a618 --- /dev/null +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/110_ignore_malformed.yml @@ -0,0 +1,93 @@ +--- +ip: + - skip: + version: " - 8.4.99" + reason: introduced in 8.5.0 + + - do: + indices.create: + index: test + body: + mappings: + properties: + ip: + type: ip + ignore_malformed: true + + - do: + index: + index: test + id: 1 + refresh: true + body: + ip: 192.168.0.1 + - do: + index: + index: test + id: 2 + refresh: true + body: + ip: garbage + - do: + index: + index: test + id: 3 + refresh: true + body: + ip: + - 10.10.1.1 + - 192.8.1.2 + - hot garbage + - 7 + - do: + catch: "/failed to parse field \\[ip\\] of type \\[ip\\] in document with id '4'. Preview of field's value: '\\{object=wow\\}'/" + index: + index: test + id: 4 + refresh: true + body: + ip: + object: wow + + - do: + get: + index: test + id: 1 + - match: {_index: "test"} + - match: {_id: "1"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: 192.168.0.1 + - is_false: fields + + - do: + get: + index: test + id: 2 + - match: {_index: "test"} + - match: {_id: "2"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: garbage + - is_false: fields + + - do: + get: + index: test + id: 3 + - match: {_index: "test"} + - match: {_id: "3"} + - match: {_version: 1} + - match: {found: true} + - match: + _source: + ip: + - 10.10.1.1 + - 192.8.1.2 + - hot garbage # fields saved by ignore_malformed are sorted after doc values + - 7 + - is_false: fields diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValues.java b/server/src/main/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValues.java new file mode 100644 index 0000000000000..135fb7115a6f4 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValues.java @@ -0,0 +1,197 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.document.StoredField; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.util.ByteUtils; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParser; + +import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static java.util.Collections.emptyList; + +/** + * Saves malformed values to stored fields so they can be loaded for synthetic + * {@code _source}. + */ +public abstract class IgnoreMalformedStoredValues { + /** + * Build a {@link StoredField} for the value on which the parser is + * currently positioned. + *

+ * We try to use {@link StoredField}'s native types for fields where + * possible but we have to preserve more type information than + * stored fields support, so we encode all of those into stored fields' + * {@code byte[]} type and then encode type information in the first byte. + *

+ */ + public static StoredField storedField(String fieldName, XContentParser parser) throws IOException { + String name = name(fieldName); + return switch (parser.currentToken()) { + case VALUE_STRING -> new StoredField(name, parser.text()); + case VALUE_NUMBER -> switch (parser.numberType()) { + case INT -> new StoredField(name, parser.intValue()); + case LONG -> new StoredField(name, parser.longValue()); + case DOUBLE -> new StoredField(name, parser.doubleValue()); + case FLOAT -> new StoredField(name, parser.floatValue()); + case BIG_INTEGER -> new StoredField(name, encode((BigInteger) parser.numberValue())); + case BIG_DECIMAL -> new StoredField(name, encode((BigDecimal) parser.numberValue())); + }; + case VALUE_BOOLEAN -> new StoredField(name, new byte[] { parser.booleanValue() ? (byte) 't' : (byte) 'f' }); + case VALUE_EMBEDDED_OBJECT -> new StoredField(name, encode(parser.binaryValue())); + default -> throw new IllegalArgumentException("synthetic _source doesn't support malformed objects"); + }; + } + + /** + * Build a {@link IgnoreMalformedStoredValues} that never contains any values. + */ + public static IgnoreMalformedStoredValues empty() { + return EMPTY; + } + + /** + * Build a {@link IgnoreMalformedStoredValues} that loads from stored fields. + */ + public static IgnoreMalformedStoredValues stored(String fieldName) { + return new Stored(fieldName); + } + + /** + * A {@link Stream} mapping stored field paths to a place to put them + * so they can be included in the next document. + */ + public abstract Stream> storedFieldLoaders(); + + /** + * How many values has this field loaded for this document? + */ + public abstract int count(); + + /** + * Write values for this document. + */ + public abstract void write(XContentBuilder b) throws IOException; + + private static final Empty EMPTY = new Empty(); + + private static class Empty extends IgnoreMalformedStoredValues { + @Override + public Stream> storedFieldLoaders() { + return Stream.empty(); + } + + @Override + public int count() { + return 0; + } + + @Override + public void write(XContentBuilder b) throws IOException {} + } + + private static class Stored extends IgnoreMalformedStoredValues { + private final String fieldName; + + private List values = emptyList(); + + Stored(String fieldName) { + this.fieldName = fieldName; + } + + @Override + public Stream> storedFieldLoaders() { + return Stream.of(Map.entry(name(fieldName), values -> this.values = values)); + } + + @Override + public int count() { + return values.size(); + } + + @Override + public void write(XContentBuilder b) throws IOException { + for (Object v : values) { + if (v instanceof BytesRef r) { + decodeAndWrite(b, r); + } else { + b.value(v); + } + } + values = emptyList(); + } + + private void decodeAndWrite(XContentBuilder b, BytesRef r) throws IOException { + switch (r.bytes[r.offset]) { + case 'b': + b.value(r.bytes, r.offset + 1, r.length - 1); + return; + case 'd': + if (r.length < 5) { + throw new IllegalArgumentException("Can't decode " + r); + } + int scale = ByteUtils.readIntLE(r.bytes, r.offset + 1); + b.value(new BigDecimal(new BigInteger(r.bytes, r.offset + 5, r.length - 5), scale)); + return; + case 'f': + if (r.length != 1) { + throw new IllegalArgumentException("Can't decode " + r); + } + b.value(false); + return; + case 'i': + b.value(new BigInteger(r.bytes, r.offset + 1, r.length - 1)); + return; + case 't': + if (r.length != 1) { + throw new IllegalArgumentException("Can't decode " + r); + } + b.value(true); + return; + default: + throw new IllegalArgumentException("Can't decode " + r); + } + } + } + + private static String name(String fieldName) { + return fieldName + "._ignore_malformed"; + } + + private static byte[] encode(BigInteger n) { + byte[] twosCompliment = n.toByteArray(); + byte[] encoded = new byte[1 + twosCompliment.length]; + encoded[0] = 'i'; + System.arraycopy(twosCompliment, 0, encoded, 1, twosCompliment.length); + return encoded; + } + + private static byte[] encode(BigDecimal n) { + byte[] twosCompliment = n.unscaledValue().toByteArray(); + byte[] encoded = new byte[5 + twosCompliment.length]; + encoded[0] = 'd'; + ByteUtils.writeIntLE(n.scale(), encoded, 1); + System.arraycopy(twosCompliment, 0, encoded, 5, twosCompliment.length); + return encoded; + } + + private static byte[] encode(byte[] b) { + byte[] encoded = new byte[1 + b.length]; + encoded[0] = 'b'; + System.arraycopy(b, 0, encoded, 1, b.length); + return encoded; + } +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 1b0821799597a..5c7270151e9eb 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -478,6 +478,10 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio } catch (IllegalArgumentException e) { if (ignoreMalformed) { context.addIgnoredField(fieldType().name()); + if (context.isSyntheticSource()) { + // Save a copy of the field so synthetic source can load it + context.doc().add(IgnoreMalformedStoredValues.storedField(name(), context.parser())); + } return; } else { throw e; @@ -548,17 +552,12 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() { "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it doesn't have doc values" ); } - if (ignoreMalformed) { - throw new IllegalArgumentException( - "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it ignores malformed ips" - ); - } if (copyTo.copyToFields().isEmpty() != true) { throw new IllegalArgumentException( "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to" ); } - return new SortedSetDocValuesSyntheticFieldLoader(name(), simpleName(), null) { + return new SortedSetDocValuesSyntheticFieldLoader(name(), simpleName(), null, ignoreMalformed) { @Override protected BytesRef convert(BytesRef value) { byte[] bytes = Arrays.copyOfRange(value.bytes, value.offset, value.offset + value.length); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 42b2654cad41b..5dabedd08cc85 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -1105,7 +1105,8 @@ protected void write(XContentBuilder b, Object value) throws IOException { return new SortedSetDocValuesSyntheticFieldLoader( name(), simpleName, - fieldType().ignoreAbove == Defaults.IGNORE_ABOVE ? null : originalName() + fieldType().ignoreAbove == Defaults.IGNORE_ABOVE ? null : originalName(), + false ) { @Override protected BytesRef convert(BytesRef value) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SortedSetDocValuesSyntheticFieldLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SortedSetDocValuesSyntheticFieldLoader.java index 70077ac422503..4e380a6a74a8b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SortedSetDocValuesSyntheticFieldLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SortedSetDocValuesSyntheticFieldLoader.java @@ -43,21 +43,41 @@ public abstract class SortedSetDocValuesSyntheticFieldLoader implements SourceLo private final String storedValuesName; private List storedValues = emptyList(); + /** + * Optionally loads malformed values from stored fields. + */ + private final IgnoreMalformedStoredValues ignoreMalformedValues; + /** * Build a loader from doc values and, optionally, a stored field. * @param name the name of the field to load from doc values * @param simpleName the name to give the field in the rendered {@code _source} * @param storedValuesName the name of a stored field to load or null if there aren't any stored field for this field + * @param loadIgnoreMalformedValues should we load values skipped by {@code ignore_malfored} */ - public SortedSetDocValuesSyntheticFieldLoader(String name, String simpleName, @Nullable String storedValuesName) { + public SortedSetDocValuesSyntheticFieldLoader( + String name, + String simpleName, + @Nullable String storedValuesName, + boolean loadIgnoreMalformedValues + ) { this.name = name; this.simpleName = simpleName; this.storedValuesName = storedValuesName; + this.ignoreMalformedValues = loadIgnoreMalformedValues + ? IgnoreMalformedStoredValues.stored(name) + : IgnoreMalformedStoredValues.empty(); } @Override public Stream> storedFieldLoaders() { - return storedValuesName == null ? Stream.of() : Stream.of(Map.entry(storedValuesName, values -> this.storedValues = values)); + if (storedValuesName == null) { + return ignoreMalformedValues.storedFieldLoaders(); + } + return Stream.concat( + Stream.of(Map.entry(storedValuesName, values -> this.storedValues = values)), + ignoreMalformedValues.storedFieldLoaders() + ); } @Override @@ -87,12 +107,12 @@ public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) th @Override public boolean hasValue() { - return docValues.count() > 0 || storedValues.isEmpty() == false; + return docValues.count() > 0 || storedValues.isEmpty() == false || ignoreMalformedValues.count() > 0; } @Override public void write(XContentBuilder b) throws IOException { - int total = docValues.count() + storedValues.size(); + int total = docValues.count() + storedValues.size() + ignoreMalformedValues.count(); switch (total) { case 0: return; @@ -101,23 +121,31 @@ public void write(XContentBuilder b) throws IOException { if (docValues.count() > 0) { assert docValues.count() == 1; assert storedValues.isEmpty(); + assert ignoreMalformedValues.count() == 0; docValues.write(b); - } else { + } else if (storedValues.isEmpty() == false) { assert docValues.count() == 0; assert storedValues.size() == 1; - BytesRef ref = (BytesRef) storedValues.get(0); - b.utf8Value(ref.bytes, ref.offset, ref.length); + assert ignoreMalformedValues.count() == 0; + BytesRef converted = convert((BytesRef) storedValues.get(0)); + b.utf8Value(converted.bytes, converted.offset, converted.length); storedValues = emptyList(); + } else { + assert docValues.count() == 0; + assert storedValues.isEmpty(); + assert ignoreMalformedValues.count() == 1; + ignoreMalformedValues.write(b); } return; default: b.startArray(simpleName); docValues.write(b); for (Object v : storedValues) { - BytesRef ref = (BytesRef) v; - b.utf8Value(ref.bytes, ref.offset, ref.length); + BytesRef converted = convert((BytesRef) v); + b.utf8Value(converted.bytes, converted.offset, converted.length); } storedValues = emptyList(); + ignoreMalformedValues.write(b); b.endArray(); return; } 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 bc724e884f651..394bf736a158b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java @@ -207,6 +207,9 @@ public void write(XContentBuilder b) {} */ DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException; + /** + * Has this field loaded any values for this document? + */ boolean hasValue(); /** diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValuesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValuesTests.java new file mode 100644 index 0000000000000..8d7d1084949b1 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValuesTests.java @@ -0,0 +1,131 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.document.StoredField; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.cbor.CborXContent; + +import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.List; +import java.util.stream.Stream; + +import static org.hamcrest.Matchers.equalTo; + +public class IgnoreMalformedStoredValuesTests extends ESTestCase { + public void testIgnoreMalformedBoolean() throws IOException { + boolean b = randomBoolean(); + XContentParser p = ignoreMalformed(b); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_BOOLEAN)); + assertThat(p.booleanValue(), equalTo(b)); + } + + public void testIgnoreMalformedString() throws IOException { + String s = randomAlphaOfLength(5); + XContentParser p = ignoreMalformed(s); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_STRING)); + assertThat(p.text(), equalTo(s)); + } + + public void testIgnoreMalformedInt() throws IOException { + int i = randomInt(); + XContentParser p = ignoreMalformed(i); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.INT)); + assertThat(p.intValue(), equalTo(i)); + } + + public void testIgnoreMalformedLong() throws IOException { + long l = randomLong(); + XContentParser p = ignoreMalformed(l); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.LONG)); + assertThat(p.longValue(), equalTo(l)); + } + + public void testIgnoreMalformedFloat() throws IOException { + float f = randomFloat(); + XContentParser p = ignoreMalformed(f); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.FLOAT)); + assertThat(p.floatValue(), equalTo(f)); + } + + public void testIgnoreMalformedDouble() throws IOException { + double d = randomDouble(); + XContentParser p = ignoreMalformed(d); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.DOUBLE)); + assertThat(p.doubleValue(), equalTo(d)); + } + + public void testIgnoreMalformedBigInteger() throws IOException { + BigInteger i = randomBigInteger(); + XContentParser p = ignoreMalformed(i); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.BIG_INTEGER)); + assertThat(p.numberValue(), equalTo(i)); + } + + public void testIgnoreMalformedBigDecimal() throws IOException { + BigDecimal d = new BigDecimal(randomBigInteger(), randomInt()); + XContentParser p = ignoreMalformed(d); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_NUMBER)); + assertThat(p.numberType(), equalTo(XContentParser.NumberType.BIG_DECIMAL)); + assertThat(p.numberValue(), equalTo(d)); + } + + public void testIgnoreMalformedBytes() throws IOException { + byte[] b = randomByteArrayOfLength(10); + XContentParser p = ignoreMalformed(b); + assertThat(p.currentToken(), equalTo(XContentParser.Token.VALUE_EMBEDDED_OBJECT)); + assertThat(p.binaryValue(), equalTo(b)); + } + + private static XContentParser ignoreMalformed(Object value) throws IOException { + String fieldName = randomAlphaOfLength(10); + StoredField s = ignoreMalformedStoredField(value); + Object stored = Stream.of(s.numericValue(), s.binaryValue(), s.stringValue()).filter(v -> v != null).findFirst().get(); + IgnoreMalformedStoredValues values = IgnoreMalformedStoredValues.stored(fieldName); + values.storedFieldLoaders().forEach(e -> e.getValue().load(List.of(stored))); + return parserFrom(values, fieldName); + } + + private static StoredField ignoreMalformedStoredField(Object value) throws IOException { + XContentBuilder b = CborXContent.contentBuilder(); + b.startObject().field("name", value).endObject(); + XContentParser p = CborXContent.cborXContent.createParser(XContentParserConfiguration.EMPTY, BytesReference.bytes(b).streamInput()); + assertThat(p.nextToken(), equalTo(XContentParser.Token.START_OBJECT)); + assertThat(p.nextToken(), equalTo(XContentParser.Token.FIELD_NAME)); + assertThat(p.currentName(), equalTo("name")); + assertThat(p.nextToken().isValue(), equalTo(true)); + + return IgnoreMalformedStoredValues.storedField("foo.name", p); + } + + private static XContentParser parserFrom(IgnoreMalformedStoredValues values, String fieldName) throws IOException { + XContentBuilder b = CborXContent.contentBuilder(); + b.startObject(); + b.field(fieldName); + values.write(b); + b.endObject(); + XContentParser p = CborXContent.cborXContent.createParser(XContentParserConfiguration.EMPTY, BytesReference.bytes(b).streamInput()); + assertThat(p.nextToken(), equalTo(XContentParser.Token.START_OBJECT)); + assertThat(p.nextToken(), equalTo(XContentParser.Token.FIELD_NAME)); + assertThat(p.currentName(), equalTo(fieldName)); + assertThat(p.nextToken().isValue(), equalTo(true)); + return p; + } +} 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 f78ef96ed17cb..c83ef9fbfb436 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -21,11 +21,14 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.index.termvectors.TermVectorsService; import org.elasticsearch.script.IpFieldScript; +import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; import java.net.InetAddress; +import java.util.ArrayList; import java.util.List; +import java.util.function.Supplier; import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsString; @@ -314,28 +317,45 @@ public void testScriptAndPrecludedParameters() { protected SyntheticSourceSupport syntheticSourceSupport() { return new SyntheticSourceSupport() { private final InetAddress nullValue = usually() ? null : randomIp(randomBoolean()); + private final boolean ignoreMalformed = rarely(); @Override public SyntheticSourceExample example(int maxValues) { if (randomBoolean()) { - Tuple v = generateValue(); - return new SyntheticSourceExample(v.v1(), NetworkAddress.format(v.v2()), this::mapping); + Tuple v = generateValue(); + if (v.v2()instanceof InetAddress a) { + return new SyntheticSourceExample(v.v1(), NetworkAddress.format(a), this::mapping); + } + return new SyntheticSourceExample(v.v1(), v.v2(), this::mapping); } - List> values = randomList(1, maxValues, this::generateValue); - List in = values.stream().map(Tuple::v1).toList(); - List outList = values.stream() - .map(v -> new BytesRef(InetAddressPoint.encode(v.v2()))) + List> values = randomList(1, maxValues, this::generateValue); + List in = values.stream().map(Tuple::v1).toList(); + List outList = values.stream() + .filter(v -> v.v2() instanceof InetAddress) + .map(v -> new BytesRef(InetAddressPoint.encode((InetAddress) v.v2()))) .collect(Collectors.toSet()) .stream() .sorted() .map(v -> InetAddressPoint.decode(v.bytes)) .map(NetworkAddress::format) - .toList(); + .collect(Collectors.toCollection(ArrayList::new)); + values.stream().filter(v -> false == v.v2() instanceof InetAddress).map(v -> v.v2()).forEach(outList::add); Object out = outList.size() == 1 ? outList.get(0) : outList; return new SyntheticSourceExample(in, out, this::mapping); } - private Tuple generateValue() { + private Tuple generateValue() { + if (ignoreMalformed && randomBoolean()) { + List> choices = List.of( + () -> randomAlphaOfLength(3), + ESTestCase::randomInt, + ESTestCase::randomLong, + ESTestCase::randomFloat, + ESTestCase::randomDouble + ); + Object v = randomFrom(choices).get(); + return Tuple.tuple(v, v); + } if (nullValue != null && randomBoolean()) { return Tuple.tuple(null, nullValue); } @@ -354,6 +374,9 @@ private void mapping(XContentBuilder b) throws IOException { if (rarely()) { b.field("store", false); } + if (ignoreMalformed) { + b.field("ignore_malformed", true); + } } @Override @@ -362,10 +385,6 @@ public List invalidExample() throws IOException { new SyntheticSourceInvalidExample( equalTo("field [field] of type [ip] doesn't support synthetic source because it doesn't have doc values"), b -> b.field("type", "ip").field("doc_values", false) - ), - new SyntheticSourceInvalidExample( - equalTo("field [field] of type [ip] doesn't support synthetic source because it ignores malformed ips"), - b -> b.field("type", "ip").field("ignore_malformed", true) ) ); } diff --git a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapper.java b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapper.java index 30827aaa25555..41f1dd4f24cf0 100644 --- a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapper.java +++ b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionStringFieldMapper.java @@ -406,7 +406,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() { "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to" ); } - return new SortedSetDocValuesSyntheticFieldLoader(name(), simpleName(), null) { + return new SortedSetDocValuesSyntheticFieldLoader(name(), simpleName(), null, false) { @Override protected BytesRef convert(BytesRef value) { return VersionEncoder.decodeVersion(value); From 8fdf9cfc4c9a13e597008e40a84888f9738b77bf Mon Sep 17 00:00:00 2001 From: weizijun Date: Mon, 26 Sep 2022 22:12:29 +0800 Subject: [PATCH 16/30] Fix `aggregate_metric_double` multi-values exception (#90290) Add the `allowMultipleValues(false)` call for all metrics. Previously, it was only added to `value_count` metric. This would cause the multi values exception when there was no value_count metric. --- docs/changelog/90290.yaml | 5 ++ .../AggregateDoubleMetricFieldMapper.java | 2 +- ...AggregateDoubleMetricFieldMapperTests.java | 51 +++++++++++-------- 3 files changed, 37 insertions(+), 21 deletions(-) create mode 100644 docs/changelog/90290.yaml diff --git a/docs/changelog/90290.yaml b/docs/changelog/90290.yaml new file mode 100644 index 0000000000000..b62da2376adac --- /dev/null +++ b/docs/changelog/90290.yaml @@ -0,0 +1,5 @@ +pr: 90290 +summary: "Fixed: aggregate_metric_double multi values exception" +area: Aggregations +type: bug +issues: [] 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 8953d03351ccb..f66445ecebb6f 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 @@ -231,7 +231,7 @@ public AggregateDoubleMetricFieldMapper build(MapperBuilderContext context) { false, true, indexCreatedVersion - ); + ).allowMultipleValues(false); } NumberFieldMapper fieldMapper = builder.build(context); metricMappers.put(m, fieldMapper); diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java index 7d58d8c818ebc..4d40ae89216af 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java @@ -353,30 +353,41 @@ public void testInvalidDoubleValueCount() throws Exception { ); } + private void randomMapping(XContentBuilder b, int randomNumber) throws IOException { + b.field("type", CONTENT_TYPE); + switch (randomNumber) { + case 0 -> b.field(METRICS_FIELD, new String[] { "min" }).field(DEFAULT_METRIC, "min"); + case 1 -> b.field(METRICS_FIELD, new String[] { "max" }).field(DEFAULT_METRIC, "max"); + case 2 -> b.field(METRICS_FIELD, new String[] { "value_count" }).field(DEFAULT_METRIC, "value_count"); + case 3 -> b.field(METRICS_FIELD, new String[] { "sum" }).field(DEFAULT_METRIC, "sum"); + } + } + /** * Test inserting a document containing an array of metrics. An exception must be thrown. */ public void testParseArrayValue() throws Exception { - DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); - Exception e = expectThrows( - MapperParsingException.class, - () -> mapper.parse( - source( - b -> b.startArray("field") - .startObject() - .field("min", 10.0) - .field("max", 50.0) - .field("value_count", 3) - .endObject() - .startObject() - .field("min", 11.0) - .field("max", 51.0) - .field("value_count", 3) - .endObject() - .endArray() - ) - ) - ); + int randomNumber = randomIntBetween(0, 3); + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> randomMapping(b, randomNumber))); + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> { + b.startArray("field").startObject(); + switch (randomNumber) { + case 0 -> b.field("min", 10.0); + case 1 -> b.field("max", 50); + case 2 -> b.field("value_count", 3); + case 3 -> b.field("sum", 100.0); + } + b.endObject(); + b.startObject(); + switch (randomNumber) { + case 0 -> b.field("min", 20.0); + case 1 -> b.field("max", 60); + case 2 -> b.field("value_count", 2); + case 3 -> b.field("sum", 200.0); + } + + b.endObject().endArray(); + }))); assertThat( e.getCause().getMessage(), containsString( From 13d946c81b6d3a26666b9944616ee54f84f2beff Mon Sep 17 00:00:00 2001 From: Salvatore Campagna <93581129+salvatore-campagna@users.noreply.github.com> Date: Mon, 26 Sep 2022 16:53:25 +0200 Subject: [PATCH 17/30] test: test date histogram aggregations using different date field types (#90342) As a result of closing issue #75509 here we test data histogram aggregations including auto date histograms and composite aggregations, running the aggregation on two different indices having the same field but with different date type. --- .../test/search.aggregation/230_composite.yml | 94 +++++++++++++++++++ .../330_auto_date_histogram.yml | 79 ++++++++++++++++ .../search.aggregation/360_date_histogram.yml | 79 ++++++++++++++++ 3 files changed, 252 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml index a47cd97773e0c..26a2910b8c5c6 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -1516,3 +1516,97 @@ setup: - match: { aggregations.date_histogram_no__tz.buckets.1.doc_count: 2 } - match: { aggregations.date_histogram_no__tz.buckets.2.doc_count: 4 } - match: { aggregations.date_histogram_no__tz.buckets.3.doc_count: 3 } + +--- +"Composite with date histogram on two indices with same field but different date type": + - do: + indices.create: + index: date_field_type_date + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date + + - do: + indices.create: + index: date_field_type_date_nanos + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date_nanos + + - do: + index: + index: date_field_type_date + refresh: true + id: "1" + body: + date_field: "2017-10-20T03:08:45" + + - do: + index: + index: date_field_type_date_nanos + refresh: true + id: "2" + body: + date_field: "2017-10-20T04:10:15" + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + composite_date_histogram: + composite: + sources: [ + { + "date": { + "date_histogram": { + "field": "date_field", + "calendar_interval": "hour" + } + } + } + ] + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.composite_date_histogram.buckets: 2 } + - match: { aggregations.composite_date_histogram.after_key.date: 1508472000000 } + - match: { aggregations.composite_date_histogram.buckets.0.key.date: 1508468400000 } + - match: { aggregations.composite_date_histogram.buckets.0.doc_count: 1 } + - match: { aggregations.composite_date_histogram.buckets.1.key.date: 1508472000000 } + - match: { aggregations.composite_date_histogram.buckets.1.doc_count: 1 } + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + composite_date_histogram: + composite: + sources: [ + { + "date": { + "date_histogram": { + "field": "date_field", + "calendar_interval": "day" + } + } + } + ] + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.composite_date_histogram.buckets: 1 } + - match: { aggregations.composite_date_histogram.after_key.date: 1508457600000 } + - match: { aggregations.composite_date_histogram.buckets.0.key.date: 1508457600000 } + - match: { aggregations.composite_date_histogram.buckets.0.doc_count: 2 } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/330_auto_date_histogram.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/330_auto_date_histogram.yml index 3ef42022714ca..93c934804ebd4 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/330_auto_date_histogram.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/330_auto_date_histogram.yml @@ -98,3 +98,82 @@ setup: - length: { aggregations.histo.buckets: 2 } - match: { profile.shards.0.aggregations.0.type: AutoDateHistogramAggregator.FromSingle } - match: { profile.shards.0.aggregations.0.debug.surviving_buckets: 4 } + +--- +"Auto date histogram on two indices with same field but different date type": + - do: + indices.create: + index: date_field_type_date + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date + + - do: + indices.create: + index: date_field_type_date_nanos + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date_nanos + + - do: + index: + index: date_field_type_date + refresh: true + id: "1" + body: + date_field: "2017-10-20T03:08:45" + + - do: + index: + index: date_field_type_date_nanos + refresh: true + id: "2" + body: + date_field: "2017-10-20T04:10:15" + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + auto_date_histogram: + auto_date_histogram: + field: date_field + buckets: 2 + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.auto_date_histogram.buckets: 2 } + - match: { aggregations.auto_date_histogram.buckets.0.key_as_string: "2017-10-20T03:00:00.000Z" } + - match: { aggregations.auto_date_histogram.buckets.0.key: 1508468400000 } + - match: { aggregations.auto_date_histogram.buckets.0.doc_count: 1 } + - match: { aggregations.auto_date_histogram.buckets.1.key_as_string: "2017-10-20T04:00:00.000Z" } + - match: { aggregations.auto_date_histogram.buckets.1.key: 1508472000000 } + - match: { aggregations.auto_date_histogram.buckets.1.doc_count: 1 } + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + date_histogram: + auto_date_histogram: + field: date_field + buckets: 1 + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.date_histogram.buckets: 1 } + - match: { aggregations.date_histogram.buckets.0.key_as_string: "2017-10-20T03:00:00.000Z" } + - match: { aggregations.date_histogram.buckets.0.key: 1508468400000 } + - match: { aggregations.date_histogram.buckets.0.doc_count: 2 } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/360_date_histogram.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/360_date_histogram.yml index 17619964c9269..1e4ae5abc14d4 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/360_date_histogram.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/360_date_histogram.yml @@ -492,3 +492,82 @@ setup: field: range calendar_interval: year - match: { hits.total.value: 0 } + +--- +"Date histogram on two indices with same field but different date type": + - do: + indices.create: + index: date_field_type_date + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date + + - do: + indices.create: + index: date_field_type_date_nanos + body: + settings: + number_of_replicas: 0 + mappings: + properties: + date_field: + type: date_nanos + + - do: + index: + index: date_field_type_date + refresh: true + id: "1" + body: + date_field: "2017-10-20T03:08:45" + + - do: + index: + index: date_field_type_date_nanos + refresh: true + id: "2" + body: + date_field: "2017-10-20T04:10:15" + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + date_histogram: + date_histogram: + field: date_field + calendar_interval: hour + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.date_histogram.buckets: 2 } + - match: { aggregations.date_histogram.buckets.0.key_as_string: "2017-10-20T03:00:00.000Z" } + - match: { aggregations.date_histogram.buckets.0.key: 1508468400000 } + - match: { aggregations.date_histogram.buckets.0.doc_count: 1 } + - match: { aggregations.date_histogram.buckets.1.key_as_string: "2017-10-20T04:00:00.000Z" } + - match: { aggregations.date_histogram.buckets.1.key: 1508472000000 } + - match: { aggregations.date_histogram.buckets.1.doc_count: 1 } + + - do: + search: + index: date_field_type_date,date_field_type_date_nanos + body: + aggs: + date_histogram: + date_histogram: + field: date_field + calendar_interval: day + + - match: { hits.total.value: 2 } + - match: { hits.total.relation: "eq" } + - length: { hits.hits: 2 } + - length: { aggregations.date_histogram.buckets: 1 } + - match: { aggregations.date_histogram.buckets.0.key_as_string: "2017-10-20T00:00:00.000Z" } + - match: { aggregations.date_histogram.buckets.0.key: 1508457600000 } + - match: { aggregations.date_histogram.buckets.0.doc_count: 2 } From 05c742d166e93e34e37765c0a1fa751066c4df02 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Mon, 26 Sep 2022 17:10:25 +0200 Subject: [PATCH 18/30] [Transform] fix NPE in transform scheduling (#90347) fix a NPE if a transform hasn't been triggered yet and another potential NPE fixes #88203 fixes #90301 fixes #90255 fixes #90356 --- docs/changelog/90347.yaml | 9 +++++++ .../scheduling/TransformScheduledTask.java | 17 ++++++++----- .../TransformScheduledTaskTests.java | 24 ++++++++++++++++--- 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 docs/changelog/90347.yaml diff --git a/docs/changelog/90347.yaml b/docs/changelog/90347.yaml new file mode 100644 index 0000000000000..07ef748841b1b --- /dev/null +++ b/docs/changelog/90347.yaml @@ -0,0 +1,9 @@ +pr: 90347 +summary: Fix NPE in transform scheduling +area: Transform +type: bug +issues: + - 90356 + - 88203 + - 90301 + - 90255 diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java index 2c32092155f3c..88405b3e76b79 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTask.java @@ -64,9 +64,7 @@ final class TransformScheduledTask { frequency, lastTriggeredTimeMillis, failureCount, - failureCount == 0 - ? lastTriggeredTimeMillis + frequency.millis() - : calculateNextScheduledTimeAfterFailure(lastTriggeredTimeMillis, failureCount), + calculateNextScheduledTime(lastTriggeredTimeMillis, frequency, failureCount), listener ); } @@ -74,17 +72,24 @@ final class TransformScheduledTask { // Visible for testing /** - * Calculates the appropriate next scheduled time after a number of failures. + * Calculates the appropriate next scheduled time taking number of failures into account. * This method implements exponential backoff approach. * * @param lastTriggeredTimeMillis the last time (in millis) the task was triggered + * @param frequency the frequency of the transform * @param failureCount the number of failures that happened since the task was triggered * @return next scheduled time for a task */ - static long calculateNextScheduledTimeAfterFailure(long lastTriggeredTimeMillis, int failureCount) { + static long calculateNextScheduledTime(Long lastTriggeredTimeMillis, TimeValue frequency, int failureCount) { + final long baseTime = lastTriggeredTimeMillis != null ? lastTriggeredTimeMillis : System.currentTimeMillis(); + + if (failureCount == 0) { + return baseTime + (frequency != null ? frequency : Transform.DEFAULT_TRANSFORM_FREQUENCY).millis(); + } + // Math.min(failureCount, 32) is applied in order to avoid overflow. long delayMillis = Math.min(Math.max((1L << Math.min(failureCount, 32)) * 1000, MIN_DELAY_MILLIS), MAX_DELAY_MILLIS); - return lastTriggeredTimeMillis + delayMillis; + return baseTime + delayMillis; } String getTransformId() { diff --git a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskTests.java b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskTests.java index 1bca54361744a..a63dcd8457efa 100644 --- a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskTests.java +++ b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/scheduling/TransformScheduledTaskTests.java @@ -9,11 +9,13 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.transform.Transform; import org.elasticsearch.xpack.transform.transforms.scheduling.TransformScheduler.Listener; import java.time.Instant; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; public class TransformScheduledTaskTests extends ESTestCase { @@ -62,10 +64,10 @@ public void testNextScheduledTimeMillis() { } } - public void testCalculateNextScheduledTimeAfterFailure() { + public void testCalculateNextScheduledTimeExponentialBackoff() { long lastTriggeredTimeMillis = Instant.now().toEpochMilli(); long[] expectedDelayMillis = { - 5000, // 5s + Transform.DEFAULT_TRANSFORM_FREQUENCY.millis(), // normal schedule 5000, // 5s 5000, // 5s 8000, // 8s @@ -85,9 +87,25 @@ public void testCalculateNextScheduledTimeAfterFailure() { for (int failureCount = 0; failureCount < 1000; ++failureCount) { assertThat( "failureCount = " + failureCount, - TransformScheduledTask.calculateNextScheduledTimeAfterFailure(lastTriggeredTimeMillis, failureCount), + TransformScheduledTask.calculateNextScheduledTime(lastTriggeredTimeMillis, null, failureCount), is(equalTo(lastTriggeredTimeMillis + expectedDelayMillis[Math.min(failureCount, expectedDelayMillis.length - 1)])) ); } } + + public void testCalculateNextScheduledTime() { + long now = Instant.now().toEpochMilli(); + assertThat( + TransformScheduledTask.calculateNextScheduledTime(null, TimeValue.timeValueSeconds(10), 0), + is(greaterThanOrEqualTo(now + 10_000)) + ); + assertThat( + TransformScheduledTask.calculateNextScheduledTime(now, null, 0), + is(equalTo(now + Transform.DEFAULT_TRANSFORM_FREQUENCY.millis())) + ); + assertThat( + TransformScheduledTask.calculateNextScheduledTime(null, null, 0), + is(greaterThanOrEqualTo(now + Transform.DEFAULT_TRANSFORM_FREQUENCY.millis())) + ); + } } From a1ea442e25e9695bcc9e8d39c9d0c28d1f467e97 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Mon, 26 Sep 2022 16:27:59 +0100 Subject: [PATCH 19/30] [ML] Reduce logging from trace to debug for PyTorch inference tests (#90361) Previously logging was set to trace for all tests in the PyTorchModelIT suite. However, this causes the entire cluster state to be logged many times during the tests, and this can slow down the tests to the extent that they are at risk of timing out. This change removes the trace logging (which is the level that logs the entire cluster state). --- .../elasticsearch/xpack/ml/integration/PyTorchModelIT.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java index 9a16b50d73235..b19d4eabc25c3 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java @@ -122,9 +122,9 @@ public void setLogging() throws IOException { Request loggingSettings = new Request("PUT", "_cluster/settings"); loggingSettings.setJsonEntity(""" {"persistent" : { - "logger.org.elasticsearch.xpack.ml.inference.assignment" : "TRACE", - "logger.org.elasticsearch.xpack.ml.inference.deployment" : "TRACE", - "logger.org.elasticsearch.xpack.ml.process.logging" : "TRACE" + "logger.org.elasticsearch.xpack.ml.inference.assignment" : "DEBUG", + "logger.org.elasticsearch.xpack.ml.inference.deployment" : "DEBUG", + "logger.org.elasticsearch.xpack.ml.process.logging" : "DEBUG" }}"""); client().performRequest(loggingSettings); } From 366eb44a18cd5e1ce390752069ab36fe00192d5f Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Sep 2022 12:02:11 -0400 Subject: [PATCH 20/30] Aggs: fix auto_date_histogram > ip_range (#90317) If an `auto_date_histogram` has an `ip_range` under it we frequently have to merge "empty" `ip_range` aggregations with "non-empty" `ip_range` aggregations. This was failing because those "empty" `ip_range` aggregations had a different number of ranges than the "non-empty" ranges. This fixes that. No "empty" `ip_range` aggregations have the same size as their "non-empty" counterparts. Closes #90121 --- docs/changelog/90317.yaml | 6 ++ .../bucket/range/BinaryRangeAggregator.java | 13 ++- .../AutoDateHistogramAggregatorTests.java | 88 +++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/90317.yaml diff --git a/docs/changelog/90317.yaml b/docs/changelog/90317.yaml new file mode 100644 index 0000000000000..687ff69b4904f --- /dev/null +++ b/docs/changelog/90317.yaml @@ -0,0 +1,6 @@ +pr: 90317 +summary: "Aggs: fix `auto_date_histogram` > `ip_range`" +area: Aggregations +type: bug +issues: + - 90121 diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/BinaryRangeAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/BinaryRangeAggregator.java index 86e1876c8bd00..be9fca9acdbb5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/BinaryRangeAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/BinaryRangeAggregator.java @@ -17,6 +17,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.CardinalityUpperBound; import org.elasticsearch.search.aggregations.InternalAggregation; +import org.elasticsearch.search.aggregations.InternalAggregations; import org.elasticsearch.search.aggregations.LeafBucketCollector; import org.elasticsearch.search.aggregations.LeafBucketCollectorBase; import org.elasticsearch.search.aggregations.bucket.BucketsAggregator; @@ -24,13 +25,12 @@ import org.elasticsearch.search.aggregations.support.ValuesSource; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.List; import java.util.Map; -import static java.util.Collections.emptyList; - /** A range aggregator for values that are stored in SORTED_SET doc values. */ public final class BinaryRangeAggregator extends BucketsAggregator { @@ -330,6 +330,13 @@ public InternalAggregation[] buildAggregations(long[] owningBucketOrds) throws I @Override public InternalAggregation buildEmptyAggregation() { - return new InternalBinaryRange(name, format, keyed, emptyList(), metadata()); + // Create empty buckets with 0 count and with empty sub-aggs so we can merge them with non-empty aggs + InternalAggregations subAggs = buildEmptySubAggregations(); + List buckets = new ArrayList<>(ranges.length); + for (Range range : ranges) { + InternalBinaryRange.Bucket bucket = new InternalBinaryRange.Bucket(format, keyed, range.key, range.from, range.to, 0, subAggs); + buckets.add(bucket); + } + return new InternalBinaryRange(name, format, keyed, buckets, metadata()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java index d10de8039cdee..7d9f18c6623b2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorTests.java @@ -10,6 +10,7 @@ import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; +import org.apache.lucene.document.InetAddressPoint; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.document.SortedSetDocValuesField; @@ -26,11 +27,13 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.CheckedBiConsumer; +import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.Maps; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.BooleanFieldMapper; import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.IpFieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.NumberFieldMapper; @@ -38,6 +41,10 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.MultiBucketConsumerService; +import org.elasticsearch.search.aggregations.bucket.range.InternalBinaryRange; +import org.elasticsearch.search.aggregations.bucket.range.InternalRange; +import org.elasticsearch.search.aggregations.bucket.range.IpRangeAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.range.RangeAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.InternalStats; @@ -65,6 +72,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import static org.elasticsearch.test.ListMatcher.matchesList; +import static org.elasticsearch.test.MapMatcher.assertMap; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; @@ -74,6 +83,7 @@ public class AutoDateHistogramAggregatorTests extends DateHistogramAggregatorTes private static final String DATE_FIELD = "date"; private static final String INSTANT_FIELD = "instant"; private static final String NUMERIC_FIELD = "numeric"; + private static final String IP_FIELD = "ip"; private static final List DATES_WITH_TIME = Arrays.asList( ZonedDateTime.of(2010, 3, 12, 1, 7, 45, 0, ZoneOffset.UTC), @@ -903,6 +913,78 @@ public void testWithPipelineReductions() throws IOException { ); } + public void testSubNumericRange() throws IOException { + assertSubNumericRange(DATES_WITH_TIME, 2); + } + + /** + * Tests very few documents with a sub {@code range} agg which causes + * us to collect in a very tight time range and then merge many of those + * ranges together, thus merging unmapped {@code range} aggs with mapped + * ones. + */ + public void testSmallSubNumericRange() throws IOException { + assertSubNumericRange(DATES_WITH_TIME.subList(0, 2), 1); + } + + private void assertSubNumericRange(List dates, long firstBucketIpCount) throws IOException { + MappedFieldType dateFieldType = new DateFieldMapper.DateFieldType(DATE_FIELD); + MappedFieldType numericFieldType = new NumberFieldMapper.NumberFieldType(NUMERIC_FIELD, NumberFieldMapper.NumberType.LONG); + + AutoDateHistogramAggregationBuilder b = new AutoDateHistogramAggregationBuilder("a").field(DATE_FIELD) + .subAggregation(new RangeAggregationBuilder("r").field(NUMERIC_FIELD).addRange(0, 2).addRange(3, 4)); + testCase(b, new MatchAllDocsQuery(), iw -> indexSampleData(dates, iw), (InternalAutoDateHistogram h) -> { + InternalAutoDateHistogram.Bucket bucket = h.getBuckets().get(0); + InternalRange range = bucket.getAggregations().get("r"); + assertMap( + range.getBuckets().stream().map(InternalRange.Bucket::getKeyAsString).toList(), + matchesList().item("0.0-2.0").item("3.0-4.0") + ); + assertMap( + range.getBuckets().stream().map(InternalRange.Bucket::getDocCount).toList(), + matchesList().item(firstBucketIpCount).item(0L) + ); + }, dateFieldType, numericFieldType); + } + + public void testSubIpRange() throws IOException { + assertSubIpRange(DATES_WITH_TIME, 2); + } + + /** + * Tests very few documents with a sub {@code ip_range} agg which causes + * us to collect in a very tight time range and then merge many of those + * ranges together, thus merging unmapped {@code ip_range} aggs with mapped + * ones. + */ + public void testSmallSubIpRange() throws IOException { + assertSubIpRange(DATES_WITH_TIME.subList(0, 2), 1); + } + + private void assertSubIpRange(List dates, long firstBucketIpCount) throws IOException { + MappedFieldType dateFieldType = new DateFieldMapper.DateFieldType(DATE_FIELD); + MappedFieldType ipFieldType = new IpFieldMapper.IpFieldType(IP_FIELD); + + AutoDateHistogramAggregationBuilder b = new AutoDateHistogramAggregationBuilder("a").field(DATE_FIELD) + .subAggregation( + new IpRangeAggregationBuilder("r").field(IP_FIELD) + .addRange("192.168.0.0", "192.168.0.2") + .addRange("192.168.0.3", "192.168.0.4") + ); + testCase(b, new MatchAllDocsQuery(), iw -> indexSampleData(dates, iw), (InternalAutoDateHistogram h) -> { + InternalAutoDateHistogram.Bucket bucket = h.getBuckets().get(0); + InternalBinaryRange range = bucket.getAggregations().get("r"); + assertMap( + range.getBuckets().stream().map(InternalBinaryRange.Bucket::getKeyAsString).toList(), + matchesList().item("192.168.0.0-192.168.0.2").item("192.168.0.3-192.168.0.4") + ); + assertMap( + range.getBuckets().stream().map(InternalBinaryRange.Bucket::getDocCount).toList(), + matchesList().item(firstBucketIpCount).item(0L) + ); + }, dateFieldType, ipFieldType); + } + @Override protected IndexSettings createIndexSettings() { final Settings nodeSettings = Settings.builder().put("search.max_buckets", 25000).build(); @@ -957,6 +1039,12 @@ private void indexSampleData(List dataset, RandomIndexWriter inde document.add(new SortedNumericDocValuesField(DATE_FIELD, instant)); document.add(new LongPoint(INSTANT_FIELD, instant)); document.add(new SortedNumericDocValuesField(NUMERIC_FIELD, i)); + document.add( + new SortedSetDocValuesField( + IP_FIELD, + new BytesRef(InetAddressPoint.encode(InetAddresses.forString("192.168.0." + (i % 256)))) + ) + ); indexWriter.addDocument(document); document.clear(); i += 1; From 50cfd2b17ad1ec11d64b4c8473effc0ce962c5ee Mon Sep 17 00:00:00 2001 From: weizijun Date: Tue, 27 Sep 2022 00:06:24 +0800 Subject: [PATCH 21/30] ILM: Get policy support wildcard name (#89238) ILM Get policy support wildcard name is a useful feature. We can use Get /_ilm/policy/policy* to get the wildcard result. --- docs/changelog/89238.yaml | 5 ++ .../rest-api-spec/test/ilm/10_basic.yml | 90 +++++++++++++++++++ .../action/TransportGetLifecycleAction.java | 51 +++++++---- 3 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 docs/changelog/89238.yaml diff --git a/docs/changelog/89238.yaml b/docs/changelog/89238.yaml new file mode 100644 index 0000000000000..7f434db2aefef --- /dev/null +++ b/docs/changelog/89238.yaml @@ -0,0 +1,5 @@ +pr: 89238 +summary: "ILM: Get policy support wildcard name" +area: ILM+SLM +type: enhancement +issues: [] diff --git a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml index 89f51590861a6..df7fab311130a 100644 --- a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml +++ b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml @@ -58,6 +58,96 @@ setup: ilm.get_lifecycle: policy: "my_timeseries_lifecycle" +--- +"Test Policy Get": + - do: + ilm.put_lifecycle: + policy: "my_timeseries_lifecycle_1" + body: | + { + "policy": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + "forcemerge": { + "max_num_segments": 10000 + } + } + } + } + } + } + + - do: + ilm.put_lifecycle: + policy: "my_timeseries_lifecycle_2" + body: | + { + "policy": { + "phases": { + "delete": { + "min_age": "30s", + "actions": { + "delete": {} + } + } + } + } + } + + - do: + ilm.put_lifecycle: + policy: "other_lifecycle" + body: | + { + "policy": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + "forcemerge": { + "max_num_segments": 10000 + } + } + }, + "delete": { + "min_age": "30s", + "actions": { + "delete": {} + } + } + } + } + } + + - do: + ilm.get_lifecycle: + policy: "my_timeseries*" + - match: { my_timeseries_lifecycle_1.version: 1 } + - match: { my_timeseries_lifecycle_2.version: 1 } + + - do: + ilm.get_lifecycle: + policy: "my_timeseries_lifecycle_1,my_timeseries_lifecycle_2" + - match: { my_timeseries_lifecycle_1.version: 1 } + - match: { my_timeseries_lifecycle_2.version: 1 } + + - do: + catch: missing + ilm.get_lifecycle: + policy: "lifecycle_not_found" + + - do: + catch: /wildcard only supports a single value, please use comma-separated values or a single wildcard value/ + ilm.get_lifecycle: + policy: "my_timeseries*,other_lifecycle" + + - do: + catch: missing + ilm.get_lifecycle: + policy: "other_lifecycle,lifecycle_not_found" + --- "Test Policy Update": - do: diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportGetLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportGetLifecycleAction.java index 6282d70695d24..73e4ee94d676f 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportGetLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportGetLifecycleAction.java @@ -17,6 +17,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.regex.Regex; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -31,7 +32,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; public class TransportGetLifecycleAction extends TransportMasterNodeAction { @@ -68,29 +71,44 @@ protected void masterOperation(Task task, Request request, ClusterState state, A ); } } else { - List requestedPolicies; - // if no policies explicitly provided, behave as if `*` was specified + List names; if (request.getPolicyNames().length == 0) { - requestedPolicies = new ArrayList<>(metadata.getPolicyMetadatas().size()); - for (LifecyclePolicyMetadata policyMetadata : metadata.getPolicyMetadatas().values()) { - requestedPolicies.add( - new LifecyclePolicyResponseItem( - policyMetadata.getPolicy(), - policyMetadata.getVersion(), - policyMetadata.getModifiedDateString(), - LifecyclePolicyUtils.calculateUsage(indexNameExpressionResolver, state, policyMetadata.getName()) - ) - ); - } + names = List.of("*"); } else { - requestedPolicies = new ArrayList<>(request.getPolicyNames().length); - for (String name : request.getPolicyNames()) { + names = Arrays.asList(request.getPolicyNames()); + } + + if (names.size() > 1 && names.stream().filter(Regex::isSimpleMatchPattern).count() > 0) { + throw new IllegalArgumentException( + "wildcard only supports a single value, please use comma-separated values or a single wildcard value" + ); + } + + Map policyResponseItemMap = new LinkedHashMap<>(); + for (String name : names) { + if (Regex.isSimpleMatchPattern(name)) { + for (Map.Entry entry : metadata.getPolicyMetadatas().entrySet()) { + LifecyclePolicyMetadata policyMetadata = entry.getValue(); + if (Regex.simpleMatch(name, entry.getKey())) { + policyResponseItemMap.put( + entry.getKey(), + new LifecyclePolicyResponseItem( + policyMetadata.getPolicy(), + policyMetadata.getVersion(), + policyMetadata.getModifiedDateString(), + LifecyclePolicyUtils.calculateUsage(indexNameExpressionResolver, state, policyMetadata.getName()) + ) + ); + } + } + } else { LifecyclePolicyMetadata policyMetadata = metadata.getPolicyMetadatas().get(name); if (policyMetadata == null) { listener.onFailure(new ResourceNotFoundException("Lifecycle policy not found: {}", name)); return; } - requestedPolicies.add( + policyResponseItemMap.put( + name, new LifecyclePolicyResponseItem( policyMetadata.getPolicy(), policyMetadata.getVersion(), @@ -100,6 +118,7 @@ protected void masterOperation(Task task, Request request, ClusterState state, A ); } } + List requestedPolicies = new ArrayList<>(policyResponseItemMap.values()); listener.onResponse(new Response(requestedPolicies)); } } From 2ef957a0ee7ced5211a71a4cff14ea772f121339 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Mon, 26 Sep 2022 17:14:35 +0100 Subject: [PATCH 22/30] Update min version for health node reporting to 8.5 (#90365) The health node and disk health reporting (via the health node) was enabled starting with 8.5. Some version checks were still on 8.4 (when this feature started being developed). This can create failures in mixed cluster environments. This updates the min version for the health node reporting functionality to 8.5 --- docs/changelog/90365.yaml | 6 ++++++ .../health/metadata/HealthMetadataService.java | 2 +- .../health/node/selection/HealthNodeTaskExecutor.java | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/90365.yaml diff --git a/docs/changelog/90365.yaml b/docs/changelog/90365.yaml new file mode 100644 index 0000000000000..0ca6f650cc985 --- /dev/null +++ b/docs/changelog/90365.yaml @@ -0,0 +1,6 @@ +pr: 90365 +summary: Update min version for health node reporting to 8.5 +area: Health +type: bug +issues: + - 90359 diff --git a/server/src/main/java/org/elasticsearch/health/metadata/HealthMetadataService.java b/server/src/main/java/org/elasticsearch/health/metadata/HealthMetadataService.java index e29eec27f7583..436b1e24eb335 100644 --- a/server/src/main/java/org/elasticsearch/health/metadata/HealthMetadataService.java +++ b/server/src/main/java/org/elasticsearch/health/metadata/HealthMetadataService.java @@ -142,7 +142,7 @@ private void updateOnSettingsUpdated(String setting, String value) { // We do not use the cluster state to check if this is the master node because the cluster state might not have been initialized if (isMaster && enabled) { ClusterState clusterState = clusterService.state(); - if (clusterState.nodesIfRecovered().getMinNodeVersion().onOrAfter(Version.V_8_4_0)) { + if (clusterState.nodesIfRecovered().getMinNodeVersion().onOrAfter(Version.V_8_5_0)) { var task = new UpdateHealthMetadata(setting, value); var config = ClusterStateTaskConfig.build(Priority.NORMAL); clusterService.submitStateUpdateTask("health-metadata-update", task, config, executor); diff --git a/server/src/main/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutor.java b/server/src/main/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutor.java index b3b3ece3b8ec2..197bd4aaa8f83 100644 --- a/server/src/main/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutor.java +++ b/server/src/main/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutor.java @@ -142,8 +142,8 @@ public PersistentTasksCustomMetadata.Assignment getAssignment( // visible for testing void startTask(ClusterChangedEvent event) { - // Wait until every node in the cluster is upgraded to 8.4.0 or later - if (event.state().nodesIfRecovered().getMinNodeVersion().onOrAfter(Version.V_8_4_0)) { + // Wait until every node in the cluster is upgraded to 8.5.0 or later + if (event.state().nodesIfRecovered().getMinNodeVersion().onOrAfter(Version.V_8_5_0)) { boolean healthNodeTaskExists = HealthNode.findTask(event.state()) != null; boolean isElectedMaster = event.localNodeMaster(); if (isElectedMaster || healthNodeTaskExists) { From eec7ba47379a97803753695ce2bf90554a555d7d Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Sep 2022 12:45:04 -0400 Subject: [PATCH 23/30] Put synthetic source back in tech preview (#90371) I got some new this morning that we're going to have to rework how we handle ignore-above in synthetic _source which makes me a bit weary of removing tech-preview in 8.5. I asked a few folks and they felt more comfortable giving it a little longer in tech preview. I expect until ignore-above is in. --- docs/reference/mapping/fields/synthetic-source.asciidoc | 6 +++--- .../mapping/types/aggregate-metric-double.asciidoc | 2 +- docs/reference/mapping/types/boolean.asciidoc | 2 +- docs/reference/mapping/types/date.asciidoc | 2 +- docs/reference/mapping/types/date_nanos.asciidoc | 2 +- docs/reference/mapping/types/dense-vector.asciidoc | 2 +- docs/reference/mapping/types/geo-point.asciidoc | 2 +- docs/reference/mapping/types/histogram.asciidoc | 2 +- docs/reference/mapping/types/ip.asciidoc | 2 +- docs/reference/mapping/types/keyword.asciidoc | 2 +- docs/reference/mapping/types/numeric.asciidoc | 2 +- docs/reference/mapping/types/text.asciidoc | 2 +- docs/reference/mapping/types/version.asciidoc | 2 +- 13 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/reference/mapping/fields/synthetic-source.asciidoc b/docs/reference/mapping/fields/synthetic-source.asciidoc index f2b481960d4a4..789ac28f05236 100644 --- a/docs/reference/mapping/fields/synthetic-source.asciidoc +++ b/docs/reference/mapping/fields/synthetic-source.asciidoc @@ -1,5 +1,5 @@ [[synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] Though very handy to have around, the source field takes up a significant amount of space on disk. Instead of storing source documents on disk exactly as you @@ -45,13 +45,13 @@ types: ** <> ** <> ** <> -** <> (with a `keyword` sub-field) +** <> ** <> Runtime fields cannot, at this stage, use synthetic `_source`. [[synthetic-source-modifications]] -===== Synthetic source modifications +===== Synthetic `_source` modifications When synthetic `_source` is enabled, retrieved documents undergo some modifications compared to the original JSON. diff --git a/docs/reference/mapping/types/aggregate-metric-double.asciidoc b/docs/reference/mapping/types/aggregate-metric-double.asciidoc index 4e07ff5c9d38d..9df58119e8a3f 100644 --- a/docs/reference/mapping/types/aggregate-metric-double.asciidoc +++ b/docs/reference/mapping/types/aggregate-metric-double.asciidoc @@ -248,7 +248,7 @@ The search returns the following hit. The value of the `default_metric` field, // TESTRESPONSE[s/\.\.\./"took": $body.took,"timed_out": false,"_shards": $body._shards,/] [[aggregate-metric-double-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `aggregate_metric-double` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <>. diff --git a/docs/reference/mapping/types/boolean.asciidoc b/docs/reference/mapping/types/boolean.asciidoc index a6c1725047539..c1347866e4067 100644 --- a/docs/reference/mapping/types/boolean.asciidoc +++ b/docs/reference/mapping/types/boolean.asciidoc @@ -216,7 +216,7 @@ The following parameters are accepted by `boolean` fields: Metadata about the field. [[boolean-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `boolean` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <> or with <> disabled. diff --git a/docs/reference/mapping/types/date.asciidoc b/docs/reference/mapping/types/date.asciidoc index 419aa221f876c..eb5aeee9e8a18 100644 --- a/docs/reference/mapping/types/date.asciidoc +++ b/docs/reference/mapping/types/date.asciidoc @@ -232,7 +232,7 @@ Which will reply with a date like: ---- [[date-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `date` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <>, <> set to true diff --git a/docs/reference/mapping/types/date_nanos.asciidoc b/docs/reference/mapping/types/date_nanos.asciidoc index 3fec5876cd1df..493a08dda74f6 100644 --- a/docs/reference/mapping/types/date_nanos.asciidoc +++ b/docs/reference/mapping/types/date_nanos.asciidoc @@ -141,7 +141,7 @@ field. This limitation also affects <>. --- [[date-nanos-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `date_nanos` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <>, <> set to true diff --git a/docs/reference/mapping/types/dense-vector.asciidoc b/docs/reference/mapping/types/dense-vector.asciidoc index 59a9375e04a6b..96ea6d203d8aa 100644 --- a/docs/reference/mapping/types/dense-vector.asciidoc +++ b/docs/reference/mapping/types/dense-vector.asciidoc @@ -180,5 +180,5 @@ neighbors for each new node. Defaults to `100`. ==== [[dense-vector-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `dense_vector` fields support <> . diff --git a/docs/reference/mapping/types/geo-point.asciidoc b/docs/reference/mapping/types/geo-point.asciidoc index 4d7a9ce675080..fad74ba733cc2 100644 --- a/docs/reference/mapping/types/geo-point.asciidoc +++ b/docs/reference/mapping/types/geo-point.asciidoc @@ -205,7 +205,7 @@ def lon = doc['location'].lon; -------------------------------------------------- [[geo-point-synthetic-source]] -==== Synthetic `_source` +==== Synthetic source preview:[] `geo_point` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <>, <>, or with diff --git a/docs/reference/mapping/types/histogram.asciidoc b/docs/reference/mapping/types/histogram.asciidoc index 92cd38ef824d1..703cce6c83d0a 100644 --- a/docs/reference/mapping/types/histogram.asciidoc +++ b/docs/reference/mapping/types/histogram.asciidoc @@ -69,7 +69,7 @@ means the field can technically be aggregated with either algorithm, in practice index data in that manner (e.g. centroids for T-Digest or intervals for HDRHistogram) to ensure best accuracy. [[histogram-synthetic-source]] -==== Synthetic source +==== Synthetic `_source` preview:[] `histogram` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <> or <>. diff --git a/docs/reference/mapping/types/ip.asciidoc b/docs/reference/mapping/types/ip.asciidoc index abcf084b54a12..1a7f90954a963 100644 --- a/docs/reference/mapping/types/ip.asciidoc +++ b/docs/reference/mapping/types/ip.asciidoc @@ -152,7 +152,7 @@ GET my-index-000001/_search -------------------------------------------------- [[ip-synthetic-source]] -==== Synthetic source +==== Synthetic `_source` preview:[] `ip` fields support <> in their default configuration. Synthetic `_source` cannot be used together with <> or with <> disabled. diff --git a/docs/reference/mapping/types/keyword.asciidoc b/docs/reference/mapping/types/keyword.asciidoc index 96cc3e5fd5749..c1bd5b49dfc99 100644 --- a/docs/reference/mapping/types/keyword.asciidoc +++ b/docs/reference/mapping/types/keyword.asciidoc @@ -170,7 +170,7 @@ Dimension fields have the following constraints: -- [[keyword-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `keyword` fields support <> in their default configuration. Synthetic `_source` cannot be used together with a <> or <>. diff --git a/docs/reference/mapping/types/numeric.asciidoc b/docs/reference/mapping/types/numeric.asciidoc index 5d08b0641c965..2e9805d2f178e 100644 --- a/docs/reference/mapping/types/numeric.asciidoc +++ b/docs/reference/mapping/types/numeric.asciidoc @@ -228,7 +228,7 @@ numeric field can't be both a time series dimension and a time series metric. This parameter is required. [[numeric-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] All numeric fields except `unsigned_long` support <> in their default configuration. Synthetic `_source` cannot be used together with <>, <>, or diff --git a/docs/reference/mapping/types/text.asciidoc b/docs/reference/mapping/types/text.asciidoc index b2eb8f550377f..730ca12c1a3ed 100644 --- a/docs/reference/mapping/types/text.asciidoc +++ b/docs/reference/mapping/types/text.asciidoc @@ -160,7 +160,7 @@ The following parameters are accepted by `text` fields: Metadata about the field. [[text-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `text` fields support <> if they have a <> sub-field that supports synthetic `_source` or if the `text` field sets `store` to `true`. Either way, it may diff --git a/docs/reference/mapping/types/version.asciidoc b/docs/reference/mapping/types/version.asciidoc index a183d79e233f4..b042ea02da7c8 100644 --- a/docs/reference/mapping/types/version.asciidoc +++ b/docs/reference/mapping/types/version.asciidoc @@ -69,7 +69,7 @@ you strongly rely on these kind of queries. [[version-synthetic-source]] -==== Synthetic `_source` +==== Synthetic `_source` preview:[] `version` fields support <> so long as they don't declare <>. From 031849b6ee6fb8547c163683b85eeb7574cc6459 Mon Sep 17 00:00:00 2001 From: Christos Soulios <1561376+csoulios@users.noreply.github.com> Date: Mon, 26 Sep 2022 21:34:05 +0300 Subject: [PATCH 24/30] Add highlight changelog for TSDB release (#90354) Add highlight part to changelog for PR #90116 --- docs/changelog/90116.yaml | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/docs/changelog/90116.yaml b/docs/changelog/90116.yaml index 4536c396c2676..2cb0630a28c61 100644 --- a/docs/changelog/90116.yaml +++ b/docs/changelog/90116.yaml @@ -2,4 +2,34 @@ pr: 90116 summary: "Release time-series (TSDB) functionality" area: "TSDB" type: feature -issues: [] +issues: + - 74660 +highlight: + title: Release time series data stream (TSDS) functionality + body: |- + Elasticsearch offers support for time series data stream (TSDS) indices. A TSDS index is + an index that contains time series metrics data as part of a data stream. Elasticsearch + routes the incoming documents into a TSDS index so + that all the documents for a particular time series are on the same shard, and + then sorts the shard by time series and timestamp. This structure + has a few advantages: + + 1. Documents from the same time series are next to each other on the shard, and + hence stored next to each other on the disk, so the operating system + pages are much more homogeneous and compress better, yielding massive reduction + in TCO. + + 2. The analysis of a time series typically involves comparing each two consecutive + docs (samples), examining the last doc in a given time window, etc., which is quite + complex when the next doc could be on any shard, and in fact on any index. Sorting + by time series and timestamp allows improved analysis, both in terms of performance + and in terms of our ability to add new aggregations. + + Finally, as part of the Index Lifecycle Management of metrics data time series, + Elasticsearch enables a Downsampling action. When an index is downsampled, + Elasticsearch keeps a single document with statistical summaries per each bucket + of time in the time series. Supported aggregations can then be run on the data + stream and include both downsampled indices and raw data indices, without the + user needing to be aware of that. Downsampling of downsampled indices, to more + coarse time resolution, is also supported. + notable: true From 7f5c2a0dc9041e3ec67d52006430139e0e7ab3ca Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 26 Sep 2022 14:36:14 -0400 Subject: [PATCH 25/30] Stable Plugins: Handle service loading with UberModuleClassLoader (#90206) If a class in the ubermodule uses SPI, the service loader will check whether its module has declared a use of the service it's requesting. We scan the boot module layer for any public services provided by the JDK, and we scan the META-INF/services directory in the jar for any services internal to the module. Since some of these provides/uses relationships may be between jars in the module, we have to declare that the module both uses and provides any of the services we find in the jars. Previously, we added reads relationships to java modules as a final step in constructing the module. It turns out to be simpler and more idiomatic to get the list of modules in the boot layer and require them when building the module descriptor. This lets us add the services and providers much closer to when we scan them in the jar; otherwise, we would get an error when trying to use a service defined in a module that we don't read. --- .../elasticsearch/plugins/ModuleSupport.java | 105 ++++++++++++- .../plugins/UberModuleClassLoader.java | 85 +++++----- .../plugins/UberModuleClassLoaderTests.java | 147 +++++++++++++++++- 3 files changed, 286 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/ModuleSupport.java b/server/src/main/java/org/elasticsearch/plugins/ModuleSupport.java index d736d177c12cd..06f5a718229ba 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ModuleSupport.java +++ b/server/src/main/java/org/elasticsearch/plugins/ModuleSupport.java @@ -10,7 +10,9 @@ import org.elasticsearch.core.SuppressForbidden; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; import java.io.UncheckedIOException; import java.lang.module.InvalidModuleDescriptorException; import java.lang.module.ModuleDescriptor; @@ -18,15 +20,20 @@ import java.lang.module.ModuleReader; import java.lang.module.ModuleReference; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.zip.ZipFile; /** @@ -38,10 +45,10 @@ private ModuleSupport() { throw new AssertionError("Utility class, should not be instantiated"); } - static ModuleFinder ofSyntheticPluginModule(String name, Path[] jarPaths, Set requires) { + static ModuleFinder ofSyntheticPluginModule(String name, Path[] jarPaths, Set requires, Set uses) { try { return new InMemoryModuleFinder( - new InMemoryModuleReference(createModuleDescriptor(name, jarPaths, requires), URI.create("module:/" + name)) + new InMemoryModuleReference(createModuleDescriptor(name, jarPaths, requires, uses), URI.create("module:/" + name)) ); } catch (IOException e) { throw new UncheckedIOException(e); @@ -49,22 +56,57 @@ static ModuleFinder ofSyntheticPluginModule(String name, Path[] jarPaths, Set requires) throws IOException { + static ModuleDescriptor createModuleDescriptor(String name, Path[] jarPaths, Set requires, Set uses) + throws IOException { var builder = ModuleDescriptor.newOpenModule(name); // open module, for now requires.stream().forEach(builder::requires); + uses.stream().forEach(builder::uses); // scan the names of the entries in the JARs Set pkgs = new HashSet<>(); + Map> allBundledProviders = new HashMap<>(); + Set allBundledServices = new HashSet<>(); for (Path path : jarPaths) { assert path.getFileName().toString().endsWith(".jar") : "expected jars suffix, in path: " + path; try (JarFile jf = new JarFile(path.toFile(), true, ZipFile.OPEN_READ, Runtime.version())) { - // separator = path.getFileSystem().getSeparator(); - var scan = scan(jf); - scan.classFiles().stream().map(cf -> toPackageName(cf, "/")).flatMap(Optional::stream).forEach(pkgs::add); + // if we have a module declaration, trust its uses/provides + JarEntry moduleInfo = jf.getJarEntry("module-info.class"); + if (moduleInfo != null) { + var descriptor = getDescriptorForModularJar(path); + pkgs.addAll(descriptor.packages()); + allBundledServices.addAll(descriptor.uses()); + for (ModuleDescriptor.Provides p : descriptor.provides()) { + String serviceName = p.service(); + List providersInModule = p.providers(); + + allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInModule)); + allBundledServices.add(serviceName); + } + } else { + var scan = scan(jf); + scan.classFiles().stream().map(cf -> toPackageName(cf, "/")).flatMap(Optional::stream).forEach(pkgs::add); + + // read providers from the list of service files + for (String serviceFileName : scan.serviceFiles()) { + String serviceName = getServiceName(serviceFileName); + List providersInJar = getProvidersFromServiceFile(jf, serviceFileName); + + allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInJar)); + allBundledServices.add(serviceName); + } + } } } + builder.packages(pkgs); - // TODO: provides and uses - it is possible that one plugin could define a service and another could provide it + + // the module needs to use all services it provides, for the case of internal use + allBundledServices.addAll(allBundledProviders.keySet()); + // but we don't want to add any services we already got from the parent layer + allBundledServices.removeAll(uses); + + allBundledServices.forEach(builder::uses); + allBundledProviders.forEach(builder::provides); return builder.build(); } @@ -178,4 +220,53 @@ static boolean isJavaIdentifier(String str) { } return true; } + + /** + * If a module has at least one unqualified export, then it has a public API + * that can be used by other modules. If all of its exports are qualified, only + * modules specified in its descriptor can read from it, and there's no + * use in requiring it for a synthetic module. + * @param md A module descriptor. + * @return true if the module as at least one unqualified export, false otherwise + */ + static boolean hasAtLeastOneUnqualifiedExport(ModuleDescriptor md) { + return md.exports().stream().anyMatch(Predicate.not(ModuleDescriptor.Exports::isQualified)); + } + + /** + * We assume that if a module name starts with "java." or "jdk.", it is a Java + * platform module. We also assume that there are no Java platform modules that + * start with other prefixes. This assumption is true as of Java 17, where "java." + * is for Java SE APIs and "jdk." is for JDK-specific modules. + */ + static boolean isJavaPlatformModule(ModuleDescriptor md) { + return md.name().startsWith("java.") || md.name().startsWith("jdk."); + } + + @SuppressForbidden(reason = "need access to the jar file") + private static List getProvidersFromServiceFile(JarFile jf, String sf) throws IOException { + try (BufferedReader bf = new BufferedReader(new InputStreamReader(jf.getInputStream(jf.getEntry(sf)), StandardCharsets.UTF_8))) { + return bf.lines().toList(); + } + } + + private static List createListOrAppend(List currentList, List newList) { + if (currentList == null) { + return List.copyOf(newList); + } + return Stream.concat(currentList.stream(), newList.stream()).toList(); + } + + private static String getServiceName(String sf) { + return sf.substring("META-INF/services/".length()); + } + + private static ModuleDescriptor getDescriptorForModularJar(Path path) { + return ModuleFinder.of(path) + .findAll() + .stream() + .findFirst() + .orElseThrow(() -> new IllegalStateException("found a module descriptor but failed to load a module from " + path)) + .descriptor(); + } } diff --git a/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java b/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java index 374494af24c0f..3a9149e84d8de 100644 --- a/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java +++ b/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java @@ -10,12 +10,13 @@ import org.elasticsearch.core.SuppressForbidden; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; import java.lang.module.Configuration; +import java.lang.module.ModuleDescriptor; import java.lang.module.ModuleFinder; +import java.lang.module.ModuleReference; import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; @@ -26,12 +27,11 @@ import java.security.PrivilegedAction; import java.security.SecureClassLoader; import java.util.Enumeration; -import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; -import java.util.jar.JarFile; +import java.util.function.Predicate; import java.util.stream.Collectors; /** @@ -57,35 +57,58 @@ public class UberModuleClassLoader extends SecureClassLoader implements AutoClos private final ModuleLayer.Controller moduleController; private final Set packageNames; + private static final Map> platformModulesToServices; + + static { + Set unqualifiedExports = ModuleLayer.boot() + .modules() + .stream() + .flatMap(module -> module.getDescriptor().exports().stream()) + .filter(Predicate.not(ModuleDescriptor.Exports::isQualified)) + .map(ModuleDescriptor.Exports::source) + .collect(Collectors.toSet()); + platformModulesToServices = ModuleLayer.boot() + .modules() + .stream() + .map(Module::getDescriptor) + .filter(ModuleSupport::isJavaPlatformModule) + .filter(ModuleSupport::hasAtLeastOneUnqualifiedExport) + .collect( + Collectors.toMap( + ModuleDescriptor::name, + md -> md.provides() + .stream() + .map(ModuleDescriptor.Provides::service) + .filter(name -> unqualifiedExports.contains(packageName(name))) + .collect(Collectors.toSet()) + ) + ); + } + static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set jarUrls) { return getInstance(parent, moduleName, jarUrls, Set.of()); } - @SuppressForbidden(reason = "need access to the jar file") @SuppressWarnings("removal") static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set jarUrls, Set moduleDenyList) { Path[] jarPaths = jarUrls.stream().map(UberModuleClassLoader::urlToPathUnchecked).toArray(Path[]::new); - ModuleFinder finder = ModuleSupport.ofSyntheticPluginModule(moduleName, jarPaths, Set.of()); + Set requires = platformModulesToServices.keySet() + .stream() + .filter(Predicate.not(moduleDenyList::contains)) + .collect(Collectors.toSet()); + Set uses = platformModulesToServices.entrySet() + .stream() + .filter(Predicate.not(entry -> moduleDenyList.contains(entry.getKey()))) + .flatMap(entry -> entry.getValue().stream()) + .collect(Collectors.toSet()); + + ModuleFinder finder = ModuleSupport.ofSyntheticPluginModule(moduleName, jarPaths, requires, uses); ModuleLayer mparent = ModuleLayer.boot(); - Configuration cf = mparent.configuration().resolveAndBind(finder, ModuleFinder.of(), Set.of(moduleName)); - - Set packageNames = new HashSet<>(); - for (URL url : jarUrls) { - try (JarFile jarFile = new JarFile(new File(url.toURI()))) { - Set jarPackages = ModuleSupport.scan(jarFile) - .classFiles() - .stream() - .map(e -> ModuleSupport.toPackageName(e, "/")) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(Collectors.toSet()); - - packageNames.addAll(jarPackages); - } catch (IOException | URISyntaxException e) { - throw new IllegalArgumentException(e); - } - } + // TODO: check that denied modules are not brought as transitive dependencies (or switch to allow-list?) + Configuration cf = mparent.configuration().resolve(finder, ModuleFinder.of(), Set.of(moduleName)); + + Set packageNames = finder.find(moduleName).map(ModuleReference::descriptor).map(ModuleDescriptor::packages).orElseThrow(); PrivilegedAction pa = () -> new UberModuleClassLoader( parent, @@ -93,7 +116,6 @@ static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, jarUrls.toArray(new URL[0]), cf, mparent, - moduleDenyList, packageNames ); return AccessController.doPrivileged(pa); @@ -108,7 +130,6 @@ private UberModuleClassLoader( URL[] jarURLs, Configuration cf, ModuleLayer mparent, - Set moduleDenyList, Set packageNames ) { super(parent); @@ -122,15 +143,6 @@ private UberModuleClassLoader( this.moduleController = ModuleLayer.defineModules(cf, List.of(mparent), s -> this); this.module = this.moduleController.layer().findModule(moduleName).orElseThrow(); - // Every module reads java.base by default, but we can add all other modules - // that are not in the deny list so that plugins can use, for example, java.management - mparent.modules() - .stream() - .filter(Module::isNamed) - .filter(m -> "java.base".equals(m.getName()) == false) - .filter(m -> moduleDenyList.contains(m.getName()) == false) - .forEach(m -> moduleController.addReads(module, m)); - this.packageNames = packageNames; } @@ -255,7 +267,7 @@ protected Class loadClass(String cn, boolean resolve) throws ClassNotFoundExc /** * Returns the package name for the given class name */ - private String packageName(String cn) { + private static String packageName(String cn) { int pos = cn.lastIndexOf('.'); return (pos < 0) ? "" : cn.substring(0, pos); } @@ -282,4 +294,5 @@ public void close() throws Exception { }; AccessController.doPrivileged(pa); } + } diff --git a/server/src/test/java/org/elasticsearch/plugins/UberModuleClassLoaderTests.java b/server/src/test/java/org/elasticsearch/plugins/UberModuleClassLoaderTests.java index 0d0a57f520b78..d790e100df4dc 100644 --- a/server/src/test/java/org/elasticsearch/plugins/UberModuleClassLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/UberModuleClassLoaderTests.java @@ -150,11 +150,11 @@ public void testHideSplitPackageInParentClassloader() throws Exception { Path jar = tempDir.resolve("my-jar.jar"); createMinimalJar(jar, "p.MyClassInPackageP"); - URL[] urls = new URL[] { overlappingJar.toUri().toURL() }; + URL[] urls = new URL[] { toUrl(overlappingJar) }; try ( URLClassLoader parent = URLClassLoader.newInstance(urls, UberModuleClassLoaderTests.class.getClassLoader()); - UberModuleClassLoader loader = UberModuleClassLoader.getInstance(parent, "synthetic", Set.of(jar.toUri().toURL())) + UberModuleClassLoader loader = UberModuleClassLoader.getInstance(parent, "synthetic", Set.of(toUrl(jar))) ) { // stable plugin loader gives us the good class... Class c = loader.loadClass("p.MyClassInPackageP"); @@ -186,11 +186,11 @@ public void testNoParentFirstSearch() throws Exception { Path jar = tempDir.resolve("my-jar.jar"); createMinimalJar(jar, "p." + className); - URL[] urls = new URL[] { overlappingJar.toUri().toURL() }; + URL[] urls = new URL[] { toUrl(overlappingJar) }; try ( URLClassLoader parent = URLClassLoader.newInstance(urls, UberModuleClassLoaderTests.class.getClassLoader()); - UberModuleClassLoader loader = UberModuleClassLoader.getInstance(parent, "synthetic", Set.of(jar.toUri().toURL())) + UberModuleClassLoader loader = UberModuleClassLoader.getInstance(parent, "synthetic", Set.of(toUrl(jar))) ) { // stable plugin loader gives us the good class... Class c = loader.loadClass("p.MyClass"); @@ -302,8 +302,8 @@ public String toString() { UberModuleClassLoader denyListLoader = UberModuleClassLoader.getInstance( UberModuleClassLoaderTests.class.getClassLoader(), "synthetic", - Set.of(jar.toUri().toURL()), - Set.of("java.sql") + Set.of(toUrl(jar)), + Set.of("java.sql", "java.sql.rowset") // if present, java.sql.rowset requires java.sql transitively ) ) { Class denyListed = denyListLoader.loadClass("p.MyImportingClass"); @@ -333,6 +333,134 @@ public void testMultiReleaseJar() throws Exception { assertThat(fooBar.toString(), equalTo("FooBar 0")); } + public void testServiceLoadingWithMetaInf() throws Exception { + Path topLevelDir = createTempDir(getTestName()); + Path jar = topLevelDir.resolve("my-service-jar.jar"); + + createServiceTestJar(jar, false, true); + + try ( + UberModuleClassLoader cl = UberModuleClassLoader.getInstance( + this.getClass().getClassLoader(), + "p.synthetic.test", + Set.of(toUrl(jar)) + ) + ) { + Class demoClass = cl.loadClass("p.ServiceCaller"); + var method = demoClass.getMethod("demo"); + String result = (String) method.invoke(null); + assertThat(result, equalTo("The test string.")); + } + } + + public void testServiceLoadingWithModuleInfo() throws Exception { + Path topLevelDir = createTempDir(getTestName()); + Path jar = topLevelDir.resolve("my-service-jar.jar"); + + createServiceTestJar(jar, true, false); + + try ( + UberModuleClassLoader cl = UberModuleClassLoader.getInstance( + this.getClass().getClassLoader(), + "p.synthetic.test", + Set.of(toUrl(jar)) + ) + ) { + Class demoClass = cl.loadClass("p.ServiceCaller"); + var method = demoClass.getMethod("demo"); + String result = (String) method.invoke(null); + assertThat(result, equalTo("The test string.")); + } + } + + public void testServiceLoadingWithRedundantDeclarations() throws Exception { + Path topLevelDir = createTempDir(getTestName()); + Path jar = topLevelDir.resolve("my-service-jar.jar"); + + createServiceTestJar(jar, true, true); + + try ( + UberModuleClassLoader cl = UberModuleClassLoader.getInstance( + this.getClass().getClassLoader(), + "p.synthetic.test", + Set.of(toUrl(jar)) + ) + ) { + Class demoClass = cl.loadClass("p.ServiceCaller"); + var method = demoClass.getMethod("demo"); + String result = (String) method.invoke(null); + assertThat(result, equalTo("The test string.")); + } + } + + private static void createServiceTestJar(Path jar, boolean modularize, boolean addMetaInfService) throws IOException { + String serviceInterface = """ + package p; + + public interface MyService { + public String getTestString(); + } + """; + String implementingClass = """ + package p; + + public class MyServiceImpl implements MyService { + + public MyServiceImpl() { + // no-args + } + + @Override + public String getTestString() { + return "The test string."; + } + } + """; + String retrievingClass = """ + package p; + + import java.util.ServiceLoader; + import java.util.random.RandomGenerator; + + public class ServiceCaller { + public static String demo() { + // check no error if we load a service from the jdk + ServiceLoader randomLoader = ServiceLoader.load(RandomGenerator.class); + + ServiceLoader loader = ServiceLoader.load(MyService.class, ServiceCaller.class.getClassLoader()); + return loader.findFirst().get().getTestString(); + } + } + """; + String moduleInfo = """ + module p.services { + uses p.MyService; + provides p.MyService with p.MyServiceImpl; + } + """; + + Map sources = new HashMap<>(); + sources.put("p.MyService", serviceInterface); + sources.put("p.MyServiceImpl", implementingClass); + sources.put("p.ServiceCaller", retrievingClass); + if (modularize) { + sources.put("module-info", moduleInfo); + } + var compiledCode = InMemoryJavaCompiler.compile(sources); + Map jarEntries = new HashMap<>(); + jarEntries.put("p/MyService.class", compiledCode.get("p.MyService")); + jarEntries.put("p/MyServiceImpl.class", compiledCode.get("p.MyServiceImpl")); + jarEntries.put("p/ServiceCaller.class", compiledCode.get("p.ServiceCaller")); + if (modularize) { + jarEntries.put("module-info.class", compiledCode.get("module-info")); + } + if (addMetaInfService) { + jarEntries.put("META-INF/services/p.MyService", "p.MyServiceImpl".getBytes(StandardCharsets.UTF_8)); + } + + JarUtils.createJarWithEntries(jar, jarEntries); + } + private static UberModuleClassLoader getLoader(Path jar) { return getLoader(List.of(jar)); } @@ -347,7 +475,7 @@ private static UberModuleClassLoader getLoader(List jars) { private static URL pathToUrlUnchecked(Path path) { try { - return path.toUri().toURL(); + return toUrl(path); } catch (MalformedURLException e) { throw new IllegalArgumentException(e); } @@ -403,7 +531,6 @@ private static void createMinimalJar(Path jar, String className) throws Exceptio private static void createSingleClassJar(Path jar, String canonicalName, String source) throws IOException { Map sources = new HashMap<>(); - // TODO: windows String jarPath = canonicalName.replace(".", "/") + ".class"; sources.put(canonicalName, source); var classToBytes = InMemoryJavaCompiler.compile(sources); @@ -438,4 +565,8 @@ public String toString() { } """, Strings.isNullOrEmpty(packageName) ? "" : "package " + packageName + ";", className, toStringOutput); } + + private static URL toUrl(Path jar) throws MalformedURLException { + return jar.toUri().toURL(); + } } From d3c9564eb17f3ed853683d23d7d51427be596f83 Mon Sep 17 00:00:00 2001 From: David Kilfoyle <41695641+kilfoyle@users.noreply.github.com> Date: Mon, 26 Sep 2022 14:49:12 -0400 Subject: [PATCH 26/30] Update language around eliminating replicate shards (#90375) * Update language around eliminating replicate shards * Update docs/reference/searchable-snapshots/index.asciidoc Co-authored-by: Adam Locke Co-authored-by: Adam Locke --- docs/reference/searchable-snapshots/index.asciidoc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/reference/searchable-snapshots/index.asciidoc b/docs/reference/searchable-snapshots/index.asciidoc index 5b0df4a43f3ca..5f883535c49a2 100644 --- a/docs/reference/searchable-snapshots/index.asciidoc +++ b/docs/reference/searchable-snapshots/index.asciidoc @@ -6,10 +6,11 @@ infrequently accessed and read-only data in a very cost-effective fashion. The <> and <> data tiers use {search-snaps} to reduce your storage and operating costs. -{search-snaps-cap} eliminate the need for <>, -potentially halving the local storage needed to search your data. -{search-snaps-cap} rely on the same snapshot mechanism you already use for -backups and have minimal impact on your snapshot repository storage costs. +{search-snaps-cap} eliminate the need for <> +after rolling over from the hot tier, potentially halving the local storage needed to search +your data. {search-snaps-cap} rely on the same snapshot mechanism you already +use for backups and have minimal impact on your snapshot repository storage +costs. [discrete] [[using-searchable-snapshots]] From 9f0f02d232102eaeb6473947e5a8d39188fa1469 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 26 Sep 2022 12:07:13 -0700 Subject: [PATCH 27/30] Make force merge warning more precise (#90151) We tell users not to force merge unless their index is read-only. This PR proposes to soften the warning and make it more precise. This way, more users can consider force merging for their use case, like those with append-only indices, or those with a small number of updates that can regularly perform a force merge. --- docs/reference/indices/forcemerge.asciidoc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/reference/indices/forcemerge.asciidoc b/docs/reference/indices/forcemerge.asciidoc index f3a4ec7d73e5e..e316e0c5e1ae8 100644 --- a/docs/reference/indices/forcemerge.asciidoc +++ b/docs/reference/indices/forcemerge.asciidoc @@ -39,13 +39,16 @@ deleted documents. Merging normally happens automatically, but sometimes it is useful to trigger a merge manually. // tag::force-merge-read-only-warn[] -WARNING: **Force merge should only be called against an index after you have -finished writing to it.** Force merge can cause very large (>5GB) segments to -be produced, and if you continue to write to such an index then the automatic -merge policy will never consider these segments for future merges until they -mostly consist of deleted documents. This can cause very large segments to -remain in the index which can result in increased disk usage and worse search -performance. +WARNING: **We recommend only force merging a read-only index (meaning the index +is no longer receiving writes).** When documents are updated or deleted, the +old version is not immediately removed, but instead soft-deleted and marked +with a "tombstone". These soft-deleted documents are automatically cleaned up +during regular segment merges. But force merge can cause very large (> 5GB) +segments to be produced, which are not eligible for regular merges. So the +number of soft-deleted documents can then grow rapidly, resulting in higher +disk usage and worse search performance. If you regularly force merge an index +receiving writes, this can also make snapshots more expensive, since the new +documents can't be backed up incrementally. // end::force-merge-read-only-warn[] From 0764ddccb556f9e6f420cd5e37ef6f177ceaa244 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Sep 2022 16:34:00 -0400 Subject: [PATCH 28/30] Graph tests - more data on failure (#90379) Get more data when the graph timeout test fails. Relates to #90286. --- .../java/org/elasticsearch/xpack/graph/test/GraphTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/graph/src/internalClusterTest/java/org/elasticsearch/xpack/graph/test/GraphTests.java b/x-pack/plugin/graph/src/internalClusterTest/java/org/elasticsearch/xpack/graph/test/GraphTests.java index e9178675bd1a6..c0fe47634574d 100644 --- a/x-pack/plugin/graph/src/internalClusterTest/java/org/elasticsearch/xpack/graph/test/GraphTests.java +++ b/x-pack/plugin/graph/src/internalClusterTest/java/org/elasticsearch/xpack/graph/test/GraphTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeResponse; import org.elasticsearch.action.admin.indices.segments.IndexShardSegments; import org.elasticsearch.action.admin.indices.segments.ShardSegments; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings.Builder; import org.elasticsearch.core.TimeValue; @@ -236,7 +237,7 @@ public void testTimedoutQueryCrawl() { grb.createNextHop(QueryBuilders.termQuery("decade", "00s")).addVertexRequest("people").size(100).minDocCount(1); GraphExploreResponse response = grb.get(); - assertTrue(response.isTimedOut()); + assertTrue(Strings.toString(response), response.isTimedOut()); checkVertexDepth(response, 0, "john", "paul", "george", "ringo"); From 94f05da248cd9d6f36f5e54c76ca6743b7bf8c91 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 26 Sep 2022 15:31:16 -0700 Subject: [PATCH 29/30] Add profiling information for knn vector queries (#90200) This adds timers to the dfs phase to profile a knn vector query and provide a breakdown of several parts of the query. --- docs/changelog/90200.yaml | 5 + .../rest-api-spec/test/search/370_profile.yml | 116 +++++++++++++++ .../action/search/DfsQueryPhase.java | 1 + .../elasticsearch/search/dfs/DfsPhase.java | 140 +++++++++++------- .../search/dfs/DfsSearchResult.java | 17 +++ .../search/profile/Profilers.java | 13 ++ .../profile/SearchProfileDfsPhaseResult.java | 86 +++++++++++ .../SearchProfileQueryPhaseResult.java | 33 ++++- .../search/profile/SearchProfileResults.java | 12 +- .../profile/SearchProfileShardResult.java | 21 ++- .../search/query/QuerySearchResult.java | 8 + .../search/KnnSearchSingleNodeTests.java | 1 + .../SearchProfileDfsPhaseResultTests.java | 38 +++++ .../SearchProfileQueryPhaseResultTests.java | 9 +- 14 files changed, 427 insertions(+), 73 deletions(-) create mode 100644 docs/changelog/90200.yaml create mode 100644 server/src/main/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResult.java create mode 100644 server/src/test/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResultTests.java diff --git a/docs/changelog/90200.yaml b/docs/changelog/90200.yaml new file mode 100644 index 0000000000000..36b2017ac7120 --- /dev/null +++ b/docs/changelog/90200.yaml @@ -0,0 +1,5 @@ +pr: 90200 +summary: Add profiling information for knn vector queries +area: Vector Search +type: enhancement +issues: [] diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/370_profile.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/370_profile.yml index 259ff3fb3c66f..485215e7fc058 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/370_profile.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/370_profile.yml @@ -161,3 +161,119 @@ disabling stored fields removes fetch sub phases: - match: { hits.hits.0._index: test } - match: { profile.shards.0.fetch.debug.stored_fields: [] } - is_false: profile.shards.0.fetch.children + +--- +dfs knn vector profiling: + - skip: + version: ' - 8.5.99' + reason: dfs profiling implemented in 8.6.0 + + - do: + indices.create: + index: images + body: + settings: + index.number_of_shards: 1 + mappings: + properties: + image: + type: "dense_vector" + dims: 3 + index: true + similarity: "l2_norm" + + - do: + index: + index: images + id: "1" + refresh: true + body: + image: [1, 5, -20] + + - do: + search: + index: images + body: + profile: true + knn: + field: "image" + query_vector: [-5, 9, -12] + k: 1 + num_candidates: 100 + + - match: { hits.total.value: 1 } + - match: { profile.shards.0.dfs.knn.query.0.type: "DocAndScoreQuery" } + - match: { profile.shards.0.dfs.knn.query.0.description: "DocAndScore[100]" } + - gt: { profile.shards.0.dfs.knn.query.0.time_in_nanos: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.set_min_competitive_score_count: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.set_min_competitive_score: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.match_count: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.match: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.shallow_advance_count: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.shallow_advance: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.next_doc_count: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.next_doc: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.score_count: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.score: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.compute_max_score_count: 0 } + - match: { profile.shards.0.dfs.knn.query.0.breakdown.compute_max_score: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.advance_count: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.advance: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.build_scorer_count: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.build_scorer: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.create_weight: 0 } + - gt: { profile.shards.0.dfs.knn.query.0.breakdown.create_weight_count: 0 } + - gt: { profile.shards.0.dfs.knn.rewrite_time: 0 } + - match: { profile.shards.0.dfs.knn.collector.0.name: "SimpleTopScoreDocCollector" } + - match: { profile.shards.0.dfs.knn.collector.0.reason: "search_top_hits" } + - gt: { profile.shards.0.dfs.knn.collector.0.time_in_nanos: 0 } + +--- +dfs without knn vector profiling: + - skip: + version: ' - 8.5.99' + reason: dfs profiling implemented in 8.6.0 + + - do: + indices.create: + index: keywords + body: + settings: + index.number_of_shards: 1 + mappings: + properties: + keyword: + type: "keyword" + - do: + index: + index: keywords + id: "1" + refresh: true + body: + keyword: "a" + + - do: + search: + index: keywords + search_type: dfs_query_then_fetch + body: + profile: true + query: + term: + keyword: "a" + + - match: { hits.total.value: 1 } + - is_false: profile.shards.0.dfs + + - do: + search: + index: keywords + search_type: query_then_fetch + body: + profile: true + query: + term: + keyword: "a" + + - match: { hits.total.value: 1 } + - is_false: profile.shards.0.dfs diff --git a/server/src/main/java/org/elasticsearch/action/search/DfsQueryPhase.java b/server/src/main/java/org/elasticsearch/action/search/DfsQueryPhase.java index 5e187ad249189..7927779c70b91 100644 --- a/server/src/main/java/org/elasticsearch/action/search/DfsQueryPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/DfsQueryPhase.java @@ -98,6 +98,7 @@ public void run() throws IOException { @Override protected void innerOnResponse(QuerySearchResult response) { try { + response.setSearchProfileDfsPhaseResult(dfsResult.searchProfileDfsPhaseResult()); counter.onResult(response); } catch (Exception e) { context.onPhaseFailure(DfsQueryPhase.this, "", e); diff --git a/server/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java b/server/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java index 16b31bb67a6ec..90f584f73e383 100644 --- a/server/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java +++ b/server/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java @@ -10,15 +10,17 @@ import org.apache.lucene.index.Term; import org.apache.lucene.search.CollectionStatistics; +import org.apache.lucene.search.Collector; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.search.ScoreMode; import org.apache.lucene.search.TermStatistics; -import org.apache.lucene.search.TopDocs; -import org.elasticsearch.index.query.ParsedQuery; +import org.apache.lucene.search.TopScoreDocCollector; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.search.profile.query.CollectorResult; +import org.elasticsearch.search.profile.query.InternalProfileCollector; import org.elasticsearch.search.rescore.RescoreContext; import org.elasticsearch.search.vectors.KnnSearchBuilder; import org.elasticsearch.search.vectors.KnnVectorQueryBuilder; @@ -26,6 +28,7 @@ import java.io.IOException; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -39,71 +42,96 @@ public class DfsPhase { public void execute(SearchContext context) { try { - Map fieldStatistics = new HashMap<>(); - Map stats = new HashMap<>(); - IndexSearcher searcher = new IndexSearcher(context.searcher().getIndexReader()) { - @Override - public TermStatistics termStatistics(Term term, int docFreq, long totalTermFreq) throws IOException { - if (context.isCancelled()) { - throw new TaskCancelledException("cancelled"); - } - TermStatistics ts = super.termStatistics(term, docFreq, totalTermFreq); - if (ts != null) { - stats.put(term, ts); - } - return ts; - } + collectStatistics(context); + executeKnnVectorQuery(context); + } catch (Exception e) { + throw new DfsPhaseExecutionException(context.shardTarget(), "Exception during dfs phase", e); + } + } + + private void collectStatistics(SearchContext context) throws IOException { + Map fieldStatistics = new HashMap<>(); + Map stats = new HashMap<>(); - @Override - public CollectionStatistics collectionStatistics(String field) throws IOException { - if (context.isCancelled()) { - throw new TaskCancelledException("cancelled"); - } - CollectionStatistics cs = super.collectionStatistics(field); - if (cs != null) { - fieldStatistics.put(field, cs); - } - return cs; + IndexSearcher searcher = new IndexSearcher(context.searcher().getIndexReader()) { + @Override + public TermStatistics termStatistics(Term term, int docFreq, long totalTermFreq) throws IOException { + if (context.isCancelled()) { + throw new TaskCancelledException("cancelled"); + } + TermStatistics ts = super.termStatistics(term, docFreq, totalTermFreq); + if (ts != null) { + stats.put(term, ts); } - }; + return ts; + } - searcher.createWeight(context.rewrittenQuery(), ScoreMode.COMPLETE, 1); - for (RescoreContext rescoreContext : context.rescore()) { - for (Query query : rescoreContext.getQueries()) { - searcher.createWeight(context.searcher().rewrite(query), ScoreMode.COMPLETE, 1); + @Override + public CollectionStatistics collectionStatistics(String field) throws IOException { + if (context.isCancelled()) { + throw new TaskCancelledException("cancelled"); + } + CollectionStatistics cs = super.collectionStatistics(field); + if (cs != null) { + fieldStatistics.put(field, cs); } + return cs; } + }; - Term[] terms = stats.keySet().toArray(new Term[0]); - TermStatistics[] termStatistics = new TermStatistics[terms.length]; - for (int i = 0; i < terms.length; i++) { - termStatistics[i] = stats.get(terms[i]); + searcher.createWeight(context.rewrittenQuery(), ScoreMode.COMPLETE, 1); + for (RescoreContext rescoreContext : context.rescore()) { + for (Query query : rescoreContext.getQueries()) { + searcher.createWeight(context.searcher().rewrite(query), ScoreMode.COMPLETE, 1); } + } - context.dfsResult() - .termsStatistics(terms, termStatistics) - .fieldStatistics(fieldStatistics) - .maxDoc(context.searcher().getIndexReader().maxDoc()); + Term[] terms = stats.keySet().toArray(new Term[0]); + TermStatistics[] termStatistics = new TermStatistics[terms.length]; + for (int i = 0; i < terms.length; i++) { + termStatistics[i] = stats.get(terms[i]); + } - // If kNN search is requested, perform kNN query and gather top docs - SearchSourceBuilder source = context.request().source(); - if (source != null && source.knnSearch() != null) { - SearchExecutionContext searchExecutionContext = context.getSearchExecutionContext(); - KnnSearchBuilder knnSearch = source.knnSearch(); + context.dfsResult() + .termsStatistics(terms, termStatistics) + .fieldStatistics(fieldStatistics) + .maxDoc(context.searcher().getIndexReader().maxDoc()); + } - KnnVectorQueryBuilder knnVectorQueryBuilder = knnSearch.toQueryBuilder(); - if (context.request().getAliasFilter().getQueryBuilder() != null) { - knnVectorQueryBuilder.addFilterQuery(context.request().getAliasFilter().getQueryBuilder()); - } - ParsedQuery query = searchExecutionContext.toQuery(knnVectorQueryBuilder); + private void executeKnnVectorQuery(SearchContext context) throws IOException { + SearchSourceBuilder source = context.request().source(); + if (source == null || source.knnSearch() == null) { + return; + } - TopDocs topDocs = searcher.search(query.query(), knnSearch.k()); - DfsKnnResults knnResults = new DfsKnnResults(topDocs.scoreDocs); - context.dfsResult().knnResults(knnResults); - } - } catch (Exception e) { - throw new DfsPhaseExecutionException(context.shardTarget(), "Exception during dfs phase", e); + SearchExecutionContext searchExecutionContext = context.getSearchExecutionContext(); + KnnSearchBuilder knnSearch = context.request().source().knnSearch(); + KnnVectorQueryBuilder knnVectorQueryBuilder = knnSearch.toQueryBuilder(); + + if (context.request().getAliasFilter().getQueryBuilder() != null) { + knnVectorQueryBuilder.addFilterQuery(context.request().getAliasFilter().getQueryBuilder()); } - } + Query query = searchExecutionContext.toQuery(knnVectorQueryBuilder).query(); + TopScoreDocCollector topScoreDocCollector = TopScoreDocCollector.create(knnSearch.k(), Integer.MAX_VALUE); + Collector collector = topScoreDocCollector; + + if (context.getProfilers() != null) { + InternalProfileCollector ipc = new InternalProfileCollector( + topScoreDocCollector, + CollectorResult.REASON_SEARCH_TOP_HITS, + List.of() + ); + context.getProfilers().getCurrentQueryProfiler().setCollector(ipc); + collector = ipc; + } + + context.searcher().search(query, collector); + DfsKnnResults knnResults = new DfsKnnResults(topScoreDocCollector.topDocs().scoreDocs); + context.dfsResult().knnResults(knnResults); + + if (context.getProfilers() != null) { + context.dfsResult().profileResult(context.getProfilers().buildDfsPhaseResults()); + } + } } diff --git a/server/src/main/java/org/elasticsearch/search/dfs/DfsSearchResult.java b/server/src/main/java/org/elasticsearch/search/dfs/DfsSearchResult.java index ffa5f91f5b634..9d4b87e1a205d 100644 --- a/server/src/main/java/org/elasticsearch/search/dfs/DfsSearchResult.java +++ b/server/src/main/java/org/elasticsearch/search/dfs/DfsSearchResult.java @@ -19,6 +19,7 @@ import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.ShardSearchContextId; import org.elasticsearch.search.internal.ShardSearchRequest; +import org.elasticsearch.search.profile.SearchProfileDfsPhaseResult; import java.io.IOException; import java.util.HashMap; @@ -33,6 +34,7 @@ public class DfsSearchResult extends SearchPhaseResult { private Map fieldStatistics = new HashMap<>(); private DfsKnnResults knnResults; private int maxDoc; + private SearchProfileDfsPhaseResult searchProfileDfsPhaseResult; public DfsSearchResult(StreamInput in) throws IOException { super(in); @@ -56,6 +58,9 @@ public DfsSearchResult(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_8_4_0)) { knnResults = in.readOptionalWriteable(DfsKnnResults::new); } + if (in.getVersion().onOrAfter(Version.V_8_6_0)) { + searchProfileDfsPhaseResult = in.readOptionalWriteable(SearchProfileDfsPhaseResult::new); + } } public DfsSearchResult(ShardSearchContextId contextId, SearchShardTarget shardTarget, ShardSearchRequest shardSearchRequest) { @@ -89,6 +94,11 @@ public DfsSearchResult knnResults(DfsKnnResults knnResults) { return this; } + public DfsSearchResult profileResult(SearchProfileDfsPhaseResult searchProfileDfsPhaseResult) { + this.searchProfileDfsPhaseResult = searchProfileDfsPhaseResult; + return this; + } + public Term[] terms() { return terms; } @@ -105,6 +115,10 @@ public DfsKnnResults knnResults() { return knnResults; } + public SearchProfileDfsPhaseResult searchProfileDfsPhaseResult() { + return searchProfileDfsPhaseResult; + } + @Override public void writeTo(StreamOutput out) throws IOException { contextId.writeTo(out); @@ -121,6 +135,9 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_8_4_0)) { out.writeOptionalWriteable(knnResults); } + if (out.getVersion().onOrAfter(Version.V_8_6_0)) { + out.writeOptionalWriteable(searchProfileDfsPhaseResult); + } } public static void writeFieldStats(StreamOutput out, Map fieldStatistics) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/search/profile/Profilers.java b/server/src/main/java/org/elasticsearch/search/profile/Profilers.java index 15c392406a29b..0e36f1426c2dd 100644 --- a/server/src/main/java/org/elasticsearch/search/profile/Profilers.java +++ b/server/src/main/java/org/elasticsearch/search/profile/Profilers.java @@ -66,6 +66,19 @@ public static FetchProfiler startProfilingFetchPhase() { return new FetchProfiler(); } + /** + * Build the results for the dfs phase. + */ + public SearchProfileDfsPhaseResult buildDfsPhaseResults() { + QueryProfiler queryProfiler = getCurrentQueryProfiler(); + QueryProfileShardResult queryProfileShardResult = new QueryProfileShardResult( + queryProfiler.getTree(), + queryProfiler.getRewriteTime(), + queryProfiler.getCollector() + ); + return new SearchProfileDfsPhaseResult(queryProfileShardResult); + } + /** * Build the results for the query phase. */ diff --git a/server/src/main/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResult.java b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResult.java new file mode 100644 index 0000000000000..dbe3d28c6e77d --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResult.java @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.search.profile; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.search.profile.query.QueryProfileShardResult; +import org.elasticsearch.xcontent.InstantiatingObjectParser; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.ParserConstructor; +import org.elasticsearch.xcontent.ToXContentObject; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParser; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; + +public class SearchProfileDfsPhaseResult implements Writeable, ToXContentObject { + + private final QueryProfileShardResult queryProfileShardResult; + + @ParserConstructor + public SearchProfileDfsPhaseResult(@Nullable QueryProfileShardResult queryProfileShardResult) { + this.queryProfileShardResult = queryProfileShardResult; + } + + public SearchProfileDfsPhaseResult(StreamInput in) throws IOException { + queryProfileShardResult = in.readOptionalWriteable(QueryProfileShardResult::new); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeOptionalWriteable(queryProfileShardResult); + } + + private static final ParseField KNN = new ParseField("knn"); + private static final InstantiatingObjectParser PARSER; + + static { + InstantiatingObjectParser.Builder parser = InstantiatingObjectParser.builder( + "search_profile_dfs_phase_result", + true, + SearchProfileDfsPhaseResult.class + ); + parser.declareObject(optionalConstructorArg(), (p, c) -> QueryProfileShardResult.fromXContent(p), KNN); + PARSER = parser.build(); + } + + public static SearchProfileDfsPhaseResult fromXContent(XContentParser parser) throws IOException { + return PARSER.parse(parser, null); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + if (queryProfileShardResult != null) { + builder.startObject(); + builder.field(KNN.getPreferredName()); + queryProfileShardResult.toXContent(builder, params); + builder.endObject(); + } + return builder; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SearchProfileDfsPhaseResult that = (SearchProfileDfsPhaseResult) o; + return Objects.equals(queryProfileShardResult, that.queryProfileShardResult); + } + + @Override + public int hashCode() { + return Objects.hash(queryProfileShardResult); + } +} diff --git a/server/src/main/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResult.java b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResult.java index 38c5cb250f9d0..caeac1a74e379 100644 --- a/server/src/main/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResult.java +++ b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResult.java @@ -8,6 +8,7 @@ package org.elasticsearch.search.profile; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -25,6 +26,8 @@ */ public class SearchProfileQueryPhaseResult implements Writeable { + private SearchProfileDfsPhaseResult searchProfileDfsPhaseResult; + private final List queryProfileResults; private final AggregationProfileShardResult aggProfileShardResult; @@ -33,11 +36,15 @@ public SearchProfileQueryPhaseResult( List queryProfileResults, AggregationProfileShardResult aggProfileShardResult ) { + this.searchProfileDfsPhaseResult = null; this.aggProfileShardResult = aggProfileShardResult; this.queryProfileResults = Collections.unmodifiableList(queryProfileResults); } public SearchProfileQueryPhaseResult(StreamInput in) throws IOException { + if (in.getVersion().onOrAfter(Version.V_8_6_0)) { + searchProfileDfsPhaseResult = in.readOptionalWriteable(SearchProfileDfsPhaseResult::new); + } int profileSize = in.readVInt(); List queryProfileResults = new ArrayList<>(profileSize); for (int i = 0; i < profileSize; i++) { @@ -50,6 +57,9 @@ public SearchProfileQueryPhaseResult(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { + if (out.getVersion().onOrAfter(Version.V_8_6_0)) { + out.writeOptionalWriteable(searchProfileDfsPhaseResult); + } out.writeVInt(queryProfileResults.size()); for (QueryProfileShardResult queryShardResult : queryProfileResults) { queryShardResult.writeTo(out); @@ -57,6 +67,14 @@ public void writeTo(StreamOutput out) throws IOException { aggProfileShardResult.writeTo(out); } + public void setSearchProfileDfsPhaseResult(SearchProfileDfsPhaseResult searchProfileDfsPhaseResult) { + this.searchProfileDfsPhaseResult = searchProfileDfsPhaseResult; + } + + public SearchProfileDfsPhaseResult getSearchProfileDfsPhaseResult() { + return searchProfileDfsPhaseResult; + } + public List getQueryProfileResults() { return queryProfileResults; } @@ -66,16 +84,17 @@ public AggregationProfileShardResult getAggregationProfileResults() { } @Override - public boolean equals(Object obj) { - if (obj == null || getClass() != obj.getClass()) { - return false; - } - SearchProfileQueryPhaseResult other = (SearchProfileQueryPhaseResult) obj; - return queryProfileResults.equals(other.queryProfileResults) && aggProfileShardResult.equals(other.aggProfileShardResult); + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SearchProfileQueryPhaseResult that = (SearchProfileQueryPhaseResult) o; + return Objects.equals(searchProfileDfsPhaseResult, that.searchProfileDfsPhaseResult) + && Objects.equals(queryProfileResults, that.queryProfileResults) + && Objects.equals(aggProfileShardResult, that.aggProfileShardResult); } @Override public int hashCode() { - return Objects.hash(queryProfileResults, aggProfileShardResult); + return Objects.hash(searchProfileDfsPhaseResult, queryProfileResults, aggProfileShardResult); } } diff --git a/server/src/main/java/org/elasticsearch/search/profile/SearchProfileResults.java b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileResults.java index 3a7fedb63583e..84d46af3036ac 100644 --- a/server/src/main/java/org/elasticsearch/search/profile/SearchProfileResults.java +++ b/server/src/main/java/org/elasticsearch/search/profile/SearchProfileResults.java @@ -129,6 +129,7 @@ private static void parseProfileResultsEntry(XContentParser parser, Map queryProfileResults = new ArrayList<>(); AggregationProfileShardResult aggProfileShardResult = null; ProfileResult fetchResult = null; @@ -145,7 +146,7 @@ private static void parseProfileResultsEntry(XContentParser parser, Map aggregations() { return aggregations; } + public void setSearchProfileDfsPhaseResult(SearchProfileDfsPhaseResult searchProfileDfsPhaseResult) { + if (profileShardResults == null) { + return; + } + profileShardResults.setSearchProfileDfsPhaseResult(searchProfileDfsPhaseResult); + } + /** * Returns and nulls out the profiled results for this search, or potentially null if result was empty. * This allows to free up memory once the profiled result is consumed. diff --git a/server/src/test/java/org/elasticsearch/action/search/KnnSearchSingleNodeTests.java b/server/src/test/java/org/elasticsearch/action/search/KnnSearchSingleNodeTests.java index 65c0df0f25e77..ae716482c1afe 100644 --- a/server/src/test/java/org/elasticsearch/action/search/KnnSearchSingleNodeTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/KnnSearchSingleNodeTests.java @@ -56,6 +56,7 @@ public void testKnnWithQuery() throws IOException { float[] queryVector = randomVector(); KnnSearchBuilder knnSearch = new KnnSearchBuilder("vector", queryVector, 5, 50).boost(5.0f); SearchResponse response = client().prepareSearch("index") + .setProfile(true) .setKnnSearch(knnSearch) .setQuery(QueryBuilders.matchQuery("text", "goodnight")) .addFetchField("*") diff --git a/server/src/test/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResultTests.java b/server/src/test/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResultTests.java new file mode 100644 index 0000000000000..fa68d4420df23 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/profile/SearchProfileDfsPhaseResultTests.java @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.search.profile; + +import org.elasticsearch.common.io.stream.Writeable.Reader; +import org.elasticsearch.search.profile.query.QueryProfileShardResultTests; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.xcontent.XContentParser; + +import java.io.IOException; + +public class SearchProfileDfsPhaseResultTests extends AbstractSerializingTestCase { + + static SearchProfileDfsPhaseResult createTestItem() { + return new SearchProfileDfsPhaseResult(rarely() ? null : QueryProfileShardResultTests.createTestItem()); + } + + @Override + protected SearchProfileDfsPhaseResult createTestInstance() { + return createTestItem(); + } + + @Override + protected Reader instanceReader() { + return SearchProfileDfsPhaseResult::new; + } + + @Override + protected SearchProfileDfsPhaseResult doParseInstance(XContentParser parser) throws IOException { + return SearchProfileDfsPhaseResult.fromXContent(parser); + } +} diff --git a/server/src/test/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResultTests.java b/server/src/test/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResultTests.java index 85b3ee9c7ceb5..eb99b1ef26f93 100644 --- a/server/src/test/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResultTests.java +++ b/server/src/test/java/org/elasticsearch/search/profile/SearchProfileQueryPhaseResultTests.java @@ -26,7 +26,14 @@ static SearchProfileQueryPhaseResult createTestItem() { queryProfileResults.add(QueryProfileShardResultTests.createTestItem()); } AggregationProfileShardResult aggProfileShardResult = AggregationProfileShardResultTests.createTestItem(1); - return new SearchProfileQueryPhaseResult(queryProfileResults, aggProfileShardResult); + SearchProfileQueryPhaseResult searchProfileQueryPhaseResult = new SearchProfileQueryPhaseResult( + queryProfileResults, + aggProfileShardResult + ); + if (randomBoolean()) { + searchProfileQueryPhaseResult.setSearchProfileDfsPhaseResult(SearchProfileDfsPhaseResultTests.createTestItem()); + } + return searchProfileQueryPhaseResult; } @Override From 7729972df55d4b76c48f3262ff53d625d9348874 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 26 Sep 2022 18:19:32 -0500 Subject: [PATCH 30/30] Add tls support to gradle run/run-ccs (#90051) This commit adds TLS for the transport layer and optional support for https for `./gradlew run` and `./gradlew run-ccs` tasks. A new system property --https can be passed to expose https. Transport layer TLS is always enabled which is always applicable for `./gradlew run-css`, but only applicable for `./gradlew run` when multiple nodes are configured. Additional certificates and instructions for use are also included in the commit. --- TESTING.asciidoc | 4 +- .../src/main/resources/run.ssl/private-ca.key | 27 ++++++++ .../main/resources/run.ssl/private-cert1.p12 | Bin 0 -> 2461 bytes .../main/resources/run.ssl/private-cert2.p12 | Bin 0 -> 2461 bytes .../src/main/resources/run.ssl/public-ca.pem | 20 ++++++ .../src/main/resources/run.ssl/readme.md | 54 ++++++++++++++++ .../testclusters/ElasticsearchNode.java | 8 ++- .../gradle/testclusters/RunTask.java | 60 +++++++++++++++++- 8 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 build-tools-internal/src/main/resources/run.ssl/private-ca.key create mode 100644 build-tools-internal/src/main/resources/run.ssl/private-cert1.p12 create mode 100644 build-tools-internal/src/main/resources/run.ssl/private-cert2.p12 create mode 100644 build-tools-internal/src/main/resources/run.ssl/public-ca.pem create mode 100644 build-tools-internal/src/main/resources/run.ssl/readme.md diff --git a/TESTING.asciidoc b/TESTING.asciidoc index 5e31492424dc9..82350c1f4715c 100644 --- a/TESTING.asciidoc +++ b/TESTING.asciidoc @@ -107,10 +107,12 @@ password: `elastic-password`. - In order to set a different keystore password: `--keystore-password` - In order to set an Elasticsearch setting, provide a setting with the following prefix: `-Dtests.es.` - In order to pass a JVM setting, e.g. to disable assertions: `-Dtests.jvm.argline="-da"` +- In order to use HTTPS: ./gradlew run --https ==== Customizing the test cluster for ./gradlew run You may need to customize the cluster configuration for the ./gradlew run task. +The settings can be set via the command line, but other options require updating the task itself. You can simply find the task in the source code and configure it there. (The task is currently defined in build-tools-internal/src/main/groovy/elasticsearch.run.gradle) However, this requires modifying a source controlled file and is subject to accidental commits. @@ -118,7 +120,7 @@ Alternatively, you can use a Gradle init script to inject custom build logic wit For example: -To enable HTTPS for use with ./gradlew run, an extraConfigFile is needed to be added to the cluster configuration. +To use a custom certificate for HTTPS with `./gradlew run`, you can do the following. Create a file (for example ~/custom-run.gradle) with the following contents: ------------------------------------- rootProject { diff --git a/build-tools-internal/src/main/resources/run.ssl/private-ca.key b/build-tools-internal/src/main/resources/run.ssl/private-ca.key new file mode 100644 index 0000000000000..7f3f65d659514 --- /dev/null +++ b/build-tools-internal/src/main/resources/run.ssl/private-ca.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAvdQ8UlHoUA8SnP4MtP96p9Iv0z2qTPJr+xPYtn5ZSfQQhX0t +pEfGFqnLxeYjLGM8jjaGUZGkDaWEAtFSytm9azmnBp+smrDfoG6vNj13qu051UFi +uEbqSYNDEDU65KvphWOFQUaGU0CyX0WK6+jTiC5JCO/baBgH+hxdACYDa/fb7ydd +cubHj0JvGbfWpg441JReU/QF7w9fQcp25ea9aNCMXmg+NRfSY7LAFFue9H+LGvOI +20gkQlDtI3DJqeTz0rzJaF44colZUAULhoQyb0jyhy0OWX1iYlMaIrAPoEORoOJ8 +Z/7PH7loWY8eiiWRu4EHLU06u1vysqwMb+Ij6QIDAQABAoIBAFUq8Stv4z50HMJB ++zqDuyirVVi9vHgMdeTuuRbbpjzXW0hA6ubfauEFKk8uW06RcXxOu0HCiauzvIA1 +ISOwwFrowWbn4d1/iL2mm0bHGjceewmSbfPGoVv9H+wYLcWl2b5GceVg+mhEySKU +hWkliy54sbzoPHS9/2o4KoOkCnn4Mpx+xK9J4mPb1nr7PyS7j2swrGESgtjJKleW +r8n8R0HrJvs3xF6CzteLDiDPy92eK3SWkk1xZcAzmJWFhawsTzKemY91+e1bXFsP +eOw2zrL5i/unTRRIWigAQORLxwqcqIkyeXDh8BIZBi2+9DGIlKgDLaZJ1UucR7ce +zaccfa8CgYEAzF5DNQ2LooAWviK9Z5wWY5z9ekaZNIPxwVw8GOp5BZ9O0mDoMgiC +G9TPx5QdZCQexg4iWGQsWdMsyIgFVJmWgiexjMMycSvcCj2SCi7aeTJ0aitdSoyq +jgPEn9uaN08fl3M65usCJyzPh6pgBG8BFyR5jRL5Y4zfR4LHFXZRYxsCgYEA7cmi +9T/ZGo2FX5iL0TelCr4AyoS0JH0HcvVqJ1ofqHz6VVy84y+m/OzJWbocc5gEOm/U +uryP/5Tv7HDYJHvcliqK+NJKtqF8sLK5LenYuMPX4XITapAJJk16yfjBnaq8oWVG +kjdCEHKcNrkx+/2TznnLXcJhJcrm6R8K+8FlAUsCgYB+GbOyapc8P3jI/TqNUbxm +3plw91rVEoz7WGQko5jlJTVHjk/3f1R4w8kpRnUUM01hu5rpm3XaPvklCvjvCI3b +5Y4iYtcfCYcOMouICPz5R26ZjARWWZFra1vJn4D6m7HMi2dO0LdVYMr01OXGFpA/ +rVvq9kg3atbikwkwbv8s/QKBgHVOcvkATY9e37xAWkGVbPM2ttcxzljt4V3iGkNd +n56UQT8ZaAm/+WZvPgno2Z5hETzu7IhO+87/X7lKFicxf6oJRNPpkng0hHn7QYWY +BpVn8DlE+LUqZ4kg0gGPmZy5nSMV/lGltw68K7qHdFQ3TdKfnScc/KYTSgUZjmaS +isyvAoGAbZVftomScxIQgHOCVMzPh9G3pUNCr3hakQRXzRaHWI7Z4ULUmlCU6wC+ +/2G3/nVNFy8w2TbASK0T0vXbCLJAyLpYicdPkKaZ20jvhzMOdGvPY/CtU5i/BDsX +RgLbHCWVy/1Fd08o1Z39uDbItIqxCLzahbF4UijxtOFmQtbCc1M= +-----END RSA PRIVATE KEY----- diff --git a/build-tools-internal/src/main/resources/run.ssl/private-cert1.p12 b/build-tools-internal/src/main/resources/run.ssl/private-cert1.p12 new file mode 100644 index 0000000000000000000000000000000000000000..5ee58ae60613e25baf4c91119bc5b29254821ca8 GIT binary patch literal 2461 zcmV;O31apzf(e-d0Ru3C310>YDuzgg_YDCD0ic2jPy~VrOfZ58NHBr}{{{&vhDe6@ z4FLxRpn?PNFoFZ@0s#Opf&=9S2`Yw2hW8Bt2LUh~1_~;MNQU<0sm^3L&xRhh7;N!TJoryzE$2VU&2Q$2bGe=vPrZ zxC+a5JVzsMS;(3^=1<0cca8~wf^Kd}mTD{6y%N4V9!yST~}wZg}= z6?163XeqX4#F?mUY;IQEGmereF>K9*=A{0iggL^*;W;rETjlY^O-!b9TduT*Ak^^5 zEy(dhB6z~0=Nhkk2uEf8#Oi~|ATj7jApY#K8o2f_q-e^rGC1Oe`evhv@BYmGR-HM2 z$83WB0zk&3W1vd2NKs8)jD}sYihSxZvS5x(yoOzWKdlc(#%Hm z7lJ(pmnc#@ma4R>bKIaFe24`7o>Ze{Xv7zV7T5G>A+*$q#C9|-V+W-|x5@Q1Oy)Z2 zJCnjlb+TGqgsH1O+?g=E{@ndsT)8AvBf;?zzjjUkk!T~ut1R`svB*QP!b;H4zPr*T z$M_1%$D3xq%&v;2x`2EfL}$4g<1=En-<_;4{6ph@fg4&$&7+a7-rO&x^NJ$fP2C+P zpgq6;ZhF#ZmhPKza-#iWKA{bhV7E~V1yCw8M^_|%e3UH9a?S*8Lp>3{Bj$KZWxr$( z3R6C!45RyeH^R~KgDrf1dmMp-@|(pXQ@F1%+j$K&@T7YY@KItx8rRs6;(K!}bTc$@ zc=iqKyJ=2nD)`g*A7^OjfF)O#zlPyW>!!IY`2oA(O0X2B3knhNk;%g8-;Hw%WINbg zy4o!_*y*Ga4w3_X@^|CdD;9TVA={L~W3Vs##_3zrUecstleP4FvO>C~42a$ro*6AF zMeqW3ME8aFZ5ccs;f8KZFQ|%^pqI~iHBpa4jid$0DugGUQ4r*8o{${-l0IV*u-g7< znS}1~RI`Jc;p>*V%UOv;@l_!2>+D^Lf>(KFQeD!Wz=#AgXGQYHqDc4l6tAf~u%DFe zX|S}^!R)17iB%k^*}Z~AX<9@+2!1Em1*^x~ZsJ{_nE+AfZ zU=PI9mx+d*f1LPdO!SM0mawugl{bu*N4SfQC!k)@Y;WHM1^mhZHrgZ9f-Ix&+5cs( z!o4_)`mX37DOH0=ynfZi*8mjS;+?Usm*#X7FoFd^1_>&LNQU+(}R!Ar(n>14b8Xix%DURwjx zJ(Xo{uH-fMK^f%Une^Na${~s?T=S@s}x>Dl38ILJhTIJA?tmBHxn7VzIl2b zA~>JWZ}46fX*BTo+b_t!0o@b}Wyd*@MUypN(jn`kPRH5IFT;A)z-Z?_{K&pDx8nla z(})7Fhl-4cTfnwRD0x=9ywcf1g61laAl^Lv@TdE1JA@D$g0q4+2_NG70AVI$2@F;y(vFOI*F85->O9Wf> zFoHl=vUceB^k3RC(uUA{Syk4svl(nLcc}E6i_PjqA+7`(Pqdj<{HB_j!{DOSEJ?Z_ zrV&LK#x`0GcJ!dETB2(_M!`@jD0=$D|F*^ifN*E<=e>0St0_dOn1+FVL#60(j`lzK z0l1HWs^V)$I~BZK`N`c^S8ieuG+^G6=~ZwowW{bl{0YKyAvi8b)9#KU{eAY$%7r#< z!a1aHoA&%JNtOBX9WXW&EIK}Ei^8MC3~FxKSGka4rRdbQFHf$&wu#!<2P2YdDT*Tt z4Pk$+!hF-?ct$COfOL^BzEyGd-~PO93E%n0NCpn6Nf8uHsz1m(1{%wF`Vj{UF;K4| zz%etePc1_CS4su`CEA#<-OF8N12JvS%D?K=vb)`n@oj*3zWv&30C|a$_v^WU2tAm8 zX-@F=$2&?#+YFKSl@&3;QvO8DoIm2- zQY)RiBa#Yx>y?P6hbm1^F~hrsYe zi^N?Ps>*IYdH80prZCn^AbuX6XeF2-+qce3$|O+h9lxlt&4uxr{Rb%!`@)zb&c4e$ zx>`&=5`o;+YH@0*RzsT?nj|-1DH8+`NYjvV{nG6f{yw*J)u1o{UB3%$Q3&xC)e-~Y zzg>MskDkPp~$SZv4zMJG0JonMJfW=;rxemz)%XNkC(c#N$6FD95g)5xiicF7x z8M$mfzxP^C4m%+1;Fbc%x?E(Co|8 zu%Q@4m(^7>+F?}W*j`v_z@^Y7$<*nHo0ief94dy>F(U3T)nu9fHNKaj^Js8YitSzA z4>JrKvE6NR)Fy>`^XqG4yoquv)mZ$ZXW=a1O`wdKOu+ydrT{S|Fe3&DDuzgg_YDCF z6)_eB6#2oeT((>wkog0xw6yN?uR*M#G%ztRAutIB1uG5%0vZJX1Qde8=!OKhP4*_N bdQ0l@g1DP4ia`Vj#YYy_X5K_Q0s;sCcId6Z literal 0 HcmV?d00001 diff --git a/build-tools-internal/src/main/resources/run.ssl/private-cert2.p12 b/build-tools-internal/src/main/resources/run.ssl/private-cert2.p12 new file mode 100644 index 0000000000000000000000000000000000000000..b70c8ce014a7cb1944b3ed81f73173138c413f6d GIT binary patch literal 2461 zcmV;O31apzf(e-d0Ru3C310>YDuzgg_YDCD0ic2jPy~VrOfZ58NHBr}{{{&vhDe6@ z4FLxRpn?PNFoFZ@0s#Opf&=9S2`Yw2hW8Bt2LUh~1_~;MNQU^I)w)=a zxj%Z@K^X*uH*&*agv~i6UU2*l(o4}U4d?5zKqS9#K-22sktxH1c;M|A5E`O6RJhWv z?b{mziq>*3*tb*L(&(KRez_l}sy#i)+p(SV(A;DQ@YrW`9G};5L~6ztz&>jf0^U&A_Py**e3U;LBtY!1es$MK?C^{H6p{?q9KzTC=cs3K?CbR(K( zJW$zIYT~2ecz?gj(8MRxfxFg2h;9j9p1J4=y`y)V>8)`assY91k2J&hCj@ax+#t=v8QSx;#ZKzDz>Y1p#aIN?C> zCTwRU%hYwU>n^-AHpCdATmhQa22l=n)LfW~3|Tgjp_|k$Isb)dS_zCJPD}9fC>qu* z46|JwFAV!)D3LW^mgeZs4Bk0A=)Zo})z>{+sWR-8g>5#4Y&U^qenKywI)WIv9Y)ok znFMxRG))^zZ*JkhXIJiulD;eMzvDC~zW3D+s%zld*}0exq8mAEoI~qM9@V}tcbg1O zN~oChsTWCkFdIg!iVQrJ|U6HRR4Y#e838Tvh`7TIPcz| zL8Mm5VZ%W!vy$7=w9^4d*L%7L6UFYux!d<+S~(mAmrbKurI7qd1x%FG4gbqg<;a{aJ zQ(Qs$W7+Z;4M;7SEl9*cQD4@!hEVq{m^_lONAighL)TOSi5Xcx#qi-OKH(dBhWs>V z@|2hBPQ8Ohd7e{=88Ct&QAI=hDn;xANQ>~IFoFd^1_>&LNQUiJ zC6NLG2ml0v1jt-6{QcU2MtL_YSJxn&_B}%Z0+7)knTL~BX`Afm7r(YDe=QOdayktG zu<$1d#pkS>TPy1QC9t}qVRYu^U@+PniehQ^_S<340)mN0w3Nlvw*C`Hf1>?RjuD+N(T&l3 zAj01--rbxzW=~b(&*;&0v(@0sB6QZ{Xd?84pDvQmRW(2g4|QJC#C2Z?jE22J7zW&t zA3F&Uu1F zk-Eqy7ihGT_3_(8br@)xVD(JCBx=?hCt3a)`jQrE&Ruj1TE`MQL4FdnL-}gcK?R$2 z9slc%hrVOV%H3bUN)c{qN*7xmz-QuWuJHlB(%Gk=BHIIlv|9n(frS_FlV}}Mj7}7n z%{yV5vk`+FX}-z2YMvsB50R>n^suIQT(c62ph9i6ia9+MH04%|( zF-_(;OR6*Q;;&@Nt8(-9sv(lEmy%L+5D$!b75#I&^s9R_T{xwE*`HB9?K}Ri`mb%0 z5RCcTgnFCx%(a@~?n={1{dk{|AZk5Z`iwl?9m;q|p&S_TnrjtKR)u>oZ+xNQ?(jTZg4w@Dg_?MqJpJ zEBUSH>PotgPCg2}S_45)dP>r(7>lDDLtgqX7BTC=vhDA--3g;6%~bY}5TfWJRBZS_ zBxocU&T|k>KVCtKVGT)cyxegy2OujzMoz#tEInNL$X^c@T+ma8am79|>HP+?jtnHG z;=WTK#=AhsR-Vr$WOMcH##X&&aDIq?wEv#%tEBP2eiPF0V)Ro5i-hbokK*Idwlp?G zKiJ+_kpaf1j8g;GYQ>HQP{jA~^lsM1e=FiUY#0a}%l27W0Ir+WZudd6a}=m!I#m@5 z-MKcF-#8sXOnXVqI{l@J;5f~%5UWG)%>>ld=6A0+;+hj$26NHXMk zjS!tHOIL^g?f6ueox9#BF)!$VHfMm6;aDDcXx$&Pr7-r<+Z3;nlzs)gp+~&=0mxB^ zFckRg)p&2L;P~~%A+nA)#8hN(bUh#>z`w!938Xl~KV7D6h&H}#Y^%&gi0Y-$K;`%QG^})(Ni%cFe3&DDuzgg_YDCF z6)_eB6yqsArI_qsf&gF43Ta=-{cTXvI506VAutIB1uG5%0vZJX1QZ@9wabBgm{zWu b4opN6!bVDNI4%STy3cKA!n-eW0s;sCaT%gZ literal 0 HcmV?d00001 diff --git a/build-tools-internal/src/main/resources/run.ssl/public-ca.pem b/build-tools-internal/src/main/resources/run.ssl/public-ca.pem new file mode 100644 index 0000000000000..7fd8d34ed27e7 --- /dev/null +++ b/build-tools-internal/src/main/resources/run.ssl/public-ca.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDSTCCAjGgAwIBAgIUUWUwcrChxm++t6WUAImb+lZyGGYwDQYJKoZIhvcNAQEL +BQAwNDEyMDAGA1UEAxMpRWxhc3RpYyBDZXJ0aWZpY2F0ZSBUb29sIEF1dG9nZW5l +cmF0ZWQgQ0EwHhcNMjIwOTEzMjIzMzUxWhcNNDIwOTEzMjIzMzUxWjA0MTIwMAYD +VQQDEylFbGFzdGljIENlcnRpZmljYXRlIFRvb2wgQXV0b2dlbmVyYXRlZCBDQTCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL3UPFJR6FAPEpz+DLT/eqfS +L9M9qkzya/sT2LZ+WUn0EIV9LaRHxhapy8XmIyxjPI42hlGRpA2lhALRUsrZvWs5 +pwafrJqw36BurzY9d6rtOdVBYrhG6kmDQxA1OuSr6YVjhUFGhlNAsl9Fiuvo04gu +SQjv22gYB/ocXQAmA2v32+8nXXLmx49Cbxm31qYOONSUXlP0Be8PX0HKduXmvWjQ +jF5oPjUX0mOywBRbnvR/ixrziNtIJEJQ7SNwyank89K8yWheOHKJWVAFC4aEMm9I +8octDll9YmJTGiKwD6BDkaDifGf+zx+5aFmPHoolkbuBBy1NOrtb8rKsDG/iI+kC +AwEAAaNTMFEwHQYDVR0OBBYEFMQM/59PJi8sWLRHnE9WKbGJ1df1MB8GA1UdIwQY +MBaAFMQM/59PJi8sWLRHnE9WKbGJ1df1MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZI +hvcNAQELBQADggEBAAmLsK2N/H1E3aBgHiitJog0+YMmyERI4q3Q5p3ZfIxATUUN +uxA0Q25rlwtU6VkxvQwjrSZ01CvKDdCdPV8X8TVu8xm6+3JXXmubVnejGlhIjwzW +Z9fRz4R8l9/EjoMYG2igdcc9y9kqkzsDQ4TLcn3EVkkiAENrO2flKEQOY4SXg7cN +v0I/tyIoyPT0qCr/f47BPIq0kQmfhghd5vPmiiBIQtK303/0y3/AUtLGGZLGIGNF +KAxLPoN4ly7VXQ0RZ5PDy6r0EJt5Ega8LDXPMELKN52ubxkHDE47Laq6kuRRqKxF +yXgEpxZV0K8fXvp4UXoCLonbYSQRky0GgUOvqso= +-----END CERTIFICATE----- diff --git a/build-tools-internal/src/main/resources/run.ssl/readme.md b/build-tools-internal/src/main/resources/run.ssl/readme.md new file mode 100644 index 0000000000000..099a0dc2cacf2 --- /dev/null +++ b/build-tools-internal/src/main/resources/run.ssl/readme.md @@ -0,0 +1,54 @@ +This directory contains public and private keys for use with the manual run tasks. + +All certs expire September 13, 2042, +have no required password, +require the hostname to be localhost or esX (where X=the name in the certificate) for hostname verification, +were created by the elasticsearch-certutil tool. + +## Usage + +Use public-ca.pem as the `xpack.security.transport.ssl.certificate_authorities` +Use public-ca.pem for third party tools to trust the certificates in use. +Use private-certX.p12 for node X's `xpack.security.[transport|http].ssl.keystore` + +## Certificate Authority (CA): + +* private-ca.key : the private key of the signing CA in PEM format. Useful if desired to sign additional certificates. +* public-ca.pem : the public key of the signing CA in PEM format. Useful as the certificate_authorities. + +To recreate CA : +```bash +bin/elasticsearch-certutil ca -pem -days 7305 +unzip elastic-stack-ca.zip +mv ca/ca.crt public-ca.pem +mv ca/ca.key private-ca.key +```` + +## Node Certificates signed by the CA + +* private-certX.p12 : the public/private key of the certificate signed by the CA. Useful as the keystore. + +To create new certificates signed by CA: +```bash +export i=1 # update this +rm -rf certificate-bundle.zip public-cert$i.pem private-cert$i.key private-cert$i.p12 instance +bin/elasticsearch-certutil cert -ca-key private-ca.key -ca-cert public-ca.pem -days 7305 -pem -dns localhost,es$i -ip 127.0.0.1,::1 +unzip certificate-bundle.zip +mv instance/instance.crt public-cert$i.pem +mv instance/instance.key private-cert$i.key +openssl pkcs12 -export -out private-cert$i.p12 -inkey private-cert$i.key -in public-cert$i.pem -passout pass: #convert public/private key to p12 +``` + +Other useful commands +```bash +openssl rsa -in private-ca.key -check # check private key +openssl pkcs12 -info -in private-cert$i.p12 -nodes -nocerts # read private keys from p12 +openssl pkcs12 -info -in private-cert$i.p12 -nodes -nokeys # read public keys from p12 +openssl x509 -in public-cert$i.pem -text # decode PEM formatted public key +openssl s_client -showcerts -connect localhost:9200 getSettings() { return settings.getNormalizedCollection(); } + @Internal + Set getSettingKeys() { + return settings.keySet(); + } + @Nested public List getSystemProperties() { return systemProperties.getNormalizedCollection(); @@ -1675,7 +1680,8 @@ void configureHttpWait(WaitForHttpResource wait) { if (settings.containsKey("xpack.security.http.ssl.certificate")) { wait.setCertificateAuthorities(getConfigDir().resolve(settings.get("xpack.security.http.ssl.certificate").toString()).toFile()); } - if (settings.containsKey("xpack.security.http.ssl.keystore.path")) { + if (settings.containsKey("xpack.security.http.ssl.keystore.path") + && settings.containsKey("xpack.security.http.ssl.certificate_authorities") == false) { // Can not set both trust stores and CA wait.setTrustStoreFile(getConfigDir().resolve(settings.get("xpack.security.http.ssl.keystore.path").toString()).toFile()); } if (keystoreSettings.containsKey("xpack.security.http.ssl.keystore.secure_password")) { diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/RunTask.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/RunTask.java index 6a8ad5a635bb9..502ddabeda57c 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/RunTask.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/RunTask.java @@ -17,21 +17,27 @@ import java.io.BufferedReader; import java.io.Closeable; +import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.BooleanSupplier; import java.util.function.Function; import java.util.stream.Collectors; public class RunTask extends DefaultTestClustersTask { - private static final Logger logger = Logging.getLogger(RunTask.class); public static final String CUSTOM_SETTINGS_PREFIX = "tests.es."; + private static final Logger logger = Logging.getLogger(RunTask.class); + private static final String tlsCertificateAuthority = "public-ca.pem"; + private static final String httpsCertificate = "private-cert1.p12"; + private static final String transportCertificate = "private-cert2.p12"; private Boolean debug = false; @@ -41,6 +47,12 @@ public class RunTask extends DefaultTestClustersTask { private String keystorePassword = ""; + private Boolean useHttps = false; + + private final Path tlsBasePath = Path.of( + new File(getProject().getProjectDir(), "build-tools-internal/src/main/resources/run.ssl").toURI() + ); + @Option(option = "debug-jvm", description = "Enable debugging configuration, to allow attaching a debugger to elasticsearch.") public void setDebug(boolean enabled) { this.debug = enabled; @@ -86,6 +98,17 @@ public String getDataDir() { return dataDir.toString(); } + @Option(option = "https", description = "Helper option to enable HTTPS") + public void setUseHttps(boolean useHttps) { + this.useHttps = useHttps; + } + + @Input + @Optional + public Boolean getUseHttps() { + return useHttps; + } + @Override public void beforeStart() { int httpPort = 9200; @@ -120,6 +143,22 @@ public void beforeStart() { if (keystorePassword.length() > 0) { node.keystorePassword(keystorePassword); } + if (useHttps) { + validateHelperOption("--https", "xpack.security.http.ssl", node); + node.setting("xpack.security.http.ssl.enabled", "true"); + node.extraConfigFile("https.keystore", tlsBasePath.resolve(httpsCertificate).toFile()); + node.extraConfigFile("https.ca", tlsBasePath.resolve(tlsCertificateAuthority).toFile()); + node.setting("xpack.security.http.ssl.keystore.path", "https.keystore"); + node.setting("xpack.security.http.ssl.certificate_authorities", "https.ca"); + } + if (findConfiguredSettingsByPrefix("xpack.security.transport.ssl", node).isEmpty()) { + node.setting("xpack.security.transport.ssl.enabled", "true"); + node.setting("xpack.security.transport.ssl.client_authentication", "required"); + node.extraConfigFile("transport.keystore", tlsBasePath.resolve(transportCertificate).toFile()); + node.extraConfigFile("transport.ca", tlsBasePath.resolve(tlsCertificateAuthority).toFile()); + node.setting("xpack.security.transport.ssl.keystore.path", "transport.keystore"); + node.setting("xpack.security.transport.ssl.certificate_authorities", "transport.ca"); + } } } if (debug) { @@ -192,4 +231,23 @@ public void runAndWait() throws IOException { } } } + + /** + * Disallow overlap between helper options and explicit configuration + */ + private void validateHelperOption(String option, String prefix, ElasticsearchNode node) { + Set preConfigured = findConfiguredSettingsByPrefix(prefix, node); + if (preConfigured.isEmpty() == false) { + throw new IllegalArgumentException("Can not use " + option + " with " + String.join(",", preConfigured)); + } + } + + /** + * Find any settings configured with a given prefix + */ + private Set findConfiguredSettingsByPrefix(String prefix, ElasticsearchNode node) { + Set preConfigured = new HashSet<>(); + node.getSettingKeys().stream().filter(key -> key.startsWith(prefix)).forEach(k -> preConfigured.add(prefix)); + return preConfigured; + } }