Merge branch 'main' into use_common_retry_logic_gcs

Author: nicktindall

modules/ingest-geoip/src/yamlRestTest/java/org/elasticsearch/ingest/geoip/IngestGeoIpClientYamlTestSuiteIT.java

@@ -38,6 +38,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
@@ -158,7 +159,9 @@ static void assertDatabasesLoaded() throws Exception {
 
             // ensure that the extra config database has been set up, too:
             assertThat(node.get("config_databases"), equalTo(List.of("asn.mmdb")));
-        });
+            // Downloading all four databases may take some time, so we set a longer timeout here.
+            // If 20 seconds prove insufficient, we should first investigate whether we can speed up the database downloader.
+        }, 20, TimeUnit.SECONDS);
     }
 
     @SuppressForbidden(reason = "fixtures use java.io.File based APIs")

muted-tests.yml

@@ -429,9 +429,6 @@ tests:
 - class: org.elasticsearch.xpack.inference.integration.AuthorizationTaskExecutorIT
   method: testCreatesEisChatCompletion_DoesNotRemoveEndpointWhenNoLongerAuthorized
   issue: https://github.com/elastic/elasticsearch/issues/138480
-- class: org.elasticsearch.ingest.geoip.IngestGeoIpClientYamlTestSuiteIT
-  method: test {yaml=ingest_geoip/60_ip_location_databases/Test adding, getting, and removing ip location databases}
-  issue: https://github.com/elastic/elasticsearch/issues/138502
 - class: org.elasticsearch.xpack.exponentialhistogram.ExponentialHistogramFieldMapperTests
   method: testFormattedDocValues
   issue: https://github.com/elastic/elasticsearch/issues/138504

server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java

@@ -64,6 +64,7 @@ public class MapperFeatures implements FeatureSpecification {
     public static final NodeFeature GENERIC_VECTOR_FORMAT = new NodeFeature("mapper.vectors.generic_vector_format");
     public static final NodeFeature FIX_DENSE_VECTOR_WRONG_FIELDS = new NodeFeature("mapper.fix_dense_vector_wrong_fields");
     static final NodeFeature BBQ_DISK_STATS_SUPPORT = new NodeFeature("mapper.bbq_disk_stats_support");
+    static final NodeFeature STORED_FIELDS_SPEC_MERGE_BUG = new NodeFeature("mapper.stored_fields_spec_merge_bug");
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
@@ -108,7 +109,8 @@ public Set<NodeFeature> getTestFeatures() {
             EXCLUDE_VECTORS_DOCVALUE_BUGFIX,
             BASE64_DENSE_VECTORS,
             FIX_DENSE_VECTOR_WRONG_FIELDS,
-            BBQ_DISK_STATS_SUPPORT
+            BBQ_DISK_STATS_SUPPORT,
+            STORED_FIELDS_SPEC_MERGE_BUG
         );
         if (ES93GenericFlatVectorsFormat.GENERIC_VECTOR_FORMAT.isEnabled()) {
             features = new HashSet<>(features);

server/src/main/java/org/elasticsearch/search/fetch/StoredFieldsSpec.java

@@ -77,17 +77,7 @@ public StoredFieldsSpec merge(StoredFieldsSpec other) {
             mergedFields = new HashSet<>(this.requiredStoredFields);
             mergedFields.addAll(other.requiredStoredFields);
         }
-        Set<String> mergedSourcePaths;
-        if (this.sourcePaths.isEmpty() == false && other.sourcePaths.isEmpty() == false) {
-            mergedSourcePaths = new HashSet<>(this.sourcePaths);
-            mergedSourcePaths.addAll(other.sourcePaths);
-        } else if (this.sourcePaths.isEmpty() == false) {
-            mergedSourcePaths = this.sourcePaths;
-        } else if (other.sourcePaths.isEmpty() == false) {
-            mergedSourcePaths = other.sourcePaths;
-        } else {
-            mergedSourcePaths = Set.of();
-        }
+        Set<String> mergedSourcePaths = mergeSourcePaths(other);
         IgnoredSourceFormat mergedFormat;
         if (this.ignoredSourceFormat == IgnoredSourceFormat.NO_IGNORED_SOURCE) {
             mergedFormat = other.ignoredSourceFormat;
@@ -114,6 +104,33 @@ public StoredFieldsSpec merge(StoredFieldsSpec other) {
         );
     }
 
+    /**
+     * Returns the unique source paths that should be loaded from source. Other source paths may be filtered out.
+     * If an empty set is returned, then all source paths need to be loaded.
+     */
+    private Set<String> mergeSourcePaths(StoredFieldsSpec other) {
+        Set<String> mergedSourcePaths;
+        if (this.sourcePaths.isEmpty() == false && other.sourcePaths.isEmpty() == false) {
+            mergedSourcePaths = new HashSet<>(this.sourcePaths);
+            mergedSourcePaths.addAll(other.sourcePaths);
+        } else if (this.sourcePaths.isEmpty() == false) {
+            if (other.requiresSource) {
+                mergedSourcePaths = Set.of();
+            } else {
+                mergedSourcePaths = this.sourcePaths;
+            }
+        } else if (other.sourcePaths.isEmpty() == false) {
+            if (this.requiresSource) {
+                mergedSourcePaths = Set.of();
+            } else {
+                mergedSourcePaths = other.sourcePaths;
+            }
+        } else {
+            mergedSourcePaths = Set.of();
+        }
+        return mergedSourcePaths;
+    }
+
     public Set<String> requiredStoredFields() {
         if (sourcePaths.isEmpty() || ignoredSourceFormat == IgnoredSourceFormat.NO_IGNORED_SOURCE) {
             return requiredStoredFields;

server/src/test/java/org/elasticsearch/search/fetch/StoredFieldsSpecTests.java

@@ -120,7 +120,7 @@ public void testMergeSourcePaths() {
 
         spec = spec.merge(
             new StoredFieldsSpec(
-                true,
+                false, // if set to true, then the new spec will require complete and source and so source paths would then be empty
                 false,
                 Set.of("other_field"),
                 IgnoredSourceFieldMapper.IgnoredSourceFormat.NO_IGNORED_SOURCE,
@@ -135,6 +135,24 @@ public void testMergeSourcePaths() {
         assertThat(spec.sourcePaths(), sameInstance(pref));
     }
 
+    public void testMergeSourcePathsRequireCompleteSource() {
+        var ignoredSourceFormat = IgnoredSourceFieldMapper.IgnoredSourceFormat.NO_IGNORED_SOURCE;
+        StoredFieldsSpec spec = new StoredFieldsSpec(true, false, Set.of(), ignoredSourceFormat, Set.of("field1", "field2"));
+        assertThat(spec.ignoredSourceFormat(), equalTo(ignoredSourceFormat));
+        assertThat(spec.requiresSource(), equalTo(true));
+        assertThat(spec.requiresMetadata(), equalTo(false));
+        assertThat(spec.requiredStoredFields(), empty());
+        assertThat(spec.sourcePaths(), containsInAnyOrder("field1", "field2"));
+
+        // Clears source paths, because this spec requires complete source (since no source paths are defined)
+        spec = spec.merge(new StoredFieldsSpec(true, false, Set.of(), ignoredSourceFormat, Set.of()));
+        assertThat(spec.ignoredSourceFormat(), equalTo(ignoredSourceFormat));
+        assertThat(spec.requiresSource(), equalTo(true));
+        assertThat(spec.requiresMetadata(), equalTo(false));
+        assertThat(spec.requiredStoredFields(), empty());
+        assertThat(spec.sourcePaths(), empty());
+    }
+
     private static SearchContext searchContext(SearchSourceBuilder sourceBuilder) {
         SearchContext sc = mock(SearchContext.class);
         when(sc.fetchSourceContext()).thenReturn(sourceBuilder.fetchSource());

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

@@ -45,6 +45,7 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -248,6 +249,9 @@ public void handle(final HttpExchange exchange) throws IOException {
                     if (isProtectOverwrite(exchange)) {
                         throw new AssertionError("If-None-Match: * header is not supported here");
                     }
+                    if (getRequiredExistingETag(exchange) != null) {
+                        throw new AssertionError("If-Match: * header is not supported here");
+                    }
 
                     var sourceBlob = blobs.get(copySource);
                     if (sourceBlob == null) {
@@ -406,8 +410,9 @@ public void handle(final HttpExchange exchange) throws IOException {
      * Update the blob contents if and only if the preconditions in the request are satisfied.
      *
      * @return {@link RestStatus#OK} if the blob contents were updated, or else a different status code to indicate the error: possibly
-     *         {@link RestStatus#CONFLICT} or {@link RestStatus#PRECONDITION_FAILED} if the object exists and the precondition requires it
-     *         not to.
+     *         {@link RestStatus#CONFLICT} in any case, but if not then either  {@link RestStatus#PRECONDITION_FAILED} if the object exists
+     *         but doesn't match the specified precondition, or {@link RestStatus#NOT_FOUND} if the object doesn't exist but is required to
+     *         do so by the precondition.
      *
      * @see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/conditional-writes.html#conditional-error-response">AWS docs</a>
      */
@@ -418,6 +423,25 @@ private RestStatus updateBlobContents(HttpExchange exchange, String path, BytesR
                 : ESTestCase.randomFrom(RestStatus.PRECONDITION_FAILED, RestStatus.CONFLICT);
         }
 
+        final var requireExistingETag = getRequiredExistingETag(exchange);
+        if (requireExistingETag != null) {
+            final var responseCode = new AtomicReference<>(RestStatus.OK);
+            blobs.compute(path, (ignoredPath, existingContents) -> {
+                if (existingContents != null && requireExistingETag.equals(getEtagFromContents(existingContents))) {
+                    return newContents;
+                }
+
+                responseCode.set(
+                    ESTestCase.randomFrom(
+                        existingContents == null ? RestStatus.NOT_FOUND : RestStatus.PRECONDITION_FAILED,
+                        RestStatus.CONFLICT
+                    )
+                );
+                return existingContents;
+            });
+            return responseCode.get();
+        }
+
         blobs.put(path, newContents);
         return RestStatus.OK;
     }
@@ -598,6 +622,9 @@ private static boolean isProtectOverwrite(final HttpExchange exchange) {
             return false;
         }
 
+        if (exchange.getRequestHeaders().get("If-Match") != null) {
+            throw new AssertionError("Handling both If-None-Match and If-Match headers is not supported");
+        }
         if (ifNoneMatch.size() != 1) {
             throw new AssertionError("multiple If-None-Match headers found: " + ifNoneMatch);
         }
@@ -609,6 +636,29 @@ private static boolean isProtectOverwrite(final HttpExchange exchange) {
         throw new AssertionError("invalid If-None-Match header: " + ifNoneMatch);
     }
 
+    @Nullable // if no If-Match header found
+    private static String getRequiredExistingETag(final HttpExchange exchange) {
+        final var ifMatch = exchange.getRequestHeaders().get("If-Match");
+
+        if (ifMatch == null) {
+            return null;
+        }
+
+        if (exchange.getRequestHeaders().get("If-None-Match") != null) {
+            throw new AssertionError("Handling both If-None-Match and If-Match headers is not supported");
+        }
+
+        final var iterator = ifMatch.iterator();
+        if (iterator.hasNext()) {
+            final var result = iterator.next();
+            if (iterator.hasNext() == false) {
+                return result;
+            }
+        }
+
+        throw new AssertionError("multiple If-Match headers found: " + ifMatch);
+    }
+
     MultipartUpload putUpload(String path) {
         final var upload = new MultipartUpload(UUIDs.randomBase64UUID(), path);
         synchronized (uploads) {

test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java

@@ -412,13 +412,29 @@ public void testExtractPartEtags() {
     }
 
     public void testPreventObjectOverwrite() {
+        ensureExactlyOneSuccess(new S3HttpHandler("bucket", "path"), null);
+    }
+
+    public void testConditionalOverwrite() {
         final var handler = new S3HttpHandler("bucket", "path");
 
-        var tasks = List.of(
-            createPutObjectTask(handler),
-            createPutObjectTask(handler),
-            createMultipartUploadTask(handler),
-            createMultipartUploadTask(handler)
+        final var originalBody = new BytesArray(randomAlphaOfLength(50).getBytes(StandardCharsets.UTF_8));
+        final var originalETag = S3HttpHandler.getEtagFromContents(originalBody);
+        assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/blob", originalBody).status());
+        assertEquals(
+            new TestHttpResponse(RestStatus.OK, originalBody, addETag(originalETag, TestHttpExchange.EMPTY_HEADERS)),
+            handleRequest(handler, "GET", "/bucket/path/blob")
+        );
+
+        ensureExactlyOneSuccess(handler, originalETag);
+    }
+
+    private static void ensureExactlyOneSuccess(S3HttpHandler handler, String originalETag) {
+        final var tasks = List.of(
+            createPutObjectTask(handler, originalETag),
+            createPutObjectTask(handler, originalETag),
+            createMultipartUploadTask(handler, originalETag),
+            createMultipartUploadTask(handler, originalETag)
         );
 
         runInParallel(tasks.size(), i -> tasks.get(i).consumer.run());
@@ -445,13 +461,38 @@ public void testPreventObjectOverwrite() {
         );
     }
 
-    private static TestWriteTask createPutObjectTask(S3HttpHandler handler) {
+    public void testPutObjectIfMatchWithBlobNotFound() {
+        final var handler = new S3HttpHandler("bucket", "path");
+        while (true) {
+            final var task = createPutObjectTask(handler, randomIdentifier());
+            task.consumer.run();
+            if (task.status == RestStatus.NOT_FOUND) {
+                break;
+            }
+            assertEquals(RestStatus.CONFLICT, task.status); // chosen randomly so eventually we will escape the loop
+        }
+    }
+
+    public void testCompleteMultipartUploadIfMatchWithBlobNotFound() {
+        final var handler = new S3HttpHandler("bucket", "path");
+        while (true) {
+            final var task = createMultipartUploadTask(handler, randomIdentifier());
+            task.consumer.run();
+            if (task.status == RestStatus.NOT_FOUND) {
+                break;
+            }
+            assertEquals(RestStatus.CONFLICT, task.status); // chosen randomly so eventually we will escape the loop
+        }
+    }
+
+    private static TestWriteTask createPutObjectTask(S3HttpHandler handler, @Nullable String originalETag) {
         return new TestWriteTask(
-            (task) -> task.status = handleRequest(handler, "PUT", "/bucket/path/blob", task.body, ifNoneMatchHeader()).status()
+            (task) -> task.status = handleRequest(handler, "PUT", "/bucket/path/blob", task.body, conditionalWriteHeader(originalETag))
+                .status()
         );
     }
 
-    private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler) {
+    private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler, @Nullable String originalETag) {
         final var multipartUploadTask = new TestWriteTask(
             (task) -> task.status = handleRequest(
                 handler,
@@ -465,7 +506,7 @@ private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler) {
                           <PartNumber>1</PartNumber>
                        </Part>
                     </CompleteMultipartUpload>""", task.etag)),
-                ifNoneMatchHeader()
+                conditionalWriteHeader(originalETag)
             ).status()
         );
 
@@ -594,9 +635,13 @@ private static Headers contentRangeHeader(long start, long end, long length) {
         return headers;
     }
 
-    private static Headers ifNoneMatchHeader() {
+    private static Headers conditionalWriteHeader(@Nullable String originalEtag) {
         var headers = new Headers();
-        headers.put("If-None-Match", List.of("*"));
+        if (originalEtag == null) {
+            headers.put("If-None-Match", List.of("*"));
+        } else {
+            headers.put("If-Match", List.of(originalEtag));
+        }
         return headers;
     }