Browse Source

Reduce the number of SFM singletons. (#114969) (#115036)

This remove all recovery source specific SFM singletons. Whether  recovery source is enabled can be checked via `DocumentParserContext`. This reduces the number of SFM instances by half.
Martijn van Groningen 1 year ago
parent
commit
33b96cdc81

+ 11 - 0
server/src/main/java/org/elasticsearch/index/IndexSettings.java

@@ -30,6 +30,7 @@ import org.elasticsearch.index.mapper.IgnoredSourceFieldMapper;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.index.translog.Translog;
+import org.elasticsearch.indices.recovery.RecoverySettings;
 import org.elasticsearch.ingest.IngestService;
 import org.elasticsearch.node.Node;
 
@@ -817,6 +818,7 @@ public final class IndexSettings {
     private volatile boolean skipIgnoredSourceRead;
     private volatile boolean syntheticSourceSecondDocParsingPassEnabled;
     private final SourceFieldMapper.Mode indexMappingSourceMode;
+    private final boolean recoverySourceEnabled;
 
     /**
      * The maximum number of refresh listeners allows on this shard.
@@ -979,6 +981,7 @@ public final class IndexSettings {
         skipIgnoredSourceRead = scopedSettings.get(IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_READ_SETTING);
         syntheticSourceSecondDocParsingPassEnabled = scopedSettings.get(SYNTHETIC_SOURCE_SECOND_DOC_PARSING_PASS_SETTING);
         indexMappingSourceMode = scopedSettings.get(SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING);
+        recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings);
 
         scopedSettings.addSettingsUpdateConsumer(
             MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING,
@@ -1674,6 +1677,14 @@ public final class IndexSettings {
         return indexMappingSourceMode;
     }
 
+    /**
+     * @return Whether recovery source should be enabled if needed.
+     *         Note that this is a node setting, and this setting is not sourced from index settings.
+     */
+    public boolean isRecoverySourceEnabled() {
+        return recoverySourceEnabled;
+    }
+
     /**
      * The bounds for {@code @timestamp} on this index or
      * {@code null} if there are no bounds.

+ 24 - 131
server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java

@@ -39,8 +39,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 
-import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING;
-
 public class SourceFieldMapper extends MetadataFieldMapper {
     public static final NodeFeature SYNTHETIC_SOURCE_FALLBACK = new NodeFeature("mapper.source.synthetic_source_fallback");
     public static final NodeFeature SYNTHETIC_SOURCE_STORED_FIELDS_ADVANCE_FIX = new NodeFeature(
@@ -88,8 +86,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        null,
-        true
+        null
     );
 
     private static final SourceFieldMapper DEFAULT_DISABLED = new SourceFieldMapper(
@@ -97,17 +94,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        null,
-        true
-    );
-
-    private static final SourceFieldMapper DEFAULT_DISABLED_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        Mode.DISABLED,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        null,
-        false
+        null
     );
 
     private static final SourceFieldMapper DEFAULT_SYNTHETIC = new SourceFieldMapper(
@@ -115,26 +102,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        null,
-        true
-    );
-
-    private static final SourceFieldMapper DEFAULT_SYNTHETIC_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        Mode.SYNTHETIC,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        null,
-        false
-    );
-
-    private static final SourceFieldMapper DEFAULT_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        null,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        null,
-        false
+        null
     );
 
     private static final SourceFieldMapper TSDB_DEFAULT = new SourceFieldMapper(
@@ -142,8 +110,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        true
+        IndexMode.TIME_SERIES
     );
 
     private static final SourceFieldMapper TSDB_DEFAULT_STORED = new SourceFieldMapper(
@@ -151,26 +118,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        true
-    );
-
-    private static final SourceFieldMapper TSDB_DEFAULT_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        Mode.SYNTHETIC,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        false
-    );
-
-    private static final SourceFieldMapper TSDB_DEFAULT_NO_RECOVERY_SOURCE_STORED = new SourceFieldMapper(
-        Mode.STORED,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        false
+        IndexMode.TIME_SERIES
     );
 
     private static final SourceFieldMapper LOGSDB_DEFAULT = new SourceFieldMapper(
@@ -178,8 +126,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        IndexMode.LOGSDB,
-        true
+        IndexMode.LOGSDB
     );
 
     private static final SourceFieldMapper LOGSDB_DEFAULT_STORED = new SourceFieldMapper(
@@ -187,26 +134,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        IndexMode.LOGSDB,
-        true
-    );
-
-    private static final SourceFieldMapper LOGSDB_DEFAULT_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        Mode.SYNTHETIC,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        IndexMode.LOGSDB,
-        false
-    );
-
-    private static final SourceFieldMapper LOGSDB_DEFAULT_NO_RECOVERY_SOURCE_STORED = new SourceFieldMapper(
-        Mode.STORED,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        IndexMode.LOGSDB,
-        false
+        IndexMode.LOGSDB
     );
 
     /*
@@ -218,17 +146,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         Explicit.IMPLICIT_TRUE,
         Strings.EMPTY_ARRAY,
         Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        true
-    );
-
-    private static final SourceFieldMapper TSDB_LEGACY_DEFAULT_NO_RECOVERY_SOURCE = new SourceFieldMapper(
-        null,
-        Explicit.IMPLICIT_TRUE,
-        Strings.EMPTY_ARRAY,
-        Strings.EMPTY_ARRAY,
-        IndexMode.TIME_SERIES,
-        false
+        IndexMode.TIME_SERIES
     );
 
     public static class Defaults {
@@ -289,20 +207,12 @@ public class SourceFieldMapper extends MetadataFieldMapper {
 
         private final boolean supportsNonDefaultParameterValues;
 
-        private final boolean enableRecoverySource;
-
-        public Builder(
-            IndexMode indexMode,
-            final Settings settings,
-            boolean supportsCheckForNonDefaultParams,
-            boolean enableRecoverySource
-        ) {
+        public Builder(IndexMode indexMode, final Settings settings, boolean supportsCheckForNonDefaultParams) {
             super(Defaults.NAME);
             this.settings = settings;
             this.indexMode = indexMode;
             this.supportsNonDefaultParameterValues = supportsCheckForNonDefaultParams == false
                 || settings.getAsBoolean(LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, true);
-            this.enableRecoverySource = enableRecoverySource;
         }
 
         public Builder setSynthetic() {
@@ -337,7 +247,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
                 ? INDEX_MAPPER_SOURCE_MODE_SETTING.get(settings)
                 : mode.get();
             if (isDefault(sourceMode)) {
-                return resolveSourceMode(indexMode, sourceMode == null ? Mode.STORED : sourceMode, enableRecoverySource);
+                return resolveSourceMode(indexMode, sourceMode == null ? Mode.STORED : sourceMode);
 
             }
             if (supportsNonDefaultParameterValues == false) {
@@ -368,8 +278,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
                 enabled.get(),
                 includes.getValue().toArray(Strings.EMPTY_ARRAY),
                 excludes.getValue().toArray(Strings.EMPTY_ARRAY),
-                indexMode,
-                enableRecoverySource
+                indexMode
             );
             if (indexMode != null) {
                 indexMode.validateSourceFieldMapper(sourceFieldMapper);
@@ -379,16 +288,16 @@ public class SourceFieldMapper extends MetadataFieldMapper {
 
     }
 
-    private static SourceFieldMapper resolveSourceMode(final IndexMode indexMode, final Mode sourceMode, boolean enableRecoverySource) {
+    private static SourceFieldMapper resolveSourceMode(final IndexMode indexMode, final Mode sourceMode) {
         switch (indexMode) {
             case STANDARD:
                 switch (sourceMode) {
                     case SYNTHETIC:
-                        return enableRecoverySource ? DEFAULT_SYNTHETIC : DEFAULT_SYNTHETIC_NO_RECOVERY_SOURCE;
+                        return DEFAULT_SYNTHETIC;
                     case STORED:
-                        return enableRecoverySource ? DEFAULT : DEFAULT_NO_RECOVERY_SOURCE;
+                        return DEFAULT;
                     case DISABLED:
-                        return enableRecoverySource ? DEFAULT_DISABLED : DEFAULT_DISABLED_NO_RECOVERY_SOURCE;
+                        return DEFAULT_DISABLED;
                     default:
                         throw new IllegalArgumentException("Unsupported source mode: " + sourceMode);
                 }
@@ -396,15 +305,9 @@ public class SourceFieldMapper extends MetadataFieldMapper {
             case LOGSDB:
                 switch (sourceMode) {
                     case SYNTHETIC:
-                        return enableRecoverySource
-                            ? (indexMode == IndexMode.TIME_SERIES ? TSDB_DEFAULT : LOGSDB_DEFAULT)
-                            : (indexMode == IndexMode.TIME_SERIES ? TSDB_DEFAULT_NO_RECOVERY_SOURCE : LOGSDB_DEFAULT_NO_RECOVERY_SOURCE);
+                        return indexMode == IndexMode.TIME_SERIES ? TSDB_DEFAULT : LOGSDB_DEFAULT;
                     case STORED:
-                        return enableRecoverySource
-                            ? (indexMode == IndexMode.TIME_SERIES ? TSDB_DEFAULT_STORED : LOGSDB_DEFAULT_STORED)
-                            : (indexMode == IndexMode.TIME_SERIES
-                                ? TSDB_DEFAULT_NO_RECOVERY_SOURCE_STORED
-                                : LOGSDB_DEFAULT_NO_RECOVERY_SOURCE_STORED);
+                        return indexMode == IndexMode.TIME_SERIES ? TSDB_DEFAULT_STORED : LOGSDB_DEFAULT_STORED;
                     case DISABLED:
                         throw new IllegalArgumentException("_source can not be disabled in index using [" + indexMode + "] index mode");
                     default:
@@ -417,21 +320,19 @@ public class SourceFieldMapper extends MetadataFieldMapper {
 
     public static final TypeParser PARSER = new ConfigurableTypeParser(c -> {
         final IndexMode indexMode = c.getIndexSettings().getMode();
-        boolean enableRecoverySource = INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(c.getSettings());
         final Mode settingSourceMode = INDEX_MAPPER_SOURCE_MODE_SETTING.get(c.getSettings());
 
         if (indexMode.isSyntheticSourceEnabled()) {
             if (indexMode == IndexMode.TIME_SERIES && c.getIndexSettings().getIndexVersionCreated().before(IndexVersions.V_8_7_0)) {
-                return enableRecoverySource ? TSDB_LEGACY_DEFAULT : TSDB_LEGACY_DEFAULT_NO_RECOVERY_SOURCE;
+                return TSDB_LEGACY_DEFAULT;
             }
         }
-        return resolveSourceMode(indexMode, settingSourceMode == null ? Mode.STORED : settingSourceMode, enableRecoverySource);
+        return resolveSourceMode(indexMode, settingSourceMode == null ? Mode.STORED : settingSourceMode);
     },
         c -> new Builder(
             c.getIndexSettings().getMode(),
             c.getSettings(),
-            c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
-            INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(c.getSettings())
+            c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK)
         )
     );
 
@@ -484,16 +385,8 @@ public class SourceFieldMapper extends MetadataFieldMapper {
     private final SourceFilter sourceFilter;
 
     private final IndexMode indexMode;
-    private final boolean enableRecoverySource;
-
-    private SourceFieldMapper(
-        Mode mode,
-        Explicit<Boolean> enabled,
-        String[] includes,
-        String[] excludes,
-        IndexMode indexMode,
-        boolean enableRecoverySource
-    ) {
+
+    private SourceFieldMapper(Mode mode, Explicit<Boolean> enabled, String[] includes, String[] excludes, IndexMode indexMode) {
         super(new SourceFieldType((enabled.explicit() && enabled.value()) || (enabled.explicit() == false && mode != Mode.DISABLED)));
         assert enabled.explicit() == false || mode == null;
         this.mode = mode;
@@ -506,7 +399,6 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         }
         this.complete = stored() && sourceFilter == null;
         this.indexMode = indexMode;
-        this.enableRecoverySource = enableRecoverySource;
     }
 
     private static SourceFilter buildSourceFilter(String[] includes, String[] excludes) {
@@ -551,6 +443,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
             context.doc().add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
         }
 
+        boolean enableRecoverySource = context.indexSettings().isRecoverySourceEnabled();
         if (enableRecoverySource && originalSource != null && adaptedSource != originalSource) {
             // if we omitted source or modified it we add the _recovery_source to ensure we have it for ops based recovery
             BytesRef ref = originalSource.toBytesRef();
@@ -579,7 +472,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(indexMode, Settings.EMPTY, false, enableRecoverySource).init(this);
+        return new Builder(indexMode, Settings.EMPTY, false).init(this);
     }
 
     /**

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/DynamicFieldsBuilderTests.java

@@ -69,7 +69,7 @@ public class DynamicFieldsBuilderTests extends ESTestCase {
         XContentParser parser = createParser(JsonXContent.jsonXContent, source);
         SourceToParse sourceToParse = new SourceToParse("test", new BytesArray(source), XContentType.JSON);
 
-        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false, true).setSynthetic().build();
+        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false).setSynthetic().build();
         RootObjectMapper root = new RootObjectMapper.Builder("_doc", Optional.empty()).add(
             new PassThroughObjectMapper.Builder("labels").setPriority(0).setContainsDimensions().dynamic(ObjectMapper.Dynamic.TRUE)
         ).build(MapperBuilderContext.root(false, false));

+ 1 - 1
server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java

@@ -384,7 +384,7 @@ public class SearchExecutionContextTests extends ESTestCase {
 
     public void testSyntheticSourceSearchLookup() throws IOException {
         // Build a mapping using synthetic source
-        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false, true).setSynthetic().build();
+        SourceFieldMapper sourceMapper = new SourceFieldMapper.Builder(null, Settings.EMPTY, false).setSynthetic().build();
         RootObjectMapper root = new RootObjectMapper.Builder("_doc", Optional.empty()).add(
             new KeywordFieldMapper.Builder("cat", IndexVersion.current()).ignoreAbove(100)
         ).build(MapperBuilderContext.root(true, false));