From c00b626dfbef857c9e2d7fe678c1d7453dde21ef Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Sat, 14 Oct 2023 13:04:36 +0200 Subject: [PATCH 01/20] Add test that proves you can write a ByteReference using its iterator (#100703) --- .../common/bytes/AbstractBytesReference.java | 14 -------------- .../elasticsearch/common/bytes/BytesArray.java | 18 ++++++++++++++++++ .../common/bytes/BytesReference.java | 4 +++- .../common/bytes/ZeroBytesReference.java | 11 ++++++----- .../common/bytes/ZeroBytesReferenceTests.java | 9 +++++++++ .../indices/IndicesRequestCacheTests.java | 6 ++++++ .../bytes/AbstractBytesReferenceTestCase.java | 16 ++++++++++++++++ 7 files changed, 58 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReference.java index f29423757c0e2..de4ff4d6da025 100644 --- a/server/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReference.java +++ b/server/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReference.java @@ -84,20 +84,6 @@ public String utf8ToString() { return toBytesRef().utf8ToString(); } - @Override - public BytesRefIterator iterator() { - return new BytesRefIterator() { - BytesRef ref = length() == 0 ? null : toBytesRef(); - - @Override - public BytesRef next() { - BytesRef r = ref; - ref = null; // only return it once... - return r; - } - }; - } - @Override public boolean equals(Object other) { if (this == other) { diff --git a/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java b/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java index d01ef44916cf7..1e171b954aa7d 100644 --- a/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java +++ b/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java @@ -9,6 +9,7 @@ package org.elasticsearch.common.bytes; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.BytesRefIterator; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.util.ByteUtils; @@ -111,6 +112,23 @@ public BytesRef toBytesRef() { return new BytesRef(bytes, offset, length); } + @Override + public BytesRefIterator iterator() { + if (length == 0) { + return BytesRefIterator.EMPTY; + } + return new BytesRefIterator() { + BytesRef ref = toBytesRef(); + + @Override + public BytesRef next() { + BytesRef r = ref; + ref = null; // only return it once... + return r; + } + }; + } + @Override public long ramBytesUsed() { return bytes.length; diff --git a/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java index f667bd1a49004..21292a92d1dc1 100644 --- a/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java +++ b/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java @@ -184,7 +184,9 @@ static BytesReference fromByteArray(ByteArray byteArray, int length) { /** * Returns a BytesRefIterator for this BytesReference. This method allows - * access to the internal pages of this reference without copying them. Use with care! + * access to the internal pages of this reference without copying them. + * It must return direct references to the pages, not copies. Use with care! + * * @see BytesRefIterator */ BytesRefIterator iterator(); diff --git a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java index e2db14e83fe44..ecacc29f45164 100644 --- a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java +++ b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java @@ -12,6 +12,8 @@ import org.apache.lucene.util.BytesRefIterator; import org.elasticsearch.common.util.PageCacheRecycler; +import java.util.stream.IntStream; + /** * A {@link BytesReference} of the given length which contains all zeroes. */ @@ -56,16 +58,15 @@ public BytesRef toBytesRef() { @Override public BytesRefIterator iterator() { - if (length <= PageCacheRecycler.BYTE_PAGE_SIZE) { - return super.iterator(); - } - final byte[] buffer = new byte[PageCacheRecycler.BYTE_PAGE_SIZE]; + final byte[] buffer = new byte[Math.min(length, PageCacheRecycler.BYTE_PAGE_SIZE)]; return new BytesRefIterator() { - int remaining = length; @Override public BytesRef next() { + if (IntStream.range(0, buffer.length).map(i -> buffer[i]).anyMatch(b -> b != 0)) { + throw new AssertionError("Internal pages from ZeroBytesReference must be zero"); + } if (remaining > 0) { final int nextLength = Math.min(remaining, buffer.length); remaining -= nextLength; diff --git a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java index c7c6b5958b0d2..f90cb870ea22a 100644 --- a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java +++ b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java @@ -8,6 +8,10 @@ package org.elasticsearch.common.bytes; +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; + public class ZeroBytesReferenceTests extends AbstractBytesReferenceTestCase { @Override @@ -35,4 +39,9 @@ public void testSliceToBytesRef() { // ZeroBytesReference shifts offsets } + public void testWriteWithIterator() throws IOException { + AssertionError error = expectThrows(AssertionError.class, () -> super.testWriteWithIterator()); + assertThat(error.getMessage(), containsString("Internal pages from ZeroBytesReference must be zero")); + } + } diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java index e004a20ffc203..590dc72e2a72b 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java @@ -21,6 +21,7 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.BytesRefIterator; import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.bytes.AbstractBytesReference; import org.elasticsearch.common.bytes.BytesReference; @@ -598,6 +599,11 @@ public BytesRef toBytesRef() { return null; } + @Override + public BytesRefIterator iterator() { + return BytesRefIterator.EMPTY; + } + @Override public long ramBytesUsed() { return 0; diff --git a/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java index b5360df6ea9c4..675db5bbd6330 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java @@ -666,4 +666,20 @@ public void testIndexOf() throws IOException { final byte missing = randomValueOtherThanMany(map::containsKey, ESTestCase::randomByte); assertEquals(-1, bytesReference.indexOf(missing, randomIntBetween(0, Math.max(0, size - 1)))); } + + public void testWriteWithIterator() throws IOException { + final int length = randomIntBetween(1024, 1024 * 1024); + final byte[] bytes = new byte[length]; + random().nextBytes(bytes); + final BytesReference bytesReference = newBytesReference(length); + final BytesRefIterator iterator = bytesReference.iterator(); + BytesRef bytesRef; + int offset = 0; + while ((bytesRef = iterator.next()) != null) { + final int len = Math.min(bytesRef.length, length - offset); + System.arraycopy(bytes, offset, bytesRef.bytes, bytesRef.offset, len); + offset += len; + } + assertArrayEquals(bytes, BytesReference.toBytes(bytesReference)); + } } From ec819e4a23e09f270ef2558e68da06cbf3293871 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sun, 15 Oct 2023 16:50:42 +0100 Subject: [PATCH 02/20] Relax cleanup check in SnapshotStressTestsIT (#100855) We can't assert no leaked blobs here because today the first cleanup leaves the original `RepositoryData` in place so the second cleanup is not a no-op. Relates #100718 --- .../snapshots/SnapshotStressTestsIT.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStressTestsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStressTestsIT.java index 98cbfeb5b3285..a23be834fcc53 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStressTestsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStressTestsIT.java @@ -801,15 +801,21 @@ private void startCleaner() { if (result.bytes() > 0L || result.blobs() > 0L) { // we could legitimately run into dangling blobs as the result of a shard snapshot failing half-way // through the snapshot because of a concurrent index-close or -delete. The second round of cleanup on - // the same repository however must always fully remove any dangling blobs since we block all concurrent - // operations on the repository here + // the same repository however should always find no more dangling blobs and be a no-op since we block all + // concurrent operations on the repository. client.admin() .cluster() .prepareCleanupRepository(trackedRepository.repositoryName) .execute(mustSucceed(secondCleanupRepositoryResponse -> { final RepositoryCleanupResult secondCleanupResult = secondCleanupRepositoryResponse.result(); - assertThat(Strings.toString(secondCleanupResult), secondCleanupResult.blobs(), equalTo(0L)); - assertThat(Strings.toString(secondCleanupResult), secondCleanupResult.bytes(), equalTo(0L)); + if (secondCleanupResult.blobs() == 1) { + // The previous cleanup actually leaves behind a stale index-N blob, so this cleanup removes it + // and reports it in its response. When https://github.com/elastic/elasticsearch/pull/100718 is + // fixed the second cleanup will be a proper no-op and we can remove this lenience -- TODO + } else { + assertThat(Strings.toString(secondCleanupResult), secondCleanupResult.blobs(), equalTo(0L)); + assertThat(Strings.toString(secondCleanupResult), secondCleanupResult.bytes(), equalTo(0L)); + } Releasables.close(releaseAll); logger.info("--> completed second cleanup of [{}]", trackedRepository.repositoryName); startCleaner(); From 65b4d594aed017b92c434e05e5662c6752bb0058 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 16 Oct 2023 10:01:26 +1100 Subject: [PATCH 03/20] Push s3 requests count via metrics API (#100383) This PR builds on top of #100464 to publish s3 request count via the metrics API. The metric takes the name of `repositories.requests.count` with attributes/dimensions of `{"repo_type": "s3", "repo_name": "xxx", "operation": "xxx", "purpose": "xxx"}`. Closes: ES-6801 --- docs/changelog/100383.yaml | 5 + .../s3/S3BlobStoreRepositoryTests.java | 141 +++++++++++++++++- .../repositories/s3/S3BlobStore.java | 41 ++++- .../repositories/s3/S3RepositoryPlugin.java | 4 + .../common/blobstore/OperationPurpose.java | 13 ++ .../java/org/elasticsearch/node/Node.java | 3 +- .../repositories/RepositoriesModule.java | 6 +- .../repositories/RepositoriesModuleTests.java | 13 +- .../telemetry/DelegatingMeter.java | 108 ++++++++++++++ 9 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 docs/changelog/100383.yaml create mode 100644 test/framework/src/main/java/org/elasticsearch/telemetry/DelegatingMeter.java diff --git a/docs/changelog/100383.yaml b/docs/changelog/100383.yaml new file mode 100644 index 0000000000000..6cda66149b2cc --- /dev/null +++ b/docs/changelog/100383.yaml @@ -0,0 +1,5 @@ +pr: 100383 +summary: Push s3 requests count via metrics API +area: Distributed +type: enhancement +issues: [] diff --git a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index 72a6fa1026555..7c571c9fbe71a 100644 --- a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -31,11 +31,14 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.PluginsService; +import org.elasticsearch.plugins.TelemetryPlugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.RepositoryData; @@ -48,7 +51,11 @@ import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.snapshots.mockstore.BlobStoreWrapper; +import org.elasticsearch.telemetry.DelegatingMeter; +import org.elasticsearch.telemetry.TelemetryProvider; +import org.elasticsearch.telemetry.metric.LongCounter; import org.elasticsearch.telemetry.metric.Meter; +import org.elasticsearch.telemetry.tracing.Tracer; import org.elasticsearch.test.BackgroundIndexer; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.junit.annotations.TestLogging; @@ -62,18 +69,26 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import java.util.stream.StreamSupport; +import static org.elasticsearch.repositories.RepositoriesModule.METRIC_REQUESTS_COUNT; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; @@ -126,7 +141,7 @@ protected Settings repositorySettings(String repoName) { @Override protected Collection> nodePlugins() { - return Collections.singletonList(TestS3RepositoryPlugin.class); + return List.of(TestS3RepositoryPlugin.class, TestTelemetryPlugin.class); } @Override @@ -212,6 +227,68 @@ public void testAbortRequestStats() throws Exception { assertEquals(assertionErrorMsg, mockCalls, sdkRequestCounts); } + public void testMetrics() throws Exception { + // Create the repository and perform some activities + final String repository = createRepository(randomRepositoryName()); + final String index = "index-no-merges"; + createIndex(index, 1, 0); + + final long nbDocs = randomLongBetween(10_000L, 20_000L); + try (BackgroundIndexer indexer = new BackgroundIndexer(index, client(), (int) nbDocs)) { + waitForDocs(nbDocs, indexer); + } + flushAndRefresh(index); + ForceMergeResponse forceMerge = client().admin().indices().prepareForceMerge(index).setFlush(true).setMaxNumSegments(1).get(); + assertThat(forceMerge.getSuccessfulShards(), equalTo(1)); + assertHitCount(client().prepareSearch(index).setSize(0).setTrackTotalHits(true).get(), nbDocs); + + final String snapshot = "snapshot"; + assertSuccessfulSnapshot(clusterAdmin().prepareCreateSnapshot(repository, snapshot).setWaitForCompletion(true).setIndices(index)); + assertAcked(client().admin().indices().prepareDelete(index)); + assertSuccessfulRestore(clusterAdmin().prepareRestoreSnapshot(repository, snapshot).setWaitForCompletion(true)); + ensureGreen(index); + assertHitCount(client().prepareSearch(index).setSize(0).setTrackTotalHits(true).get(), nbDocs); + assertAcked(clusterAdmin().prepareDeleteSnapshot(repository, snapshot).get()); + + final Map aggregatedMetrics = new HashMap<>(); + // Compare collected stats and metrics for each node and they should be the same + for (var nodeName : internalCluster().getNodeNames()) { + final BlobStoreRepository blobStoreRepository; + try { + blobStoreRepository = (BlobStoreRepository) internalCluster().getInstance(RepositoriesService.class, nodeName) + .repository(repository); + } catch (RepositoryMissingException e) { + continue; + } + + final BlobStore blobStore = blobStoreRepository.blobStore(); + final BlobStore delegateBlobStore = ((BlobStoreWrapper) blobStore).delegate(); + final S3BlobStore s3BlobStore = (S3BlobStore) delegateBlobStore; + final Map statsCollectors = s3BlobStore + .getStatsCollectors().collectors; + + final var plugins = internalCluster().getInstance(PluginsService.class, nodeName).filterPlugins(TestTelemetryPlugin.class); + assertThat(plugins, hasSize(1)); + final Map, AtomicLong> metrics = plugins.get(0).metrics; + + assertThat(statsCollectors.size(), equalTo(metrics.size())); + metrics.forEach((attributes, counter) -> { + final S3BlobStore.Operation operation = S3BlobStore.Operation.parse((String) attributes.get("operation")); + final S3BlobStore.StatsKey statsKey = new S3BlobStore.StatsKey( + operation, + OperationPurpose.parse((String) attributes.get("purpose")) + ); + assertThat(statsCollectors, hasKey(statsKey)); + assertThat(counter.get(), equalTo(statsCollectors.get(statsKey).counter.sum())); + + aggregatedMetrics.compute(operation.getKey(), (k, v) -> v == null ? counter.get() : v + counter.get()); + }); + } + + // Metrics number should be consistent with server side request count as well. + assertThat(aggregatedMetrics, equalTo(getMockRequestCounts())); + } + public void testRequestStatsWithOperationPurposes() throws IOException { final String repoName = createRepository(randomRepositoryName()); final RepositoriesService repositoriesService = internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class); @@ -356,7 +433,7 @@ protected S3Repository createRepository( BigArrays bigArrays, RecoverySettings recoverySettings ) { - return new S3Repository(metadata, registry, getService(), clusterService, bigArrays, recoverySettings, Meter.NOOP) { + return new S3Repository(metadata, registry, getService(), clusterService, bigArrays, recoverySettings, getMeter()) { @Override public BlobStore blobStore() { @@ -476,4 +553,64 @@ private boolean isMultiPartUpload(String request) { || Regex.simpleMatch("PUT /*/*?*uploadId=*", request); } } + + public static class TestTelemetryPlugin extends Plugin implements TelemetryPlugin { + + private final Map, AtomicLong> metrics = ConcurrentCollections.newConcurrentMap(); + + private final LongCounter longCounter = new LongCounter() { + @Override + public void increment() { + throw new UnsupportedOperationException(); + } + + @Override + public void incrementBy(long inc) { + throw new UnsupportedOperationException(); + } + + @Override + public void incrementBy(long inc, Map attributes) { + assertThat( + attributes, + allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose")) + ); + metrics.computeIfAbsent(attributes, k -> new AtomicLong()).addAndGet(inc); + } + + @Override + public String getName() { + return METRIC_REQUESTS_COUNT; + } + }; + + private final Meter meter = new DelegatingMeter(Meter.NOOP) { + @Override + public LongCounter registerLongCounter(String name, String description, String unit) { + assertThat(name, equalTo(METRIC_REQUESTS_COUNT)); + return longCounter; + } + + @Override + public LongCounter getLongCounter(String name) { + assertThat(name, equalTo(METRIC_REQUESTS_COUNT)); + return longCounter; + } + }; + + @Override + public TelemetryProvider getTelemetryProvider(Settings settings) { + return new TelemetryProvider() { + @Override + public Tracer getTracer() { + return Tracer.NOOP; + } + + @Override + public Meter getMeter() { + return meter; + } + }; + } + } } diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java index 3ff0497b42719..1371e51017bee 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.telemetry.metric.LongCounter; import org.elasticsearch.telemetry.metric.Meter; import org.elasticsearch.threadpool.ThreadPool; @@ -49,6 +50,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.core.Strings.format; +import static org.elasticsearch.repositories.RepositoriesModule.METRIC_REQUESTS_COUNT; class S3BlobStore implements BlobStore { @@ -79,6 +81,7 @@ class S3BlobStore implements BlobStore { private final ThreadPool threadPool; private final Executor snapshotExecutor; private final Meter meter; + private final LongCounter requestCounter; private final StatsCollectors statsCollectors = new StatsCollectors(); @@ -109,6 +112,7 @@ class S3BlobStore implements BlobStore { this.threadPool = threadPool; this.snapshotExecutor = threadPool.executor(ThreadPool.Names.SNAPSHOT); this.meter = meter; + this.requestCounter = this.meter.getLongCounter(METRIC_REQUESTS_COUNT); s3RequestRetryStats = new S3RequestRetryStats(getMaxRetries()); threadPool.scheduleWithFixedDelay(() -> { var priorRetryStats = s3RequestRetryStats; @@ -138,13 +142,24 @@ public TimeValue getCompareAndExchangeTimeToLive() { // metrics collector that ignores null responses that we interpret as the request not reaching the S3 endpoint due to a network // issue - private static class IgnoreNoResponseMetricsCollector extends RequestMetricCollector { + class IgnoreNoResponseMetricsCollector extends RequestMetricCollector { - private final LongAdder counter = new LongAdder(); + final LongAdder counter = new LongAdder(); private final Operation operation; + private final Map attributes; - private IgnoreNoResponseMetricsCollector(Operation operation) { + private IgnoreNoResponseMetricsCollector(Operation operation, OperationPurpose purpose) { this.operation = operation; + this.attributes = Map.of( + "repo_type", + S3Repository.TYPE, + "repo_name", + repositoryMetadata.name(), + "operation", + operation.getKey(), + "purpose", + purpose.getKey() + ); } @Override @@ -152,6 +167,7 @@ public final void collectMetrics(Request request, Response response) { if (response != null) { assert assertConsistencyBetweenHttpRequestAndOperation(request, operation); counter.add(getRequestCount(request)); + requestCounter.incrementBy(getRequestCount(request), attributes); } } @@ -360,15 +376,26 @@ String getKey() { Operation(String key) { this.key = key; } + + static Operation parse(String s) { + for (Operation operation : Operation.values()) { + if (operation.key.equals(s)) { + return operation; + } + } + throw new IllegalArgumentException( + Strings.format("invalid operation [%s] expected one of [%s]", s, Strings.arrayToCommaDelimitedString(Operation.values())) + ); + } } record StatsKey(Operation operation, OperationPurpose purpose) {} - static class StatsCollectors { + class StatsCollectors { final Map collectors = new ConcurrentHashMap<>(); RequestMetricCollector getMetricCollector(Operation operation, OperationPurpose purpose) { - return collectors.computeIfAbsent(new StatsKey(operation, purpose), k -> buildMetricCollector(k.operation())); + return collectors.computeIfAbsent(new StatsKey(operation, purpose), k -> buildMetricCollector(k.operation(), k.purpose())); } Map statsMap() { @@ -377,8 +404,8 @@ Map statsMap() { return Map.copyOf(m); } - IgnoreNoResponseMetricsCollector buildMetricCollector(Operation operation) { - return new IgnoreNoResponseMetricsCollector(operation); + IgnoreNoResponseMetricsCollector buildMetricCollector(Operation operation, OperationPurpose purpose) { + return new IgnoreNoResponseMetricsCollector(operation, purpose); } } } diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java index ca1dcfae879b1..4a8d4ab6bab18 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java @@ -167,4 +167,8 @@ public void reload(Settings settings) { public void close() throws IOException { getService().close(); } + + protected Meter getMeter() { + return meter.get(); + } } diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/OperationPurpose.java b/server/src/main/java/org/elasticsearch/common/blobstore/OperationPurpose.java index 2cfa309c1f7c1..cc6c9a467c2fe 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/OperationPurpose.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/OperationPurpose.java @@ -8,6 +8,8 @@ package org.elasticsearch.common.blobstore; +import org.elasticsearch.common.Strings; + /** * The purpose of an operation against the blobstore. For example, it can be useful for stats collection * as well as other things that requires further differentiation for the same blob operation. @@ -27,4 +29,15 @@ public enum OperationPurpose { public String getKey() { return key; } + + public static OperationPurpose parse(String s) { + for (OperationPurpose purpose : OperationPurpose.values()) { + if (purpose.key.equals(s)) { + return purpose; + } + } + throw new IllegalArgumentException( + Strings.format("invalid purpose [%s] expected one of [%s]", s, Strings.arrayToCommaDelimitedString(OperationPurpose.values())) + ); + } } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 1b970fc3283db..2d17c73e774a0 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -893,7 +893,8 @@ protected Node( clusterService, bigArrays, xContentRegistry, - recoverySettings + recoverySettings, + telemetryProvider ); RepositoriesService repositoryService = repositoriesModule.getRepositoryService(); repositoriesServiceReference.set(repositoryService); diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java index 621bd98e3f299..bb522cddd22f7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java @@ -18,6 +18,7 @@ import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.snapshots.SnapshotRestoreException; +import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -33,6 +34,7 @@ */ public final class RepositoriesModule { + public static final String METRIC_REQUESTS_COUNT = "repositories.requests.count"; private final RepositoriesService repositoriesService; public RepositoriesModule( @@ -42,8 +44,10 @@ public RepositoriesModule( ClusterService clusterService, BigArrays bigArrays, NamedXContentRegistry namedXContentRegistry, - RecoverySettings recoverySettings + RecoverySettings recoverySettings, + TelemetryProvider telemetryProvider ) { + telemetryProvider.getMeter().registerLongCounter(METRIC_REQUESTS_COUNT, "repository request counter", "unit"); Map factories = new HashMap<>(); factories.put( FsRepository.TYPE, diff --git a/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java b/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java index 24edc9dac1229..e99dcb9dae561 100644 --- a/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/RepositoriesModuleTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.indices.recovery.RecoverySettings; import org.elasticsearch.plugins.RepositoryPlugin; +import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -70,8 +71,7 @@ public void testCanRegisterTwoRepositoriesWithDifferentTypes() { mock(ClusterService.class), MockBigArrays.NON_RECYCLING_INSTANCE, contentRegistry, - recoverySettings - ); + recoverySettings, TelemetryProvider.NOOP); } public void testCannotRegisterTwoRepositoriesWithSameTypes() { @@ -89,8 +89,7 @@ public void testCannotRegisterTwoRepositoriesWithSameTypes() { clusterService, MockBigArrays.NON_RECYCLING_INSTANCE, contentRegistry, - recoverySettings - ) + recoverySettings, TelemetryProvider.NOOP) ); assertEquals("Repository type [type1] is already registered", ex.getMessage()); @@ -113,8 +112,7 @@ public void testCannotRegisterTwoInternalRepositoriesWithSameTypes() { clusterService, MockBigArrays.NON_RECYCLING_INSTANCE, contentRegistry, - recoverySettings - ) + recoverySettings, TelemetryProvider.NOOP) ); assertEquals("Internal repository type [type1] is already registered", ex.getMessage()); @@ -136,8 +134,7 @@ public void testCannotRegisterNormalAndInternalRepositoriesWithSameTypes() { clusterService, MockBigArrays.NON_RECYCLING_INSTANCE, contentRegistry, - recoverySettings - ) + recoverySettings, TelemetryProvider.NOOP) ); assertEquals("Internal repository type [type1] is already registered as a non-internal repository", ex.getMessage()); diff --git a/test/framework/src/main/java/org/elasticsearch/telemetry/DelegatingMeter.java b/test/framework/src/main/java/org/elasticsearch/telemetry/DelegatingMeter.java new file mode 100644 index 0000000000000..25333c869dbf3 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/telemetry/DelegatingMeter.java @@ -0,0 +1,108 @@ +/* + * 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.telemetry; + +import org.elasticsearch.telemetry.metric.DoubleCounter; +import org.elasticsearch.telemetry.metric.DoubleGauge; +import org.elasticsearch.telemetry.metric.DoubleHistogram; +import org.elasticsearch.telemetry.metric.DoubleUpDownCounter; +import org.elasticsearch.telemetry.metric.LongCounter; +import org.elasticsearch.telemetry.metric.LongGauge; +import org.elasticsearch.telemetry.metric.LongHistogram; +import org.elasticsearch.telemetry.metric.LongUpDownCounter; +import org.elasticsearch.telemetry.metric.Meter; + +public class DelegatingMeter implements Meter { + + private final Meter delegate; + + public DelegatingMeter(Meter delegate) { + this.delegate = delegate; + } + + @Override + public DoubleCounter registerDoubleCounter(String name, String description, String unit) { + return delegate.registerDoubleCounter(name, description, unit); + } + + @Override + public DoubleCounter getDoubleCounter(String name) { + return delegate.getDoubleCounter(name); + } + + @Override + public DoubleUpDownCounter registerDoubleUpDownCounter(String name, String description, String unit) { + return delegate.registerDoubleUpDownCounter(name, description, unit); + } + + @Override + public DoubleUpDownCounter getDoubleUpDownCounter(String name) { + return delegate.getDoubleUpDownCounter(name); + } + + @Override + public DoubleGauge registerDoubleGauge(String name, String description, String unit) { + return delegate.registerDoubleGauge(name, description, unit); + } + + @Override + public DoubleGauge getDoubleGauge(String name) { + return delegate.getDoubleGauge(name); + } + + @Override + public DoubleHistogram registerDoubleHistogram(String name, String description, String unit) { + return delegate.registerDoubleHistogram(name, description, unit); + } + + @Override + public DoubleHistogram getDoubleHistogram(String name) { + return delegate.getDoubleHistogram(name); + } + + @Override + public LongCounter registerLongCounter(String name, String description, String unit) { + return delegate.registerLongCounter(name, description, unit); + } + + @Override + public LongCounter getLongCounter(String name) { + return delegate.getLongCounter(name); + } + + @Override + public LongUpDownCounter registerLongUpDownCounter(String name, String description, String unit) { + return delegate.registerLongUpDownCounter(name, description, unit); + } + + @Override + public LongUpDownCounter getLongUpDownCounter(String name) { + return delegate.getLongUpDownCounter(name); + } + + @Override + public LongGauge registerLongGauge(String name, String description, String unit) { + return delegate.registerLongGauge(name, description, unit); + } + + @Override + public LongGauge getLongGauge(String name) { + return delegate.getLongGauge(name); + } + + @Override + public LongHistogram registerLongHistogram(String name, String description, String unit) { + return delegate.registerLongHistogram(name, description, unit); + } + + @Override + public LongHistogram getLongHistogram(String name) { + return delegate.getLongHistogram(name); + } +} From cb184639d248b1fc77cc85ebe2484ce929c00182 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 16 Oct 2023 06:05:24 +0100 Subject: [PATCH 04/20] Execute local action via client in RemoteClusterNodesAction (#100876) Rather than sending a nodes-info request to the local node via its transport service, we should use the `Client` to invoke the action directly. --- .../remote/RemoteClusterNodesAction.java | 50 +++++------- .../remote/RemoteClusterNodesActionTests.java | 78 +++++++++++-------- 2 files changed, 65 insertions(+), 63 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesAction.java index 473c271b43956..befec9d468f19 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesAction.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.admin.cluster.remote; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionListenerResponseHandler; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; @@ -17,10 +16,10 @@ import org.elasticsearch.action.admin.cluster.node.info.NodesInfoAction; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoMetrics; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest; -import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.nodes.BaseNodeResponse; +import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; @@ -29,7 +28,6 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.RemoteClusterServerInfo; -import org.elasticsearch.transport.TransportResponseHandler; import org.elasticsearch.transport.TransportService; import java.io.IOException; @@ -95,47 +93,41 @@ public List getNodes() { } public static class TransportAction extends HandledTransportAction { - private final TransportService transportService; + private final Client client; @Inject - public TransportAction(TransportService transportService, ActionFilters actionFilters) { + public TransportAction(TransportService transportService, ActionFilters actionFilters, Client client) { super(RemoteClusterNodesAction.NAME, transportService, actionFilters, Request::new, EsExecutors.DIRECT_EXECUTOR_SERVICE); - this.transportService = transportService; + this.client = client; } @Override protected void doExecute(Task task, Request request, ActionListener listener) { - final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); + final ThreadContext threadContext = client.threadPool().getThreadContext(); try (var ignore = threadContext.stashContext()) { threadContext.markAsSystemContext(); if (request.remoteClusterServer) { final NodesInfoRequest nodesInfoRequest = new NodesInfoRequest().clear() .addMetrics(NodesInfoMetrics.Metric.REMOTE_CLUSTER_SERVER.metricName()); - transportService.sendRequest( - transportService.getLocalNode(), - NodesInfoAction.NAME, - nodesInfoRequest, - new ActionListenerResponseHandler<>(listener.delegateFailureAndWrap((l, response) -> { - final List remoteClusterNodes = response.getNodes().stream().map(nodeInfo -> { - final RemoteClusterServerInfo remoteClusterServerInfo = nodeInfo.getInfo(RemoteClusterServerInfo.class); - if (remoteClusterServerInfo == null) { - return null; - } - return nodeInfo.getNode().withTransportAddress(remoteClusterServerInfo.getAddress().publishAddress()); - }).filter(Objects::nonNull).toList(); - l.onResponse(new Response(remoteClusterNodes)); - }), NodesInfoResponse::new, TransportResponseHandler.TRANSPORT_WORKER) - ); + client.execute(NodesInfoAction.INSTANCE, nodesInfoRequest, listener.delegateFailureAndWrap((l, response) -> { + l.onResponse(new Response(response.getNodes().stream().map(nodeInfo -> { + final RemoteClusterServerInfo remoteClusterServerInfo = nodeInfo.getInfo(RemoteClusterServerInfo.class); + if (remoteClusterServerInfo == null) { + return null; + } + return nodeInfo.getNode().withTransportAddress(remoteClusterServerInfo.getAddress().publishAddress()); + }).filter(Objects::nonNull).toList())); + })); } else { final NodesInfoRequest nodesInfoRequest = new NodesInfoRequest().clear(); - transportService.sendRequest( - transportService.getLocalNode(), - NodesInfoAction.NAME, + client.execute( + NodesInfoAction.INSTANCE, nodesInfoRequest, - new ActionListenerResponseHandler<>(listener.delegateFailureAndWrap((l, response) -> { - final List nodes = response.getNodes().stream().map(BaseNodeResponse::getNode).toList(); - l.onResponse(new Response(nodes)); - }), NodesInfoResponse::new, TransportResponseHandler.TRANSPORT_WORKER) + listener.delegateFailureAndWrap( + (l, response) -> l.onResponse( + new Response(response.getNodes().stream().map(BaseNodeResponse::getNode).toList()) + ) + ) ); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesActionTests.java index 18c586a4a51ae..00a4ad29f68d3 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/remote/RemoteClusterNodesActionTests.java @@ -10,7 +10,10 @@ import org.elasticsearch.TransportVersion; import org.elasticsearch.Version; -import org.elasticsearch.action.ActionListenerResponseHandler; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; import org.elasticsearch.action.admin.cluster.node.info.NodeInfo; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoAction; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoMetrics; @@ -18,6 +21,7 @@ import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.client.internal.support.AbstractClient; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeUtils; @@ -42,9 +46,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -55,11 +56,6 @@ public void testDoExecuteForRemoteServerNodes() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); when(threadPool.getThreadContext()).thenReturn(threadContext); - final TransportService transportService = mock(TransportService.class); - final DiscoveryNode localNode = mock(DiscoveryNode.class); - when(transportService.getLocalNode()).thenReturn(localNode); - when(transportService.getThreadPool()).thenReturn(threadPool); - // Prepare nodesInfo response final int numberOfNodes = randomIntBetween(1, 6); final List nodeInfos = new ArrayList<>(); @@ -107,17 +103,28 @@ public void testDoExecuteForRemoteServerNodes() { List.of() ); - doAnswer(invocation -> { - final NodesInfoRequest nodesInfoRequest = invocation.getArgument(2); - assertThat(nodesInfoRequest.requestedMetrics(), containsInAnyOrder(NodesInfoMetrics.Metric.REMOTE_CLUSTER_SERVER.metricName())); - final ActionListenerResponseHandler handler = invocation.getArgument(3); - handler.handleResponse(nodesInfoResponse); - return null; - }).when(transportService).sendRequest(eq(localNode), eq(NodesInfoAction.NAME), any(NodesInfoRequest.class), any()); - final RemoteClusterNodesAction.TransportAction action = new RemoteClusterNodesAction.TransportAction( - transportService, - mock(ActionFilters.class) + mock(TransportService.class), + new ActionFilters(Set.of()), + new AbstractClient(Settings.EMPTY, threadPool) { + @SuppressWarnings("unchecked") + @Override + protected void doExecute( + ActionType action, + Request request, + ActionListener listener + ) { + assertSame(NodesInfoAction.INSTANCE, action); + assertThat( + asInstanceOf(NodesInfoRequest.class, request).requestedMetrics(), + containsInAnyOrder(NodesInfoMetrics.Metric.REMOTE_CLUSTER_SERVER.metricName()) + ); + listener.onResponse((Response) nodesInfoResponse); + } + + @Override + public void close() {} + } ); final PlainActionFuture future = new PlainActionFuture<>(); @@ -136,11 +143,6 @@ public void testDoExecuteForRemoteNodes() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); when(threadPool.getThreadContext()).thenReturn(threadContext); - final TransportService transportService = mock(TransportService.class); - final DiscoveryNode localNode = mock(DiscoveryNode.class); - when(transportService.getLocalNode()).thenReturn(localNode); - when(transportService.getThreadPool()).thenReturn(threadPool); - // Prepare nodesInfo response final int numberOfNodes = randomIntBetween(1, 6); final List nodeInfos = new ArrayList<>(); @@ -178,17 +180,25 @@ public void testDoExecuteForRemoteNodes() { List.of() ); - doAnswer(invocation -> { - final NodesInfoRequest nodesInfoRequest = invocation.getArgument(2); - assertThat(nodesInfoRequest.requestedMetrics(), empty()); - final ActionListenerResponseHandler handler = invocation.getArgument(3); - handler.handleResponse(nodesInfoResponse); - return null; - }).when(transportService).sendRequest(eq(localNode), eq(NodesInfoAction.NAME), any(NodesInfoRequest.class), any()); - final RemoteClusterNodesAction.TransportAction action = new RemoteClusterNodesAction.TransportAction( - transportService, - mock(ActionFilters.class) + mock(TransportService.class), + new ActionFilters(Set.of()), + new AbstractClient(Settings.EMPTY, threadPool) { + @SuppressWarnings("unchecked") + @Override + protected void doExecute( + ActionType action, + Request request, + ActionListener listener + ) { + assertSame(NodesInfoAction.INSTANCE, action); + assertThat(asInstanceOf(NodesInfoRequest.class, request).requestedMetrics(), empty()); + listener.onResponse((Response) nodesInfoResponse); + } + + @Override + public void close() {} + } ); final PlainActionFuture future = new PlainActionFuture<>(); From 42cf90f67c7312e653345f43da4df32c36da5f27 Mon Sep 17 00:00:00 2001 From: Julia Bardi <90178898+juliaElastic@users.noreply.github.com> Date: Mon, 16 Oct 2023 09:30:50 +0200 Subject: [PATCH 05/20] using all privileges (#100764) --- .../authz/store/KibanaOwnedReservedRoleDescriptors.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java index dcd7e106b2e81..c5fc72bfeb2ab 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java @@ -194,10 +194,7 @@ static RoleDescriptor kibanaSystem(String name) { // Fleet telemetry queries Agent Logs indices in kibana task runner RoleDescriptor.IndicesPrivileges.builder().indices("logs-elastic_agent*").privileges("read").build(), // Fleet publishes Agent metrics in kibana task runner - RoleDescriptor.IndicesPrivileges.builder() - .indices("metrics-fleet_server*") - .privileges("auto_configure", "read", "write", "delete") - .build(), + RoleDescriptor.IndicesPrivileges.builder().indices("metrics-fleet_server*").privileges("all").build(), // Legacy "Alerts as data" used in Security Solution. // Kibana user creates these indices; reads / writes to them. RoleDescriptor.IndicesPrivileges.builder().indices(ReservedRolesStore.ALERTS_LEGACY_INDEX).privileges("all").build(), From 5fe7e0324875e1a3a2cd6ace475a04cf5ecb1ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Mon, 16 Oct 2023 09:34:16 +0200 Subject: [PATCH 06/20] Making yaml tests version selector parser compatible with versions returned by Build (#100794) --- .../test/rest/yaml/section/DoSection.java | 41 +++++++++++++++---- .../rest/yaml/section/DoSectionTests.java | 35 ++++++++++++++-- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 0220c0931bca1..1b1f9f7996340 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.client.HasAttributeNodeSelector; import org.elasticsearch.client.Node; @@ -38,6 +39,7 @@ import java.util.Objects; import java.util.Set; import java.util.TreeMap; +import java.util.function.Predicate; import java.util.regex.Pattern; import static java.util.Collections.emptyList; @@ -626,24 +628,47 @@ public String toString() { return result; } + private static boolean matchWithRange(String nodeVersionString, List acceptedVersionRanges, XContentLocation location) { + var unqualifiedNodeVersionString = nodeVersionString.replace("-SNAPSHOT", ""); + try { + Version version = Version.fromString(unqualifiedNodeVersionString); + return acceptedVersionRanges.stream().anyMatch(v -> v.contains(version)); + } catch (IllegalArgumentException e) { + throw new XContentParseException( + location, + "[version] range node selector expects a semantic version format (x.y.z), but found " + unqualifiedNodeVersionString, + e + ); + } + } + private static NodeSelector parseVersionSelector(XContentParser parser) throws IOException { if (false == parser.currentToken().isValue()) { throw new XContentParseException(parser.getTokenLocation(), "expected [version] to be a value"); } - List skipVersionRanges = parser.text().equals("current") - ? List.of(new VersionRange(Version.CURRENT, Version.CURRENT)) - : SkipSection.parseVersionRanges(parser.text()); + + final Predicate nodeMatcher; + final String versionSelectorString; + if (parser.text().equals("current")) { + var currentUnqualified = Build.current().version().replace("-SNAPSHOT", ""); + nodeMatcher = nodeVersion -> currentUnqualified.equals(nodeVersion.replace("-SNAPSHOT", "")); + versionSelectorString = "version is " + currentUnqualified + " (current)"; + } else { + var acceptedVersionRange = SkipSection.parseVersionRanges(parser.text()); + nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation()); + versionSelectorString = "version ranges " + acceptedVersionRange; + } + return new NodeSelector() { @Override public void select(Iterable nodes) { for (Iterator itr = nodes.iterator(); itr.hasNext();) { Node node = itr.next(); - if (node.getVersion() == null) { + String versionString = node.getVersion(); + if (versionString == null) { throw new IllegalStateException("expected [version] metadata to be set but got " + node); } - Version version = Version.fromString(node.getVersion()); - boolean skip = skipVersionRanges.stream().anyMatch(v -> v.contains(version)); - if (false == skip) { + if (nodeMatcher.test(versionString) == false) { itr.remove(); } } @@ -651,7 +676,7 @@ public void select(Iterable nodes) { @Override public String toString() { - return "version ranges " + skipVersionRanges; + return versionSelectorString; } }; } diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index 88c5fdfdb1e78..501f83bb02e1f 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.test.rest.yaml.section; import org.apache.http.HttpHost; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; @@ -19,6 +20,7 @@ import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; import org.elasticsearch.xcontent.XContentLocation; +import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.yaml.YamlXContent; import org.hamcrest.MatcherAssert; @@ -36,6 +38,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -576,7 +579,7 @@ public void testParseDoSectionAllowedWarnings() throws Exception { assertThat(e.getMessage(), equalTo("the warning [foo] was both allowed and expected")); } - public void testNodeSelectorByVersion() throws IOException { + public void testNodeSelectorByVersionRange() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: version: 5.2.0-6.0.0 @@ -626,6 +629,28 @@ public void testNodeSelectorByVersion() throws IOException { } } + public void testNodeSelectorByVersionRangeFailsWithNonSemanticVersion() throws IOException { + parser = createParser(YamlXContent.yamlXContent, """ + node_selector: + version: 5.2.0-6.0.0 + indices.get_field_mapping: + index: test_index"""); + + DoSection doSection = DoSection.parse(parser); + assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); + Node nonSemantic = nodeWithVersion("abddef"); + List nodes = new ArrayList<>(); + + var exception = expectThrows( + XContentParseException.class, + () -> doSection.getApiCallSection().getNodeSelector().select(List.of(nonSemantic)) + ); + assertThat( + exception.getMessage(), + endsWith("[version] range node selector expects a semantic version format (x.y.z), but found abddef") + ); + } + public void testNodeSelectorCurrentVersion() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: @@ -638,14 +663,16 @@ public void testNodeSelectorCurrentVersion() throws IOException { Node v170 = nodeWithVersion("1.7.0"); Node v521 = nodeWithVersion("5.2.1"); Node v550 = nodeWithVersion("5.5.0"); - Node current = nodeWithVersion(Version.CURRENT.toString()); + Node oldCurrent = nodeWithVersion(Version.CURRENT.toString()); + Node newCurrent = nodeWithVersion(Build.current().version()); List nodes = new ArrayList<>(); nodes.add(v170); nodes.add(v521); nodes.add(v550); - nodes.add(current); + nodes.add(oldCurrent); + nodes.add(newCurrent); doSection.getApiCallSection().getNodeSelector().select(nodes); - assertEquals(List.of(current), nodes); + assertEquals(List.of(oldCurrent, newCurrent), nodes); } private static Node nodeWithVersion(String version) { From baff9ae3611d4e4a028313966dddd89951d7bd6a Mon Sep 17 00:00:00 2001 From: Kostas Krikellas Date: Mon, 16 Oct 2023 11:16:53 +0300 Subject: [PATCH 07/20] Assert that both time-series indexes are created --- .../rest-api-spec/test/field_caps/40_time_series.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml index 2f99a11cce98a..3d15678544910 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml @@ -176,6 +176,10 @@ setup: index: tsdb_index1,tsdb_index2 fields: [ "metricset", "non_tsdb_field", "k8s.pod.*" ] + - length: {indices: 2} + - match: {indices.0: tsdb_index1} + - match: {indices.1: tsdb_index2} + - match: {fields.metricset.keyword.searchable: true} - match: {fields.metricset.keyword.aggregatable: true} - is_false: fields.metricset.keyword.time_series_dimension From 3ccbb001e8a1293fa12769f124aacb48a3efbff0 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Mon, 16 Oct 2023 09:34:58 +0100 Subject: [PATCH 08/20] [Transform] Check for internal index searchability as well as active primary (#100851) Currently, before performing operations that require the transform internal index be available we check whether its primary shard is active. In stateless Elasticsearch we need to separately check whether the index is searchable, as search and indexing shards are separate. --- .../xpack/transform/persistence/TransformInternalIndex.java | 2 +- .../transforms/TransformPersistentTasksExecutor.java | 4 +++- .../transform/persistence/TransformInternalIndexTests.java | 5 +++-- .../transforms/TransformPersistentTasksExecutorTests.java | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndex.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndex.java index c580417c578fe..ae890cb8321dc 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndex.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndex.java @@ -385,7 +385,7 @@ protected static boolean hasLatestVersionedIndex(ClusterState state) { protected static boolean allPrimaryShardsActiveForLatestVersionedIndex(ClusterState state) { IndexRoutingTable indexRouting = state.routingTable().index(TransformInternalIndexConstants.LATEST_INDEX_VERSIONED_NAME); - return indexRouting != null && indexRouting.allPrimaryShardsActive(); + return indexRouting != null && indexRouting.allPrimaryShardsActive() && indexRouting.readyForSearch(state); } private static void waitForLatestVersionedIndexShardsActive(Client client, ActionListener listener) { diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java index aa956e47ad49a..0d2ce26363298 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java @@ -162,7 +162,9 @@ static List verifyIndicesPrimaryShardsAreActive(ClusterState clusterStat List unavailableIndices = new ArrayList<>(indices.length); for (String index : indices) { IndexRoutingTable routingTable = clusterState.getRoutingTable().index(index); - if (routingTable == null || routingTable.allPrimaryShardsActive() == false) { + if (routingTable == null + || routingTable.allPrimaryShardsActive() == false + || routingTable.readyForSearch(clusterState) == false) { unavailableIndices.add(index); } } diff --git a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndexTests.java b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndexTests.java index cdfdaa546ace6..6b3ec86102ca7 100644 --- a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndexTests.java +++ b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/persistence/TransformInternalIndexTests.java @@ -63,11 +63,12 @@ public static ClusterState randomTransformClusterState() { } public static ClusterState randomTransformClusterState(boolean shardsReady) { + String uuid = UUIDs.randomBase64UUID(); Map indexMapBuilder = new HashMap<>(); try { IndexMetadata.Builder builder = new IndexMetadata.Builder(TransformInternalIndexConstants.LATEST_INDEX_VERSIONED_NAME).settings( Settings.builder() - .put(TransformInternalIndex.settings(Settings.EMPTY)) + .put(TransformInternalIndex.settings(Settings.builder().put(IndexMetadata.SETTING_INDEX_UUID, uuid).build())) .put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), IndexVersion.current()) .build() ).numberOfReplicas(0).numberOfShards(1).putMapping(Strings.toString(TransformInternalIndex.mappings())); @@ -80,7 +81,7 @@ public static ClusterState randomTransformClusterState(boolean shardsReady) { ClusterState.Builder csBuilder = ClusterState.builder(ClusterName.DEFAULT); csBuilder.metadata(metaBuilder.build()); - final var index = new Index(TransformInternalIndexConstants.LATEST_INDEX_VERSIONED_NAME, UUIDs.randomBase64UUID()); + final var index = new Index(TransformInternalIndexConstants.LATEST_INDEX_VERSIONED_NAME, uuid); csBuilder.routingTable( RoutingTable.builder() .add( diff --git a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java index 69f4a66b53f7c..b1582970d4e07 100644 --- a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java +++ b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java @@ -266,7 +266,7 @@ private void addIndices(Metadata.Builder metadata, RoutingTable.Builder routingT indices.add(TransformInternalIndexConstants.LATEST_INDEX_NAME); for (String indexName : indices) { IndexMetadata.Builder indexMetadata = IndexMetadata.builder(indexName); - indexMetadata.settings(indexSettings(IndexVersion.current(), 1, 0)); + indexMetadata.settings(indexSettings(IndexVersion.current(), 1, 0).put(IndexMetadata.SETTING_INDEX_UUID, "_uuid")); metadata.put(indexMetadata); Index index = new Index(indexName, "_uuid"); ShardId shardId = new ShardId(index, 0); From 43a416752826d43095293ae4371a26970ff0f083 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Mon, 16 Oct 2023 10:02:30 +0100 Subject: [PATCH 09/20] [ML] Check for internal index searchability as well as active primary (#100852) Currently, before performing operations that require the ML internal indices be available we check whether their primary shards are active. In stateless Elasticsearch we need to separately check whether the indices are searchable, as search and indexing shards are separate. --- .../xpack/ml/MlConfigMigrationEligibilityCheck.java | 2 +- .../xpack/ml/datafeed/DatafeedConfigAutoUpdater.java | 4 +++- .../xpack/ml/datafeed/DatafeedNodeSelector.java | 4 +++- .../xpack/ml/inference/TrainedModelStatsService.java | 4 +++- .../xpack/ml/task/AbstractJobPersistentTasksExecutor.java | 4 +++- .../xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java | 1 + .../xpack/ml/inference/TrainedModelStatsServiceTests.java | 8 ++++++++ .../ml/job/task/OpenJobPersistentTasksExecutorTests.java | 1 + .../ml/task/AbstractJobPersistentTasksExecutorTests.java | 1 + 9 files changed, 24 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java index 2c8d777e037a0..d1137069fea41 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java @@ -65,6 +65,6 @@ static boolean mlConfigIndexIsAllocated(ClusterState clusterState) { } IndexRoutingTable routingTable = clusterState.getRoutingTable().index(configIndexOrAlias.getWriteIndex()); - return routingTable != null && routingTable.allPrimaryShardsActive(); + return routingTable != null && routingTable.allPrimaryShardsActive() && routingTable.readyForSearch(clusterState); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdater.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdater.java index 3df50cf7f62ed..138eff9f49d01 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdater.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdater.java @@ -58,7 +58,9 @@ public boolean isAbleToRun(ClusterState latestState) { continue; } IndexRoutingTable routingTable = latestState.getRoutingTable().index(index); - if (routingTable == null || routingTable.allPrimaryShardsActive() == false) { + if (routingTable == null + || routingTable.allPrimaryShardsActive() == false + || routingTable.readyForSearch(latestState) == false) { return false; } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedNodeSelector.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedNodeSelector.java index 636457b0de970..31add7b37ac5f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedNodeSelector.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedNodeSelector.java @@ -206,7 +206,9 @@ private AssignmentFailure verifyIndicesActive() { for (String concreteIndex : concreteIndices) { IndexRoutingTable routingTable = clusterState.getRoutingTable().index(concreteIndex); - if (routingTable == null || routingTable.allPrimaryShardsActive() == false) { + if (routingTable == null + || routingTable.allPrimaryShardsActive() == false + || routingTable.readyForSearch(clusterState) == false) { return new AssignmentFailure( "cannot start datafeed [" + datafeedId diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java index 9c3a157e0de13..9fc97ff234c58 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java @@ -241,7 +241,9 @@ static boolean verifyIndicesExistAndPrimaryShardsAreActive(ClusterState clusterS return false; } IndexRoutingTable routingTable = clusterState.getRoutingTable().index(index); - if (routingTable == null || routingTable.allPrimaryShardsActive() == false) { + if (routingTable == null + || routingTable.allPrimaryShardsActive() == false + || routingTable.readyForSearch(clusterState) == false) { return false; } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutor.java index a9f55ee5f5960..becb1ac25fc5f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutor.java @@ -66,7 +66,9 @@ public static List verifyIndicesPrimaryShardsAreActive( continue; } IndexRoutingTable routingTable = clusterState.getRoutingTable().index(index); - if (routingTable == null || routingTable.allPrimaryShardsActive() == false) { + if (routingTable == null + || routingTable.allPrimaryShardsActive() == false + || routingTable.readyForSearch(clusterState) == false) { unavailableIndices.add(index); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java index ccb163edb6939..bf6e63faeb6cf 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedConfigAutoUpdaterTests.java @@ -169,6 +169,7 @@ public void testIsAbleToRun() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); metadata.put(indexMetadata); Index index = new Index(MlConfigIndex.indexName(), "_uuid"); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsServiceTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsServiceTests.java index c9dcc15bc0963..a95c8a92d93ee 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsServiceTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsServiceTests.java @@ -71,6 +71,7 @@ public void testVerifyIndicesExistAndPrimaryShardsAreActive() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); metadata.put(indexMetadata); addToRoutingTable(concreteIndex, routingTable); @@ -91,6 +92,7 @@ public void testVerifyIndicesExistAndPrimaryShardsAreActive() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); metadata.put(indexMetadata); addToRoutingTable(concreteIndex, routingTable); @@ -111,6 +113,7 @@ public void testVerifyIndicesExistAndPrimaryShardsAreActive() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); metadata.put(indexMetadata); addToRoutingTable(concreteIndex, routingTable); @@ -174,6 +177,7 @@ public void testUpdateStatsUpgradeMode() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); Metadata.Builder metadata = Metadata.builder().put(indexMetadata); @@ -201,6 +205,7 @@ public void testUpdateStatsUpgradeMode() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); // now set the upgrade mode @@ -232,6 +237,7 @@ public void testUpdateStatsUpgradeMode() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); Metadata.Builder metadata = Metadata.builder() @@ -288,6 +294,7 @@ public void testUpdateStatsResetMode() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); // now set the upgrade mode @@ -319,6 +326,7 @@ public void testUpdateStatsResetMode() { .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); Metadata.Builder metadata = Metadata.builder() diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutorTests.java index 57af1701fbb37..0b563a8a08107 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/task/OpenJobPersistentTasksExecutorTests.java @@ -257,6 +257,7 @@ private void addIndices(Metadata.Builder metadata, RoutingTable.Builder routingT .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); if (indexName.equals(AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX)) { indexMetadata.putAlias(new AliasMetadata.Builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias())); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutorTests.java index bc75f0ca82898..98169d1aa6f5b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/task/AbstractJobPersistentTasksExecutorTests.java @@ -123,6 +123,7 @@ private void addIndices(Metadata.Builder metadata, RoutingTable.Builder routingT .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_INDEX_UUID, "_uuid") ); if (indexName.equals(AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX)) { indexMetadata.putAlias(new AliasMetadata.Builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias())); From edab22a31c2ba8b88938b5db22102cb46fe20047 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 16 Oct 2023 10:11:10 +0100 Subject: [PATCH 10/20] Consistent scores for multi-term SourceConfirmedTestQuery (#100846) SourceConfirmedTestQuery uses a QueryVisitor to collect terms from its inner query to build its internal SimScorer. It is important to hold these terms in a consistent order so that when scores for each term are summed, the order of summation is the same as it would be for the inner query. This commit changes the call to visit to use a LinkedHashSet to ensure that terms are iterated in the order in which they are collected. Fixes #98712 --- docs/changelog/100846.yaml | 6 ++++++ .../index/mapper/extras/SourceConfirmedTextQuery.java | 6 ++++-- .../index/mapper/extras/SourceConfirmedTextQueryTests.java | 1 - 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/100846.yaml diff --git a/docs/changelog/100846.yaml b/docs/changelog/100846.yaml new file mode 100644 index 0000000000000..d13fb78b697a2 --- /dev/null +++ b/docs/changelog/100846.yaml @@ -0,0 +1,6 @@ +pr: 100846 +summary: Consistent scores for multi-term `SourceConfirmedTestQuery` +area: Search +type: bug +issues: + - 98712 diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java index 9e1efb34eccce..8cf1da77b545d 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java @@ -49,7 +49,7 @@ import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -215,7 +215,9 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo return in.createWeight(searcher, scoreMode, boost); } - final Set terms = new HashSet<>(); + // We use a LinkedHashSet here to preserve the ordering of terms to ensure that + // later summing of float scores per term is consistent + final Set terms = new LinkedHashSet<>(); in.visit(QueryVisitor.termCollector(terms)); if (terms.isEmpty()) { throw new IllegalStateException("Query " + in + " doesn't have any term"); diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java index 45dbb540c3a40..2b8d5870cb8aa 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java @@ -147,7 +147,6 @@ public void testPhrase() throws Exception { } } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/98712") public void testMultiPhrase() throws Exception { try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) { From fe9995965f9c09da5655d4e8e636a05b6ea3ccea Mon Sep 17 00:00:00 2001 From: Kostas Krikellas Date: Mon, 16 Oct 2023 12:23:27 +0300 Subject: [PATCH 11/20] Revert "Assert that both time-series indexes are created" This reverts commit baff9ae3611d4e4a028313966dddd89951d7bd6a. --- .../rest-api-spec/test/field_caps/40_time_series.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml index 3d15678544910..2f99a11cce98a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml @@ -176,10 +176,6 @@ setup: index: tsdb_index1,tsdb_index2 fields: [ "metricset", "non_tsdb_field", "k8s.pod.*" ] - - length: {indices: 2} - - match: {indices.0: tsdb_index1} - - match: {indices.1: tsdb_index2} - - match: {fields.metricset.keyword.searchable: true} - match: {fields.metricset.keyword.aggregatable: true} - is_false: fields.metricset.keyword.time_series_dimension From d01c61fbbee9a613cbea769f09c260f1d635ddb0 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 16 Oct 2023 11:07:41 +0100 Subject: [PATCH 12/20] Better failure logging in testFailsIfRegisterHoldsSpuriousValue (#100888) Relates #99422 --- .../blobstore/testkit/RepositoryAnalysisFailureIT.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/RepositoryAnalysisFailureIT.java b/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/RepositoryAnalysisFailureIT.java index ba132938b238e..251d20dbc4c4f 100644 --- a/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/RepositoryAnalysisFailureIT.java +++ b/x-pack/plugin/snapshot-repo-test-kit/src/internalClusterTest/java/org/elasticsearch/repositories/blobstore/testkit/RepositoryAnalysisFailureIT.java @@ -341,7 +341,9 @@ public BytesReference onCompareAndExchange(BytesRegister register, BytesReferenc analyseRepository(request); assertFalse(sawSpuriousValue.get()); } catch (RepositoryVerificationException e) { - assertTrue(sawSpuriousValue.get()); + if (sawSpuriousValue.get() == false) { + fail(e, "did not see spurious value, so why did the verification fail?"); + } } } From 83abb37f54e2947aea0552acfd770a9d14d5ca02 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 16 Oct 2023 11:10:04 +0100 Subject: [PATCH 13/20] [ML] Use correct writable name for model assignment metadata in mixed cluster (#100886) Older nodes will fail if they do not recognise the named writable --- docs/changelog/100886.yaml | 5 +++++ .../inference/assignment/TrainedModelAssignmentMetadata.java | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/100886.yaml diff --git a/docs/changelog/100886.yaml b/docs/changelog/100886.yaml new file mode 100644 index 0000000000000..b926f924c7a7c --- /dev/null +++ b/docs/changelog/100886.yaml @@ -0,0 +1,5 @@ +pr: 100886 +summary: Use the correct writable name for model assignment metadata in mixed version clusters. Prevents a node failure due to IllegalArgumentException Unknown NamedWriteable [trained_model_assignment] +area: Machine Learning +type: bug +issues: [] diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentMetadata.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentMetadata.java index 8391d287a6847..aabedfc4351b5 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentMetadata.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentMetadata.java @@ -309,7 +309,8 @@ private TrainedModeAssignmentDiff(final StreamInput in, String writeableName) th @Override public Metadata.Custom apply(Metadata.Custom part) { return new TrainedModelAssignmentMetadata( - new TreeMap<>(modelRoutingEntries.apply(((TrainedModelAssignmentMetadata) part).deploymentRoutingEntries)) + new TreeMap<>(modelRoutingEntries.apply(((TrainedModelAssignmentMetadata) part).deploymentRoutingEntries)), + writeableName ); } From 3247accddf6e6eeb8816f1870305f7db8f9a12fd Mon Sep 17 00:00:00 2001 From: Kostas Krikellas <131142368+kkrik-es@users.noreply.github.com> Date: Mon, 16 Oct 2023 13:10:47 +0300 Subject: [PATCH 14/20] [TEST] Assert that both time-series indexes are created (#100885) * Assert that both time-series indexes are created * Exclude from 8.7-8.10 mixedClusterTests * Restore asserts * Fix assert --- .../rest-api-spec/test/field_caps/40_time_series.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml index 2f99a11cce98a..e0d4b19a1a228 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/field_caps/40_time_series.yml @@ -1,8 +1,8 @@ --- setup: - skip: - version: " - 8.0.99" - reason: introduced in 8.1.0 + version: " - 8.0.99, 8.7.00 - 8.9.99" + reason: introduced in 8.1.0, synthetic source shows up in the mapping in 8.10 and on, may trigger assert failures in mixed cluster tests - do: indices.create: @@ -176,6 +176,8 @@ setup: index: tsdb_index1,tsdb_index2 fields: [ "metricset", "non_tsdb_field", "k8s.pod.*" ] + - length: {indices: 2} + - match: {fields.metricset.keyword.searchable: true} - match: {fields.metricset.keyword.aggregatable: true} - is_false: fields.metricset.keyword.time_series_dimension From a82f0ac7b098a7838aabe395640ba9e8c4f549f7 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Mon, 16 Oct 2023 13:14:28 +0200 Subject: [PATCH 15/20] Add runtime field of type geo_shape (#100492) This commit adds the possibility to create runtime fields of type geo-shape. In order to create them, users can define an emit function that takes either a geojson object or a WKT string that internally creates a geometry object. --- docs/changelog/100492.yaml | 6 + .../action/PainlessExecuteAction.java | 21 ++ ...rg.elasticsearch.script.geometry_field.txt | 20 ++ .../action/PainlessExecuteApiTests.java | 45 +++ .../AbstractShapeGeometryFieldMapper.java | 15 + .../script/GeometryFieldScript.java | 143 ++++++++ .../elasticsearch/script/ScriptModule.java | 1 + .../mapper/GeometryFieldScriptTests.java | 82 +++++ .../script/MockScriptEngine.java | 14 + .../xpack/spatial/SpatialPlugin.java | 7 + .../fielddata/GeoShapeScriptDocValues.java | 58 ++++ .../plain/GeoShapeScriptFieldData.java | 79 +++++ .../index/mapper/GeoShapeScriptFieldType.java | 166 +++++++++ .../GeoShapeWithDocValuesFieldMapper.java | 86 ++++- .../support/GeoShapeValuesSourceType.java | 4 +- .../AbstractGeoShapeScriptFieldQuery.java | 34 ++ .../GeoShapeScriptFieldExistsQuery.java | 33 ++ .../GeoShapeScriptFieldGeoShapeQuery.java | 94 ++++++ .../mapper/GeoShapeScriptFieldTypeTests.java | 314 ++++++++++++++++++ .../mapper/GeoShapeScriptMapperTests.java | 141 ++++++++ .../GeoShapeWithDocValuesFieldTypeTests.java | 6 + .../spatial/ingest/CircleProcessorTests.java | 1 + .../geogrid/GeoShapeGeoGridTestCase.java | 1 + .../GeoShapeBoundsAggregatorTests.java | 5 + .../GeoShapeCentroidAggregatorTests.java | 5 + ...tractGeoShapeScriptFieldQueryTestCase.java | 24 ++ .../GeoShapeScriptFieldExistsQueryTests.java | 43 +++ ...GeoShapeScriptFieldGeoShapeQueryTests.java | 68 ++++ .../test/spatial/130_geo_shape_runtime.yml | 237 +++++++++++++ 29 files changed, 1748 insertions(+), 5 deletions(-) create mode 100644 docs/changelog/100492.yaml create mode 100644 modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.geometry_field.txt create mode 100644 server/src/main/java/org/elasticsearch/script/GeometryFieldScript.java create mode 100644 server/src/test/java/org/elasticsearch/index/mapper/GeometryFieldScriptTests.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeScriptDocValues.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptFieldType.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/runtime/AbstractGeoShapeScriptFieldQuery.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/runtime/GeoShapeScriptFieldExistsQuery.java create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/runtime/GeoShapeScriptFieldGeoShapeQuery.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptFieldTypeTests.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptMapperTests.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/runtime/AbstractGeoShapeScriptFieldQueryTestCase.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/runtime/GeoShapeScriptFieldExistsQueryTests.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/runtime/GeoShapeScriptFieldGeoShapeQueryTests.java create mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/spatial/130_geo_shape_runtime.yml diff --git a/docs/changelog/100492.yaml b/docs/changelog/100492.yaml new file mode 100644 index 0000000000000..e0a1020b49488 --- /dev/null +++ b/docs/changelog/100492.yaml @@ -0,0 +1,6 @@ +pr: 100492 +summary: Add runtime field of type `geo_shape` +area: Geo +type: enhancement +issues: + - 61299 diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java index 783abf5551c43..d885db5ed39a9 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java @@ -50,6 +50,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Strings; import org.elasticsearch.core.Tuple; +import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; @@ -76,6 +77,7 @@ import org.elasticsearch.script.DoubleFieldScript; import org.elasticsearch.script.FilterScript; import org.elasticsearch.script.GeoPointFieldScript; +import org.elasticsearch.script.GeometryFieldScript; import org.elasticsearch.script.IpFieldScript; import org.elasticsearch.script.LongFieldScript; import org.elasticsearch.script.ScoreScript; @@ -681,6 +683,25 @@ static Response innerShardOperation(Request request, ScriptService scriptService ); return new Response(format.apply(points)); }, indexService); + } else if (scriptContext == GeometryFieldScript.CONTEXT) { + return prepareRamIndex(request, (context, leafReaderContext) -> { + GeometryFieldScript.Factory factory = scriptService.compile(request.script, GeometryFieldScript.CONTEXT); + GeometryFieldScript.LeafFactory leafFactory = factory.newFactory( + GeometryFieldScript.CONTEXT.name, + request.getScript().getParams(), + context.lookup(), + OnScriptError.FAIL + ); + GeometryFieldScript geometryFieldScript = leafFactory.newInstance(leafReaderContext); + List geometries = new ArrayList<>(); + geometryFieldScript.runForDoc(0, geometries::add); + // convert geometries to the standard format of the fields api + Function, List> format = GeometryFormatterFactory.getFormatter( + GeometryFormatterFactory.GEOJSON, + Function.identity() + ); + return new Response(format.apply(geometries)); + }, indexService); } else if (scriptContext == IpFieldScript.CONTEXT) { return prepareRamIndex(request, (context, leafReaderContext) -> { IpFieldScript.Factory factory = scriptService.compile(request.script, IpFieldScript.CONTEXT); diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.geometry_field.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.geometry_field.txt new file mode 100644 index 0000000000000..68bcbf922e869 --- /dev/null +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.geometry_field.txt @@ -0,0 +1,20 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License +# 2.0 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. +# + +# The whitelist for runtime fields that generate geometries + +# These two whitelists are required for painless to find the classes +class org.elasticsearch.script.GeometryFieldScript @no_import { +} +class org.elasticsearch.script.GeometryFieldScript$Factory @no_import { +} + +static_import { + # The `emit` callback to collect values for the field + void emit(org.elasticsearch.script.GeometryFieldScript, Object) bound_to org.elasticsearch.script.GeometryFieldScript$Emit +} diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteApiTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteApiTests.java index 86f2b5d83ca0b..3fcddc6c2c895 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteApiTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteApiTests.java @@ -259,6 +259,51 @@ public void testGeoPointFieldExecutionContext() throws IOException { assertEquals("Point", points.get(1).get("type")); } + @SuppressWarnings("unchecked") + public void testGeometryFieldExecutionContext() throws IOException { + ScriptService scriptService = getInstanceFromNode(ScriptService.class); + IndexService indexService = createIndex("index", Settings.EMPTY, "doc", "test_point", "type=geo_point"); + + Request.ContextSetup contextSetup = new Request.ContextSetup( + "index", + new BytesArray("{\"test_point\":\"30.0,40.0\"}"), + new MatchAllQueryBuilder() + ); + contextSetup.setXContentType(XContentType.JSON); + Request request = new Request( + new Script( + ScriptType.INLINE, + "painless", + "emit(\"Point(\" + doc['test_point'].value.lon + \" \" + doc['test_point'].value.lat + \")\")", + emptyMap() + ), + "geometry_field", + contextSetup + ); + Response response = innerShardOperation(request, scriptService, indexService); + List> geometry = (List>) response.getResult(); + assertEquals(40.0, (double) ((List) geometry.get(0).get("coordinates")).get(0), 0.00001); + assertEquals(30.0, (double) ((List) geometry.get(0).get("coordinates")).get(1), 0.00001); + assertEquals("Point", geometry.get(0).get("type")); + + contextSetup = new Request.ContextSetup("index", new BytesArray("{}"), new MatchAllQueryBuilder()); + contextSetup.setXContentType(XContentType.JSON); + request = new Request( + new Script( + ScriptType.INLINE, + "painless", + "emit(\"LINESTRING(78.96 12.12, 12.12 78.96)\"); emit(\"POINT(13.45 56.78)\");", + emptyMap() + ), + "geometry_field", + contextSetup + ); + response = innerShardOperation(request, scriptService, indexService); + geometry = (List>) response.getResult(); + assertEquals("GeometryCollection", geometry.get(0).get("type")); + assertEquals(2, ((List) geometry.get(0).get("geometries")).size()); + } + public void testIpFieldExecutionContext() throws IOException { ScriptService scriptService = getInstanceFromNode(ScriptService.class); IndexService indexService = createIndex("index", Settings.EMPTY, "doc", "test_ip", "type=ip"); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java index 43032c9ce32c3..22b75c8262193 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java @@ -83,6 +83,21 @@ protected AbstractShapeGeometryFieldMapper( this.orientation = orientation; } + protected AbstractShapeGeometryFieldMapper( + String simpleName, + MappedFieldType mappedFieldType, + MultiFields multiFields, + Explicit coerce, + Explicit orientation, + CopyTo copyTo, + Parser parser, + OnScriptError onScriptError + ) { + super(simpleName, mappedFieldType, multiFields, copyTo, parser, onScriptError); + this.coerce = coerce; + this.orientation = orientation; + } + public boolean coerce() { return coerce.value(); } diff --git a/server/src/main/java/org/elasticsearch/script/GeometryFieldScript.java b/server/src/main/java/org/elasticsearch/script/GeometryFieldScript.java new file mode 100644 index 0000000000000..8582d7521ad5e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/script/GeometryFieldScript.java @@ -0,0 +1,143 @@ +/* + * 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.script; + +import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.common.geo.GeometryParser; +import org.elasticsearch.common.geo.Orientation; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.GeometryCollection; +import org.elasticsearch.index.mapper.OnScriptError; +import org.elasticsearch.search.lookup.SearchLookup; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Function; + +/** + * Script producing geometries. It generates a unique {@link Geometry} for each document. + */ +public abstract class GeometryFieldScript extends AbstractFieldScript { + public static final ScriptContext CONTEXT = newContext("geometry_field", Factory.class); + + public static final Factory PARSE_FROM_SOURCE = new Factory() { + @Override + public LeafFactory newFactory(String field, Map params, SearchLookup lookup, OnScriptError onScriptError) { + return ctx -> new GeometryFieldScript(field, params, lookup, OnScriptError.FAIL, ctx) { + @Override + public void execute() { + emitFromSource(); + } + }; + } + + @Override + public boolean isResultDeterministic() { + return true; + } + }; + + public static Factory leafAdapter(Function parentFactory) { + return (leafFieldName, params, searchLookup, onScriptError) -> { + CompositeFieldScript.LeafFactory parentLeafFactory = parentFactory.apply(searchLookup); + return (LeafFactory) ctx -> { + CompositeFieldScript compositeFieldScript = parentLeafFactory.newInstance(ctx); + return new GeometryFieldScript(leafFieldName, params, searchLookup, onScriptError, ctx) { + @Override + public void setDocument(int docId) { + compositeFieldScript.setDocument(docId); + } + + @Override + public void execute() { + emitFromCompositeScript(compositeFieldScript); + } + }; + }; + }; + } + + @SuppressWarnings("unused") + public static final String[] PARAMETERS = {}; + + public interface Factory extends ScriptFactory { + LeafFactory newFactory(String fieldName, Map params, SearchLookup searchLookup, OnScriptError onScriptError); + } + + public interface LeafFactory { + GeometryFieldScript newInstance(LeafReaderContext ctx); + } + + private final List geometries = new ArrayList<>(); + + private final GeometryParser geometryParser; + + public GeometryFieldScript( + String fieldName, + Map params, + SearchLookup searchLookup, + OnScriptError onScriptError, + LeafReaderContext ctx + ) { + super(fieldName, params, searchLookup, ctx, onScriptError); + geometryParser = new GeometryParser(Orientation.CCW.getAsBoolean(), false, true); + } + + @Override + protected void prepareExecute() { + geometries.clear(); + } + + /** + * Execute the script for the provided {@code docId}, passing results to the {@code consumer} + */ + public final void runForDoc(int docId, Consumer consumer) { + runForDoc(docId); + consumer.accept(geometry()); + } + + /** + * {@link Geometry} from the last time {@link #runForDoc(int)} was called. + */ + public final Geometry geometry() { + if (geometries.isEmpty()) { + return null; + } + return geometries.size() == 1 ? geometries.get(0) : new GeometryCollection<>(geometries); + } + + /** + * The number of results produced the last time {@link #runForDoc(int)} was called. It is 1 if + * the document exists, otherwise 0. + */ + public final int count() { + // Note that emitting multiple geometries gets handled by a GeometryCollection + return geometries.isEmpty() ? 0 : 1; + } + + @Override + protected void emitFromObject(Object value) { + geometries.add(geometryParser.parseGeometry(value)); + } + + public static class Emit { + private final GeometryFieldScript script; + + public Emit(GeometryFieldScript script) { + this.script = script; + } + + public void emit(Object object) { + script.checkMaxSize(script.geometries.size()); + script.emitFromObject(object); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/script/ScriptModule.java b/server/src/main/java/org/elasticsearch/script/ScriptModule.java index 85afb96225254..6eb3bdfba32fd 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptModule.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptModule.java @@ -34,6 +34,7 @@ public class ScriptModule { LongFieldScript.CONTEXT, StringFieldScript.CONTEXT, GeoPointFieldScript.CONTEXT, + GeometryFieldScript.CONTEXT, IpFieldScript.CONTEXT, CompositeFieldScript.CONTEXT ); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeometryFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeometryFieldScriptTests.java new file mode 100644 index 0000000000000..ab87ca40ce93a --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeometryFieldScriptTests.java @@ -0,0 +1,82 @@ +/* + * 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.index.DirectoryReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.index.RandomIndexWriter; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.script.AbstractFieldScript; +import org.elasticsearch.script.GeometryFieldScript; +import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.search.lookup.SearchLookup; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class GeometryFieldScriptTests extends FieldScriptTestCase { + public static final GeometryFieldScript.Factory DUMMY = (fieldName, params, lookup, onScriptError) -> ctx -> new GeometryFieldScript( + fieldName, + params, + lookup, + OnScriptError.FAIL, + ctx + ) { + @Override + public void execute() { + emitFromObject("POINT(0 0)"); + } + }; + + @Override + protected ScriptContext context() { + return GeometryFieldScript.CONTEXT; + } + + @Override + protected GeometryFieldScript.Factory dummyScript() { + return DUMMY; + } + + @Override + protected GeometryFieldScript.Factory fromSource() { + return GeometryFieldScript.PARSE_FROM_SOURCE; + } + + public void testTooManyValues() throws IOException { + try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) { + iw.addDocument(List.of(new StoredField("_source", new BytesRef("{}")))); + try (DirectoryReader reader = iw.getReader()) { + GeometryFieldScript script = new GeometryFieldScript( + "test", + Map.of(), + new SearchLookup(field -> null, (ft, lookup, fdt) -> null, (ctx, doc) -> null), + OnScriptError.FAIL, + reader.leaves().get(0) + ) { + @Override + public void execute() { + for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) { + new Emit(this).emit("POINT(0 0)"); + } + } + }; + Exception e = expectThrows(IllegalArgumentException.class, script::execute); + assertThat( + e.getMessage(), + equalTo("Runtime field [test] is emitting [101] values while the maximum number of values allowed is [100]") + ); + } + } + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 130eca43e7a33..5a607350d913f 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -377,6 +377,20 @@ public void execute() { } }; return context.factoryClazz.cast(objectFieldScript); + } else if (context.instanceClazz.equals(GeometryFieldScript.class)) { + GeometryFieldScript.Factory geometryFieldScript = (f, p, s, onScriptError) -> ctx -> new GeometryFieldScript( + f, + p, + s, + OnScriptError.FAIL, + ctx + ) { + @Override + public void execute() { + emitFromObject("POINT(1.2 1.2)"); + } + }; + return context.factoryClazz.cast(geometryFieldScript); } else if (context.instanceClazz.equals(DoubleValuesScript.class)) { DoubleValuesScript.Factory doubleValuesScript = () -> new MockDoubleValuesScript(); return context.factoryClazz.cast(doubleValuesScript); diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java index 66281cd21856b..7f171230e7628 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.geo.GeoFormatterFactory; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.index.mapper.RuntimeField; import org.elasticsearch.ingest.Processor; import org.elasticsearch.license.License; import org.elasticsearch.license.LicenseUtils; @@ -45,6 +46,7 @@ import org.elasticsearch.xpack.spatial.action.SpatialStatsTransportAction; import org.elasticsearch.xpack.spatial.action.SpatialUsageTransportAction; import org.elasticsearch.xpack.spatial.common.CartesianBoundingBox; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeScriptFieldType; import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper; import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper; import org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper; @@ -142,6 +144,11 @@ public Map getMappers() { ); } + @Override + public Map getRuntimeFields() { + return Map.of(GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE, GeoShapeScriptFieldType.typeParser(geoFormatterFactory.get())); + } + @Override public List> getQueries() { return List.of( diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeScriptDocValues.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeScriptDocValues.java new file mode 100644 index 0000000000000..4840ca4034fd0 --- /dev/null +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeScriptDocValues.java @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.index.fielddata; + +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.geo.Orientation; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.index.mapper.GeoShapeIndexer; +import org.elasticsearch.script.GeometryFieldScript; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; +import org.elasticsearch.xpack.spatial.index.mapper.BinaryShapeDocValuesField; +import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType; + +import java.io.IOException; +import java.util.List; + +/** + * Similarly to what {@link BinaryShapeDocValuesField} does, it encodes the shapes using the {@link GeometryDocValueWriter}. + */ +public final class GeoShapeScriptDocValues extends GeoShapeValues { + private final GeometryFieldScript script; + private final GeoShapeValues.GeoShapeValue geoShapeValue = new GeoShapeValues.GeoShapeValue(); + private final GeoShapeIndexer indexer; + + public GeoShapeScriptDocValues(GeometryFieldScript script, String fieldName) { + this.script = script; + indexer = new GeoShapeIndexer(Orientation.CCW, fieldName); + } + + @Override + public boolean advanceExact(int docId) { + script.runForDoc(docId); + return script.count() != 0; + } + + @Override + public ValuesSourceType valuesSourceType() { + return GeoShapeValuesSourceType.instance(); + } + + @Override + public GeoShapeValue value() throws IOException { + final Geometry geometry = script.geometry(); + if (geometry == null) { + return null; + } + final List fields = indexer.getIndexableFields(geometry); + final CentroidCalculator centroidCalculator = new CentroidCalculator(); + centroidCalculator.add(geometry); + geoShapeValue.reset(GeometryDocValueWriter.write(fields, CoordinateEncoder.GEO, centroidCalculator)); + return geoShapeValue; + } +} diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java new file mode 100644 index 0000000000000..4790924412cd7 --- /dev/null +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/GeoShapeScriptFieldData.java @@ -0,0 +1,79 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.index.fielddata.plain; + +import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.index.fielddata.IndexFieldData; +import org.elasticsearch.index.fielddata.IndexFieldDataCache; +import org.elasticsearch.indices.breaker.CircuitBreakerService; +import org.elasticsearch.script.GeometryFieldScript; +import org.elasticsearch.script.field.ToScriptFieldFactory; +import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeScriptDocValues; +import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; +import org.elasticsearch.xpack.spatial.index.fielddata.LeafShapeFieldData; +import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType; + +public final class GeoShapeScriptFieldData extends AbstractShapeIndexFieldData { + public static class Builder implements IndexFieldData.Builder { + private final String name; + private final GeometryFieldScript.LeafFactory leafFactory; + private final ToScriptFieldFactory toScriptFieldFactory; + + public Builder( + String name, + GeometryFieldScript.LeafFactory leafFactory, + ToScriptFieldFactory toScriptFieldFactory + ) { + this.name = name; + this.leafFactory = leafFactory; + this.toScriptFieldFactory = toScriptFieldFactory; + } + + @Override + public GeoShapeScriptFieldData build(IndexFieldDataCache cache, CircuitBreakerService breakerService) { + return new GeoShapeScriptFieldData(name, leafFactory, toScriptFieldFactory); + } + } + + private final GeometryFieldScript.LeafFactory leafFactory; + + private GeoShapeScriptFieldData( + String fieldName, + GeometryFieldScript.LeafFactory leafFactory, + ToScriptFieldFactory toScriptFieldFactory + ) { + super(fieldName, GeoShapeValuesSourceType.instance(), toScriptFieldFactory); + this.leafFactory = leafFactory; + } + + @Override + protected IllegalArgumentException sortException() { + throw new IllegalArgumentException("can't sort on geo_shape field"); + } + + @Override + public LeafShapeFieldData load(LeafReaderContext context) { + final GeometryFieldScript script = leafFactory.newInstance(context); + return new LeafShapeFieldData<>(toScriptFieldFactory) { + @Override + public GeoShapeValues getShapeValues() { + return new GeoShapeScriptDocValues(script, fieldName); + } + + @Override + public long ramBytesUsed() { + return 0; + } + + @Override + public void close() { + + } + }; + } +} diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptFieldType.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptFieldType.java new file mode 100644 index 0000000000000..358f6768132b9 --- /dev/null +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeScriptFieldType.java @@ -0,0 +1,166 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.index.mapper; + +import org.apache.lucene.geo.LatLonGeometry; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.geo.GeoFormatterFactory; +import org.elasticsearch.common.geo.GeometryFormatterFactory; +import org.elasticsearch.common.geo.ShapeRelation; +import org.elasticsearch.common.time.DateMathParser; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.index.fielddata.FieldDataContext; +import org.elasticsearch.index.mapper.AbstractScriptFieldType; +import org.elasticsearch.index.mapper.GeoShapeQueryable; +import org.elasticsearch.index.mapper.OnScriptError; +import org.elasticsearch.index.mapper.RuntimeField; +import org.elasticsearch.index.mapper.ValueFetcher; +import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.script.CompositeFieldScript; +import org.elasticsearch.script.GeometryFieldScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.search.fetch.StoredFieldsSpec; +import org.elasticsearch.search.lookup.SearchLookup; +import org.elasticsearch.search.lookup.Source; +import org.elasticsearch.xpack.spatial.index.fielddata.plain.GeoShapeScriptFieldData; +import org.elasticsearch.xpack.spatial.search.runtime.GeoShapeScriptFieldExistsQuery; +import org.elasticsearch.xpack.spatial.search.runtime.GeoShapeScriptFieldGeoShapeQuery; + +import java.io.IOException; +import java.time.ZoneId; +import java.util.List; +import java.util.Map; +import java.util.function.Function; + +public final class GeoShapeScriptFieldType extends AbstractScriptFieldType implements GeoShapeQueryable { + + public static RuntimeField.Parser typeParser(GeoFormatterFactory geoFormatterFactory) { + return new RuntimeField.Parser(name -> new Builder<>(name, GeometryFieldScript.CONTEXT) { + @Override + protected AbstractScriptFieldType createFieldType( + String name, + GeometryFieldScript.Factory factory, + Script script, + Map meta, + OnScriptError onScriptError + ) { + return new GeoShapeScriptFieldType(name, factory, getScript(), meta(), onScriptError, geoFormatterFactory); + } + + @Override + protected GeometryFieldScript.Factory getParseFromSourceFactory() { + return GeometryFieldScript.PARSE_FROM_SOURCE; + } + + @Override + protected GeometryFieldScript.Factory getCompositeLeafFactory( + Function parentScriptFactory + ) { + return GeometryFieldScript.leafAdapter(parentScriptFactory); + } + }); + } + + private final GeoFormatterFactory geoFormatterFactory; + + GeoShapeScriptFieldType( + String name, + GeometryFieldScript.Factory scriptFactory, + Script script, + Map meta, + OnScriptError onScriptError, + GeoFormatterFactory geoFormatterFactory + ) { + super( + name, + searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup, onScriptError), + script, + scriptFactory.isResultDeterministic(), + meta + ); + this.geoFormatterFactory = geoFormatterFactory; + } + + @Override + public String typeName() { + return GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE; + } + + @Override + protected Query rangeQuery( + Object lowerTerm, + Object upperTerm, + boolean includeLower, + boolean includeUpper, + ZoneId timeZone, + DateMathParser parser, + SearchExecutionContext context + ) { + throw new IllegalArgumentException("Runtime field [" + name() + "] of type [" + typeName() + "] does not support range queries"); + } + + @Override + public Query termQuery(Object value, SearchExecutionContext context) { + throw new IllegalArgumentException( + "Geometry fields do not support exact searching, use dedicated geometry queries instead: [" + name() + "]" + ); + } + + @Override + public GeoShapeScriptFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext) { + return new GeoShapeScriptFieldData.Builder( + name(), + leafFactory(fieldDataContext.lookupSupplier().get()), + GeoShapeWithDocValuesFieldMapper.GeoShapeDocValuesField::new + ); + } + + @Override + public Query existsQuery(SearchExecutionContext context) { + applyScriptContext(context); + return new GeoShapeScriptFieldExistsQuery(script, leafFactory(context), name()); + } + + @Override + public Query geoShapeQuery(SearchExecutionContext context, String fieldName, ShapeRelation relation, LatLonGeometry... geometries) { + return new GeoShapeScriptFieldGeoShapeQuery(script, leafFactory(context), fieldName, relation, geometries); + } + + @Override + public ValueFetcher valueFetcher(SearchExecutionContext context, String format) { + GeometryFieldScript.LeafFactory leafFactory = leafFactory(context.lookup()); + + Function, List> formatter = geoFormatterFactory.getFormatter( + format != null ? format : GeometryFormatterFactory.GEOJSON, + Function.identity() + ); + return new ValueFetcher() { + private GeometryFieldScript script; + + @Override + public void setNextReader(LeafReaderContext context) { + script = leafFactory.newInstance(context); + } + + @Override + public List fetchValues(Source source, int doc, List ignoredValues) throws IOException { + script.runForDoc(doc); + if (script.count() == 0) { + return List.of(); + } + return formatter.apply(List.of(script.geometry())); + } + + @Override + public StoredFieldsSpec storedFieldsSpec() { + return StoredFieldsSpec.NEEDS_SOURCE; + } + }; + } +} diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index 13fb4246a5b3a..3e904f59ad44e 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -11,6 +11,7 @@ import org.apache.lucene.document.StoredField; import org.apache.lucene.geo.LatLonGeometry; import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; @@ -43,14 +44,20 @@ import org.elasticsearch.index.mapper.MapperBuilderContext; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.MappingParserContext; +import org.elasticsearch.index.mapper.OnScriptError; import org.elasticsearch.index.mapper.StoredValueFetcher; import org.elasticsearch.index.mapper.ValueFetcher; import org.elasticsearch.index.query.QueryShardException; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.legacygeo.mapper.LegacyGeoShapeFieldMapper; +import org.elasticsearch.script.GeometryFieldScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptCompiler; import org.elasticsearch.script.field.AbstractScriptFieldFactory; import org.elasticsearch.script.field.DocValuesScriptFieldFactory; import org.elasticsearch.script.field.Field; +import org.elasticsearch.search.lookup.FieldValues; +import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.xpack.spatial.index.fielddata.CoordinateEncoder; import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; import org.elasticsearch.xpack.spatial.index.fielddata.plain.AbstractAtomicGeoShapeShapeFieldData; @@ -110,25 +117,29 @@ public static class Builder extends FieldMapper.Builder { final Parameter> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get()); final Parameter> coerce; final Parameter> orientation = orientationParam(m -> builder(m).orientation.get()); - + private final Parameter