Add force_synthetic_source to mget

nik9000 · nik9000 · commit 93f780cea990 · 2022-06-09T14:26:46.000-04:00
This adds the option to force synthetic source to the MGET API. See elastic#87068 for more discussion on why you'd want to do that - the short version is to get an upper bound on the performance cost of using synthetic source in MGET.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/mget.json b/rest-api-spec/src/main/resources/rest-api-spec/api/mget.json
@@ -35,6 +35,12 @@
       ]
     },
     "params":{
+      "force_synthetic_source": {
+        "type": "boolean",
+        "description": "Should this request force synthetic _source? Use this to test if the mapping supports synthetic _source and to get a sense of the worst case performance. Fetches with this enabled will be slower the enabling synthetic source natively in the index.",
+        "visibility": "feature_flag",
+        "feature_flag": "es.index_mode_feature_flag_registered"
+      },
       "stored_fields":{
         "type":"list",
         "description":"A comma-separated list of stored fields to return in the response"
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/90_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/90_synthetic_source.yml
@@ -45,3 +45,117 @@ keyword:
   - match:
       docs.1._source:
         kwd: bar
+
+---
+force_synthetic_source_ok:
+  - skip:
+      version: " - 8.3.99"
+      reason: introduced in 8.4.0
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          mappings:
+            _source:
+              synthetic: false
+            properties:
+              kwd:
+                type: keyword
+
+  - do:
+      index:
+        index:   test
+        id:      1
+        body:
+          kwd: foo
+
+  - do:
+      index:
+        index:   test
+        id:      2
+        body:
+          kwd: bar
+
+  # When _source is used in the fetch the original _source is perfect
+  - do:
+      mget:
+        index: test
+        body:
+          ids: [1, 2]
+  - match:
+      docs.0._source:
+        kwd: foo
+  - match:
+      docs.1._source:
+        kwd: bar
+
+  # When we force synthetic source dots in field names get turned into objects
+  - do:
+      mget:
+        index: test
+        force_synthetic_source: true
+        body:
+          ids: [ 1, 2 ]
+  - match:
+      docs.0._source:
+        kwd: foo
+  - match:
+      docs.1._source:
+        kwd: bar
+
+---
+force_synthetic_source_bad_mapping:
+  - skip:
+      version: " - 8.3.99"
+      reason: introduced in 8.4.0
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          settings:
+            number_of_shards: 1 # Use a single shard to get consistent error messages
+          mappings:
+            _source:
+              synthetic: false
+            properties:
+              text:
+                type: text
+
+  - do:
+      index:
+        index:   test
+        id:      1
+        body:
+          text: foo
+
+  - do:
+      index:
+        index:   test
+        id:      2
+        body:
+          text: bar
+
+  # When _source is used in the fetch the original _source is perfect
+  - do:
+      mget:
+        index: test
+        body:
+          ids: [ 1, 2 ]
+  - match:
+      docs.0._source:
+        text: foo
+  - match:
+      docs.1._source:
+        text: bar
+
+  # Forcing synthetic source fails because the mapping is invalid
+  - do:
+      mget:
+        index: test
+        force_synthetic_source: true
+        body:
+          ids: [ 1, 2 ]
+  - match: {docs.0.error.reason: "field [text] of type [text] doesn't support synthetic source unless it has a sub-field of type [keyword] with doc values enabled and without ignore_above or a normalizer"}
+  - match: {docs.1.error.reason: "field [text] of type [text] doesn't support synthetic source unless it has a sub-field of type [keyword] with doc values enabled and without ignore_above or a normalizer"}
diff --git a/server/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java b/server/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java
@@ -28,6 +28,7 @@
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.rest.action.document.RestMultiGetAction;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.xcontent.ParseField;
@@ -246,6 +247,14 @@ public String toString() {
     boolean refresh;
     List<Item> items = new ArrayList<>();
 
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     * Use this to test if the mapping supports synthetic _source and to get a sense
+     * of the worst case performance. Fetches with this enabled will be slower the
+     * enabling synthetic source natively in the index.
+     */
+    private boolean forceSyntheticSource = false;
+
     public MultiGetRequest() {}
 
     public MultiGetRequest(StreamInput in) throws IOException {
@@ -254,6 +263,11 @@ public MultiGetRequest(StreamInput in) throws IOException {
         refresh = in.readBoolean();
         realtime = in.readBoolean();
         items = in.readList(Item::new);
+        if (in.getVersion().onOrAfter(Version.V_8_4_0)) {
+            forceSyntheticSource = in.readBoolean();
+        } else {
+            forceSyntheticSource = false;
+        }
     }
 
     @Override
@@ -263,6 +277,13 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeBoolean(refresh);
         out.writeBoolean(realtime);
         out.writeList(items);
+        if (out.getVersion().onOrAfter(Version.V_8_4_0)) {
+            out.writeBoolean(forceSyntheticSource);
+        } else {
+            if (forceSyntheticSource) {
+                throw new IllegalArgumentException("force_synthetic_source is not supported before 8.4.0");
+            }
+        }
     }
 
     public List<Item> getItems() {
@@ -331,6 +352,26 @@ public MultiGetRequest refresh(boolean refresh) {
         return this;
     }
 
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     * Use this to test if the mapping supports synthetic _source and to get a sense
+     * of the worst case performance. Fetches with this enabled will be slower the
+     * enabling synthetic source natively in the index.
+     */
+    public void setForceSyntheticSource(boolean forceSyntheticSource) {
+        this.forceSyntheticSource = forceSyntheticSource;
+    }
+
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     * Use this to test if the mapping supports synthetic _source and to get a sense
+     * of the worst case performance. Fetches with this enabled will be slower the
+     * enabling synthetic source natively in the index.
+     */
+    public boolean isForceSyntheticSource() {
+        return forceSyntheticSource;
+    }
+
     public MultiGetRequest add(
         @Nullable String defaultIndex,
         @Nullable String[] defaultFields,
diff --git a/server/src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java b/server/src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java
@@ -8,10 +8,12 @@
 
 package org.elasticsearch.action.get;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.single.shard.SingleShardRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.index.mapper.SourceLoader;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -27,6 +29,14 @@ public class MultiGetShardRequest extends SingleShardRequest<MultiGetShardReques
     List<Integer> locations;
     List<MultiGetRequest.Item> items;
 
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     * Use this to test if the mapping supports synthetic _source and to get a sense
+     * of the worst case performance. Fetches with this enabled will be slower the
+     * enabling synthetic source natively in the index.
+     */
+    private final boolean forceSyntheticSource;
+
     MultiGetShardRequest(MultiGetRequest multiGetRequest, String index, int shardId) {
         super(index);
         this.shardId = shardId;
@@ -35,6 +45,7 @@ public class MultiGetShardRequest extends SingleShardRequest<MultiGetShardReques
         preference = multiGetRequest.preference;
         realtime = multiGetRequest.realtime;
         refresh = multiGetRequest.refresh;
+        forceSyntheticSource = multiGetRequest.isForceSyntheticSource();
     }
 
     MultiGetShardRequest(StreamInput in) throws IOException {
@@ -51,6 +62,11 @@ public class MultiGetShardRequest extends SingleShardRequest<MultiGetShardReques
         preference = in.readOptionalString();
         refresh = in.readBoolean();
         realtime = in.readBoolean();
+        if (in.getVersion().onOrAfter(Version.V_8_4_0)) {
+            forceSyntheticSource = in.readBoolean();
+        } else {
+            forceSyntheticSource = false;
+        }
     }
 
     @Override
@@ -66,6 +82,13 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeOptionalString(preference);
         out.writeBoolean(refresh);
         out.writeBoolean(realtime);
+        if (out.getVersion().onOrAfter(Version.V_8_4_0)) {
+            out.writeBoolean(forceSyntheticSource);
+        } else {
+            if (forceSyntheticSource) {
+                throw new IllegalArgumentException("force_synthetic_source is not supported before 8.4.0");
+            }
+        }
     }
 
     @Override
@@ -109,6 +132,16 @@ public MultiGetShardRequest refresh(boolean refresh) {
         return this;
     }
 
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     * Use this to test if the mapping supports synthetic _source and to get a sense
+     * of the worst case performance. Fetches with this enabled will be slower the
+     * enabling synthetic source natively in the index.
+     */
+    public boolean isForceSyntheticSource() {
+        return forceSyntheticSource;
+    }
+
     void add(int location, MultiGetRequest.Item item) {
         this.locations.add(location);
         this.items.add(item);
diff --git a/server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java b/server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java
@@ -124,7 +124,7 @@ protected MultiGetShardResponse shardOperation(MultiGetShardRequest request, Sha
                         item.version(),
                         item.versionType(),
                         item.fetchSourceContext(),
-                        false
+                        request.isForceSyntheticSource()
                     );
                 response.add(request.locations.get(i), new GetResponse(getResult));
             } catch (RuntimeException e) {
diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestMultiGetAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestMultiGetAction.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.RestApiVersion;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.action.RestToXContentListener;
@@ -60,6 +61,10 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         multiGetRequest.refresh(request.paramAsBoolean("refresh", multiGetRequest.refresh()));
         multiGetRequest.preference(request.param("preference"));
         multiGetRequest.realtime(request.paramAsBoolean("realtime", multiGetRequest.realtime()));
+        if (IndexSettings.isTimeSeriesModeEnabled() && request.paramAsBoolean("force_synthetic_source", false)) {
+            multiGetRequest.setForceSyntheticSource(true);
+        }
+
         if (request.param("fields") != null) {
             throw new IllegalArgumentException(
                 "The parameter [fields] is no longer supported, "
diff --git a/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java b/server/src/test/java/org/elasticsearch/action/get/GetRequestTests.java
@@ -7,7 +7,10 @@
  */
 package org.elasticsearch.action.get;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.CoreMatchers.hasItems;
@@ -33,4 +36,13 @@ public void testValidation() {
             assertThat(validate.validationErrors(), hasItems("id is missing"));
         }
     }
+
+    public void testForceSyntheticUnsupported() {
+        GetRequest request = new GetRequest("index", "id");
+        request.setForceSyntheticSource(true);
+        StreamOutput out = new BytesStreamOutput();
+        out.setVersion(Version.V_8_3_0);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> request.writeTo(out));
+        assertEquals(e.getMessage(), "force_synthetic_source is not supported before 8.4.0");
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/action/get/MultiGetRequestTests.java b/server/src/test/java/org/elasticsearch/action/get/MultiGetRequestTests.java
@@ -8,8 +8,11 @@
 
 package org.elasticsearch.action.get;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.test.ESTestCase;
@@ -117,6 +120,15 @@ public void testXContentSerialization() throws IOException {
         }
     }
 
+    public void testForceSyntheticUnsupported() {
+        MultiGetRequest request = createTestInstance();
+        request.setForceSyntheticSource(true);
+        StreamOutput out = new BytesStreamOutput();
+        out.setVersion(Version.V_8_3_0);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> request.writeTo(out));
+        assertEquals(e.getMessage(), "force_synthetic_source is not supported before 8.4.0");
+    }
+
     private MultiGetRequest createTestInstance() {
         int numItems = randomIntBetween(0, 128);
         MultiGetRequest request = new MultiGetRequest();
@@ -149,6 +161,9 @@ private MultiGetRequest createTestInstance() {
             }
             request.add(item);
         }
+        if (randomBoolean()) {
+            request.setForceSyntheticSource(true);
+        }
         return request;
     }
 
diff --git a/server/src/test/java/org/elasticsearch/action/get/MultiGetShardRequestTests.java b/server/src/test/java/org/elasticsearch/action/get/MultiGetShardRequestTests.java
