Add an option to _search to force synthetic source (#87068)

This adds `?force_synthetic_source` to, well, force running the fetch phase with synthetic source. If the mapping is incompatible with synthetic source it'll throw a 400 error.
elastic · Jun 7, 2022 · d0b50b5 · d0b50b5
1 parent 194cfc5
commit d0b50b5
Show file tree

Hide file tree

Showing 22 changed files with 257 additions and 18 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/search.json b/rest-api-spec/src/main/resources/rest-api-spec/api/search.json
@@ -77,6 +77,12 @@
         "type":"number",
         "description":"Starting offset (default: 0)"
       },
+      "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"
+      },
       "ignore_unavailable":{
         "type":"boolean",
         "description":"Whether specified concrete indices should be ignored when unavailable (missing or closed)"

diff --git a/rest-api-spec/src/main/resources/schema.json b/rest-api-spec/src/main/resources/schema.json
@@ -276,6 +276,14 @@
                 "required": {
                     "type": "boolean",
                     "title": "Whether the parameter is required"
+                },
+                "visibility": {
+                    "type": "string",
+                    "enum": ["private", "feature_flag", "public"]
+                },
+                "feature_flag": {
+                    "type": "string",
+                    "title": "If visibility of the API is set to `feature_flag` this documents the feature_flag to use"
                 }
             },
             "required": [

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/400_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/400_synthetic_source.yml
@@ -32,3 +32,104 @@ keyword:
   - match:
       hits.hits.0._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:
+      search:
+        index: test
+        body:
+          query:
+            ids:
+              values: [1]
+  - match:
+      hits.hits.0._source:
+        obj.kwd: foo
+
+  # When we force synthetic source dots in field names get turned into objects
+  - do:
+      search:
+        index: test
+        force_synthetic_source: true
+        body:
+          query:
+            ids:
+              values: [1]
+  - match:
+      hits.hits.0._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:
+          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:
+      search:
+        index: test
+        body:
+          query:
+            ids:
+              values: [1]
+  - match:
+      hits.hits.0._source:
+        text: foo
+
+  # Forcing synthetic source fails because the mapping is invalid
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        force_synthetic_source: true
+        body:
+          query:
+            ids:
+              values: [1]
diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchNodeRequest.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchNodeRequest.java
@@ -217,7 +217,8 @@ public ShardSearchRequest createShardSearchRequest(Shard r) {
             r.readerId,
             r.keepAlive,
             r.waitForCheckpoint,
-            waitForCheckpointsTimeout
+            waitForCheckpointsTimeout,
+            false
         );
         shardSearchRequest.setParentTask(getParentTask());
         return shardSearchRequest;

diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java
@@ -18,6 +18,7 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.query.QueryRewriteContext;
 import org.elasticsearch.index.query.Rewriteable;
 import org.elasticsearch.search.Scroll;
@@ -102,6 +103,14 @@ public class SearchRequest extends ActionRequest implements IndicesRequest.Repla
 
     private TimeValue waitForCheckpointsTimeout = TimeValue.timeValueSeconds(30);
 
+    /**
+     * 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 SearchRequest() {
         this((Version) null);
     }
@@ -212,6 +221,7 @@ private SearchRequest(
         this.minCompatibleShardNode = searchRequest.minCompatibleShardNode;
         this.waitForCheckpoints = searchRequest.waitForCheckpoints;
         this.waitForCheckpointsTimeout = searchRequest.waitForCheckpointsTimeout;
+        this.forceSyntheticSource = searchRequest.forceSyntheticSource;
     }
 
     /**
@@ -261,6 +271,9 @@ public SearchRequest(StreamInput in) throws IOException {
             waitForCheckpoints = in.readMap(StreamInput::readString, StreamInput::readLongArray);
             waitForCheckpointsTimeout = in.readTimeValue();
         }
+        if (in.getVersion().onOrAfter(Version.V_8_4_0)) {
+            forceSyntheticSource = in.readBoolean();
+        }
     }
 
     @Override
@@ -308,6 +321,9 @@ public void writeTo(StreamOutput out) throws IOException {
                     + "] or greater."
             );
         }
+        if (out.getVersion().onOrAfter(Version.V_8_4_0)) {
+            out.writeBoolean(forceSyntheticSource);
+        }
     }
 
     @Override
@@ -721,6 +737,23 @@ public int resolveTrackTotalHitsUpTo() {
         return resolveTrackTotalHitsUpTo(scroll, source);
     }
 
+    /**
+     * 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;
+    }
+
+    /**
+     * Should this request force {@link SourceLoader.Synthetic synthetic source}?
+     */
+    public void setForceSyntheticSource(boolean forceSyntheticSource) {
+        this.forceSyntheticSource = forceSyntheticSource;
+    }
+
     @Override
     public SearchRequest rewrite(QueryRewriteContext ctx) throws IOException {
         if (source == null) {
@@ -804,7 +837,8 @@ public boolean equals(Object o) {
             && Objects.equals(localClusterAlias, that.localClusterAlias)
             && absoluteStartMillis == that.absoluteStartMillis
             && ccsMinimizeRoundtrips == that.ccsMinimizeRoundtrips
-            && Objects.equals(minCompatibleShardNode, that.minCompatibleShardNode);
+            && Objects.equals(minCompatibleShardNode, that.minCompatibleShardNode)
+            && forceSyntheticSource == that.forceSyntheticSource;
     }
 
     @Override
@@ -825,7 +859,8 @@ public int hashCode() {
             localClusterAlias,
             absoluteStartMillis,
             ccsMinimizeRoundtrips,
-            minCompatibleShardNode
+            minCompatibleShardNode,
+            forceSyntheticSource
         );
     }
 

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
@@ -97,7 +97,7 @@ public void validate(IndexSettings settings, boolean checkLimits) {
          * Build an empty source loader to validate that the mapping is compatible
          * with the source loading strategy declared on the source field mapper.
          */
-        sourceMapper().newSourceLoader(mapping().getRoot());
+        sourceMapper().newSourceLoader(mapping());
 
         settings.getMode().validateMapping(mappingLookup);
         if (settings.getIndexSortConfig().hasIndexSort() && mappers().nestedLookup() != NestedLookup.EMPTY) {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java
@@ -404,9 +404,12 @@ public boolean isSourceEnabled() {
         return sfm != null && sfm.enabled();
     }
 
+    /**
+     * Build something to load source {@code _source}.
+     */
     public SourceLoader newSourceLoader() {
         SourceFieldMapper sfm = mapping.getMetadataMapperByClass(SourceFieldMapper.class);
-        return sfm == null ? SourceLoader.FROM_STORED_SOURCE : sfm.newSourceLoader(mapping.getRoot());
+        return sfm == null ? SourceLoader.FROM_STORED_SOURCE : sfm.newSourceLoader(mapping);
     }
 
     /**

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -211,9 +211,12 @@ public FieldMapper.Builder getMergeBuilder() {
         return new Builder().init(this);
     }
 
-    public <T> SourceLoader newSourceLoader(RootObjectMapper root) {
+    /**
+     * Build something to load source {@code _source}.
+     */
+    public <T> SourceLoader newSourceLoader(Mapping mapping) {
         if (synthetic) {
-            return new SourceLoader.Synthetic(root);
+            return new SourceLoader.Synthetic(mapping);
         }
         return SourceLoader.FROM_STORED_SOURCE;
     }

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java
@@ -18,7 +18,7 @@
 import java.io.IOException;
 
 /**
- * Loads source {@code _source} during the a GET or {@code _search}.
+ * Loads source {@code _source} during a GET or {@code _search}.
  */
 public interface SourceLoader {
     /**
@@ -60,8 +60,8 @@ public BytesReference source(FieldsVisitor fieldsVisitor, int docId) {
     class Synthetic implements SourceLoader {
         private final SyntheticFieldLoader loader;
 
-        Synthetic(RootObjectMapper root) {
-            loader = root.syntheticFieldLoader();
+        public Synthetic(Mapping mapping) {
+            loader = mapping.getRoot().syntheticFieldLoader();
         }
 
         @Override

diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java
@@ -390,7 +390,13 @@ public boolean isSourceEnabled() {
         return mappingLookup.isSourceEnabled();
     }
 
-    public SourceLoader newSourceLoader() {
+    /**
+     * Build something to load source {@code _source}.
+     */
+    public SourceLoader newSourceLoader(boolean forceSyntheticSource) {
+        if (forceSyntheticSource) {
+            return new SourceLoader.Synthetic(mappingLookup.getMapping());
+        }
         return mappingLookup.newSourceLoader();
     }
 

diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.core.Booleans;
 import org.elasticsearch.core.RestApiVersion;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
@@ -208,6 +209,9 @@ public static void parseSearchRequest(
                 request.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips())
             );
         }
+        if (IndexSettings.isTimeSeriesModeEnabled() && request.paramAsBoolean("force_synthetic_source", false)) {
+            searchRequest.setForceSyntheticSource(true);
+        }
 
         extraParamParser.accept(request, searchRequest);
     }

diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
 import org.elasticsearch.index.engine.Engine;
 import org.elasticsearch.index.mapper.NestedLookup;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.query.AbstractQueryBuilder;
 import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.index.query.QueryBuilder;
@@ -759,4 +760,9 @@ public boolean isCancelled() {
     public ReaderContext readerContext() {
         return readerContext;
     }
+
+    @Override
+    public SourceLoader newSourceLoader() {
+        return searchExecutionContext.newSourceLoader(request.isForceSyntheticSource());
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
@@ -125,7 +125,7 @@ private SearchHits buildSearchHits(SearchContext context, Profiler profiler) {
         LeafNestedDocuments leafNestedDocuments = null;
         CheckedBiConsumer<Integer, FieldsVisitor, IOException> fieldReader = null;
         boolean hasSequentialDocs = hasSequentialDocs(docs);
-        SourceLoader sourceLoader = context.getSearchExecutionContext().newSourceLoader();
+        SourceLoader sourceLoader = context.newSourceLoader();
         SourceLoader.Leaf leafSourceLoader = null;
         for (int index = 0; index < context.docIdsToLoadSize(); index++) {
             if (context.isCancelled()) {

diff --git a/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.action.search.SearchType;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.shard.IndexShard;
@@ -446,4 +447,9 @@ public void addRescore(RescoreContext rescore) {
     public ReaderContext readerContext() {
         return in.readerContext();
     }
+
+    @Override
+    public SourceLoader newSourceLoader() {
+        return in.newSourceLoader();
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/server/src/main/java/org/elasticsearch/search/internal/SearchContext.java
@@ -17,6 +17,7 @@
 import org.elasticsearch.core.Releasables;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.index.query.QueryShardException;
 import org.elasticsearch.index.query.SearchExecutionContext;
@@ -370,4 +371,9 @@ public String toString() {
     }
 
     public abstract ReaderContext readerContext();
+
+    /**
+     * Build something to load source {@code _source}.
+     */
+    public abstract SourceLoader newSourceLoader();
 }