Try to remove overhead of requesting default metadata fields multiple…

… times
javanna · Apr 16, 2024 · c746e14 · c746e14
1 parent d11048e
commit c746e14
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 17 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java b/server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java
@@ -28,8 +28,6 @@
 import java.util.Set;
 import java.util.function.Function;
 
-import static java.util.Collections.emptyMap;
-
 /**
  * Base {@link StoredFieldVisitor} that retrieves all non-redundant metadata.
  */
@@ -41,7 +39,7 @@ public class FieldsVisitor extends FieldNamesProvidingStoredFieldsVisitor {
     private final Set<String> requiredFields;
     protected BytesReference source;
     protected String id;
-    protected Map<String, List<Object>> fieldsValues;
+    private final Map<String, List<Object>> fieldsValues = new HashMap<>();
 
     public FieldsVisitor(boolean loadSource) {
         this(loadSource, SourceFieldMapper.NAME);
@@ -156,9 +154,6 @@ public String id() {
     }
 
     public String routing() {
-        if (fieldsValues == null) {
-            return null;
-        }
         List<Object> values = fieldsValues.get(RoutingFieldMapper.NAME);
         if (values == null || values.isEmpty()) {
             return null;
@@ -168,13 +163,13 @@ public String routing() {
     }
 
     public Map<String, List<Object>> fields() {
-        return fieldsValues != null ? fieldsValues : emptyMap();
+        return fieldsValues;
     }
 
     public void reset() {
-        if (fieldsValues != null) {
-            fieldsValues.clear();
-        }
+        fieldsValues.clear();
+        fieldsValues.put(RoutingFieldMapper.NAME, null);
+        fieldsValues.put(IgnoredFieldMapper.NAME, null);
         source = null;
         id = null;
 
@@ -185,10 +180,6 @@ public void reset() {
     }
 
     void addValue(String name, Object value) {
-        if (fieldsValues == null) {
-            fieldsValues = new HashMap<>();
-        }
-
         List<Object> values = fieldsValues.computeIfAbsent(name, k -> new ArrayList<>(2));
         values.add(value);
     }

diff --git a/server/src/main/java/org/elasticsearch/index/fieldvisitor/StoredFieldLoader.java b/server/src/main/java/org/elasticsearch/index/fieldvisitor/StoredFieldLoader.java
@@ -147,6 +147,7 @@ private static CheckedBiConsumer<Integer, FieldsVisitor, IOException> sequential
 
     private static List<String> fieldsToLoad(boolean loadSource, Set<String> fields) {
         Set<String> fieldsToLoad = new HashSet<>();
+        // TODO is this necessary? why is _ignored not in here?
         fieldsToLoad.add("_id");
         fieldsToLoad.add("_routing");
         if (loadSource) {

diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java
@@ -12,7 +12,6 @@
 import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.index.mapper.IgnoredFieldMapper;
-import org.elasticsearch.index.mapper.LegacyTypeFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.RoutingFieldMapper;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
@@ -39,8 +38,7 @@ public final class FetchFieldsPhase implements FetchSubPhase {
 
     private static final List<FieldAndFormat> DEFAULT_METADATA_FIELDS = List.of(
         new FieldAndFormat(IgnoredFieldMapper.NAME, null),
-        new FieldAndFormat(RoutingFieldMapper.NAME, null),
-        new FieldAndFormat(LegacyTypeFieldMapper.NAME, null)
+        new FieldAndFormat(RoutingFieldMapper.NAME, null)
     );
 
     @Override