Selaa lähdekoodia

Add force_synthetic_source to GET (#87536)

This adds the option to force synthetic source to the GET API. See
 #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 GET.
Nik Everett 3 vuotta sitten
vanhempi
commit
a37edb7796

+ 6 - 0
rest-api-spec/src/main/resources/rest-api-spec/api/get.json

@@ -30,6 +30,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"

+ 91 - 0
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml

@@ -33,3 +33,94 @@ keyword:
   - match:
       _source:
         kwd: foo
+
+---
+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:
+              obj:
+                properties:
+                  kwd:
+                    type: keyword
+
+  - do:
+      index:
+        index:   test
+        id:      1
+        refresh: true
+        body:
+          obj.kwd: foo
+
+  # When _source is used in the fetch the original _source is perfect
+  - do:
+      get:
+        index: test
+        id: 1
+  - match:
+      _source:
+        obj.kwd: foo
+
+  # When we force synthetic source dots in field names get turned into objects
+  - do:
+      get:
+        index: test
+        id: 1
+        force_synthetic_source: true
+  - match:
+      _source:
+        obj:
+          kwd: foo
+
+---
+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
+        refresh: true
+        body:
+          text: foo
+
+  # When _source is used in the fetch the original _source is perfect
+  - do:
+      get:
+        index: test
+        id: 1
+  - match:
+      _source:
+        text: foo
+
+  # Forcing synthetic source fails because the mapping is invalid
+  - do:
+      catch: bad_request
+      get:
+        index: test
+        id: 1
+        force_synthetic_source: true

+ 41 - 0
server/src/main/java/org/elasticsearch/action/get/GetRequest.java

@@ -19,6 +19,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 
 import java.io.IOException;
@@ -55,6 +56,14 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
     private VersionType versionType = VersionType.INTERNAL;
     private long version = Versions.MATCH_ANY;
 
+    /**
+     * 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 GetRequest() {}
 
     GetRequest(StreamInput in) throws IOException {
@@ -72,6 +81,11 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
         this.versionType = VersionType.fromValue(in.readByte());
         this.version = in.readLong();
         fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::readFrom);
+        if (in.getVersion().onOrAfter(Version.V_8_4_0)) {
+            forceSyntheticSource = in.readBoolean();
+        } else {
+            forceSyntheticSource = false;
+        }
     }
 
     @Override
@@ -90,6 +104,13 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
         out.writeByte(versionType.getValue());
         out.writeLong(version);
         out.writeOptionalWriteable(fetchSourceContext);
+        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");
+            }
+        }
     }
 
     /**
@@ -242,6 +263,26 @@ public class GetRequest extends SingleShardRequest<GetRequest> implements Realti
         return this.versionType;
     }
 
+    /**
+     * 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;
+    }
+
     @Override
     public String toString() {
         return "get [" + index + "][" + id + "]: routing [" + routing + "]";

+ 2 - 1
server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

@@ -116,7 +116,8 @@ public class TransportGetAction extends TransportSingleShardAction<GetRequest, G
                 request.realtime(),
                 request.version(),
                 request.versionType(),
-                request.fetchSourceContext()
+                request.fetchSourceContext(),
+                request.isForceSyntheticSource()
             );
         return new GetResponse(result);
     }

+ 9 - 1
server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java

@@ -117,7 +117,15 @@ public class TransportShardMultiGetAction extends TransportSingleShardAction<Mul
             MultiGetRequest.Item item = request.items.get(i);
             try {
                 GetResult getResult = indexShard.getService()
-                    .get(item.id(), item.storedFields(), request.realtime(), item.version(), item.versionType(), item.fetchSourceContext());
+                    .get(
+                        item.id(),
+                        item.storedFields(),
+                        request.realtime(),
+                        item.version(),
+                        item.versionType(),
+                        item.fetchSourceContext(),
+                        false
+                    );
                 response.add(request.locations.get(i), new GetResponse(getResult));
             } catch (RuntimeException e) {
                 if (TransportActions.isShardNotAvailableException(e)) {

+ 3 - 0
server/src/main/java/org/elasticsearch/action/search/SearchRequest.java

@@ -755,6 +755,9 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla
 
     /**
      * 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;

+ 40 - 11
server/src/main/java/org/elasticsearch/index/get/ShardGetService.java

@@ -28,6 +28,7 @@ import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MappingLookup;
 import org.elasticsearch.index.mapper.RoutingFieldMapper;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.shard.AbstractIndexShardComponent;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@@ -70,9 +71,20 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         boolean realtime,
         long version,
         VersionType versionType,
-        FetchSourceContext fetchSourceContext
+        FetchSourceContext fetchSourceContext,
+        boolean forceSyntheticSource
     ) throws IOException {
-        return get(id, gFields, realtime, version, versionType, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM, fetchSourceContext);
+        return get(
+            id,
+            gFields,
+            realtime,
+            version,
+            versionType,
+            UNASSIGNED_SEQ_NO,
+            UNASSIGNED_PRIMARY_TERM,
+            fetchSourceContext,
+            forceSyntheticSource
+        );
     }
 
     private GetResult get(
@@ -83,12 +95,23 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         VersionType versionType,
         long ifSeqNo,
         long ifPrimaryTerm,
-        FetchSourceContext fetchSourceContext
+        FetchSourceContext fetchSourceContext,
+        boolean forceSyntheticSource
     ) throws IOException {
         currentMetric.inc();
         try {
             long now = System.nanoTime();
-            GetResult getResult = innerGet(id, gFields, realtime, version, versionType, ifSeqNo, ifPrimaryTerm, fetchSourceContext);
+            GetResult getResult = innerGet(
+                id,
+                gFields,
+                realtime,
+                version,
+                versionType,
+                ifSeqNo,
+                ifPrimaryTerm,
+                fetchSourceContext,
+                forceSyntheticSource
+            );
 
             if (getResult.isExists()) {
                 existsMetric.inc(System.nanoTime() - now);
@@ -110,7 +133,8 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             VersionType.INTERNAL,
             ifSeqNo,
             ifPrimaryTerm,
-            FetchSourceContext.FETCH_SOURCE
+            FetchSourceContext.FETCH_SOURCE,
+            false
         );
     }
 
@@ -131,7 +155,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         try {
             long now = System.nanoTime();
             fetchSourceContext = normalizeFetchSourceContent(fetchSourceContext, fields);
-            GetResult getResult = innerGetLoadFromStoredFields(id, fields, fetchSourceContext, engineGetResult);
+            GetResult getResult = innerGetFetch(id, fields, fetchSourceContext, engineGetResult, false);
             if (getResult.isExists()) {
                 existsMetric.inc(System.nanoTime() - now);
             } else {
@@ -169,7 +193,8 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         VersionType versionType,
         long ifSeqNo,
         long ifPrimaryTerm,
-        FetchSourceContext fetchSourceContext
+        FetchSourceContext fetchSourceContext,
+        boolean forceSyntheticSource
     ) throws IOException {
         fetchSourceContext = normalizeFetchSourceContent(fetchSourceContext, gFields);
 
@@ -189,17 +214,18 @@ public final class ShardGetService extends AbstractIndexShardComponent {
 
         try {
             // break between having loaded it from translog (so we only have _source), and having a document to load
-            return innerGetLoadFromStoredFields(id, gFields, fetchSourceContext, get);
+            return innerGetFetch(id, gFields, fetchSourceContext, get, forceSyntheticSource);
         } finally {
             get.close();
         }
     }
 
-    private GetResult innerGetLoadFromStoredFields(
+    private GetResult innerGetFetch(
         String id,
         String[] storedFields,
         FetchSourceContext fetchSourceContext,
-        Engine.GetResult get
+        Engine.GetResult get,
+        boolean forceSyntheticSource
     ) throws IOException {
         assert get.exists() : "method should only be called if document could be retrieved";
 
@@ -228,7 +254,10 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             } catch (IOException e) {
                 throw new ElasticsearchException("Failed to get id [" + id + "]", e);
             }
-            source = mappingLookup.newSourceLoader().leaf(docIdAndVersion.reader).source(fieldVisitor, docIdAndVersion.docId);
+            SourceLoader loader = forceSyntheticSource
+                ? new SourceLoader.Synthetic(mappingLookup.getMapping())
+                : mappingLookup.newSourceLoader();
+            source = loader.leaf(docIdAndVersion.reader).source(fieldVisitor, docIdAndVersion.docId);
 
             // put stored fields into result objects
             if (fieldVisitor.fields().isEmpty() == false) {

+ 4 - 0
server/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java

@@ -13,6 +13,7 @@ import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.client.internal.node.NodeClient;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.RestApiVersion;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
@@ -78,6 +79,9 @@ public class RestGetAction extends BaseRestHandler {
         getRequest.versionType(VersionType.fromString(request.param("version_type"), getRequest.versionType()));
 
         getRequest.fetchSourceContext(FetchSourceContext.parseFromRestRequest(request));
+        if (IndexSettings.isTimeSeriesModeEnabled() && request.paramAsBoolean("force_synthetic_source", false)) {
+            getRequest.setForceSyntheticSource(true);
+        }
 
         return channel -> client.get(getRequest, new RestToXContentListener<GetResponse>(channel) {
             @Override

+ 3 - 2
server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java

@@ -160,7 +160,7 @@ public class ShardGetServiceTests extends IndexShardTestCase {
         Engine.IndexResult test2 = indexDoc(primary, "2", docToIndex, XContentType.JSON, "foobar");
         assertTrue(primary.getEngine().refreshNeeded());
         GetResult testGet2 = primary.getService()
-            .get("2", new String[] { "foo" }, true, 1, VersionType.INTERNAL, FetchSourceContext.FETCH_SOURCE);
+            .get("2", new String[] { "foo" }, true, 1, VersionType.INTERNAL, FetchSourceContext.FETCH_SOURCE, false);
         assertEquals(new String(testGet2.source() == null ? new byte[0] : testGet2.source(), StandardCharsets.UTF_8), expectedResult);
         assertTrue(testGet2.getFields().containsKey(RoutingFieldMapper.NAME));
         assertTrue(testGet2.getFields().containsKey("foo"));
@@ -174,7 +174,8 @@ public class ShardGetServiceTests extends IndexShardTestCase {
             assertEquals(searcher.getIndexReader().maxDoc(), 3);
         }
 
-        testGet2 = primary.getService().get("2", new String[] { "foo" }, true, 1, VersionType.INTERNAL, FetchSourceContext.FETCH_SOURCE);
+        testGet2 = primary.getService()
+            .get("2", new String[] { "foo" }, true, 1, VersionType.INTERNAL, FetchSourceContext.FETCH_SOURCE, false);
         assertEquals(new String(testGet2.source() == null ? new byte[0] : testGet2.source(), StandardCharsets.UTF_8), expectedResult);
         assertTrue(testGet2.getFields().containsKey(RoutingFieldMapper.NAME));
         assertTrue(testGet2.getFields().containsKey("foo"));