Skip to content

Commit

Permalink
Reduce the number of SFM singletons. (elastic#114969)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@martijnvg
martijnvg committed Oct 17, 2024
1 parent 918e8c4 commit c8d70e9
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 133 deletions.
11 changes: 11 additions & 0 deletions server/src/main/java/org/elasticsearch/index/IndexSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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;

Expand Down Expand Up @@ -817,6 +818,7 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
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.
Expand Down Expand Up @@ -979,6 +981,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
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,
Expand Down Expand Up @@ -1674,6 +1677,14 @@ public SourceFieldMapper.Mode getIndexMappingSourceMode() {
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.
Expand Down
155 changes: 24 additions & 131 deletions server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
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(
Expand Down Expand Up @@ -88,125 +86,55 @@ public enum Mode {
Explicit.IMPLICIT_TRUE,
Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY,
null,
true
null
);

private static final SourceFieldMapper DEFAULT_DISABLED = new SourceFieldMapper(
Mode.DISABLED,
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(
Mode.SYNTHETIC,
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(
Mode.SYNTHETIC,
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(
Mode.STORED,
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(
Mode.SYNTHETIC,
Explicit.IMPLICIT_TRUE,
Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY,
IndexMode.LOGSDB,
true
IndexMode.LOGSDB
);

private static final SourceFieldMapper LOGSDB_DEFAULT_STORED = new SourceFieldMapper(
Mode.STORED,
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
);

/*
Expand All @@ -218,17 +146,7 @@ public enum Mode {
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 {
Expand Down Expand Up @@ -289,20 +207,12 @@ public static class Builder extends MetadataFieldMapper.Builder {

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() {
Expand Down Expand Up @@ -337,7 +247,7 @@ public SourceFieldMapper build() {
? 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) {
Expand Down Expand Up @@ -368,8 +278,7 @@ public SourceFieldMapper build() {
enabled.get(),
includes.getValue().toArray(Strings.EMPTY_ARRAY),
excludes.getValue().toArray(Strings.EMPTY_ARRAY),
indexMode,
enableRecoverySource
indexMode
);
if (indexMode != null) {
indexMode.validateSourceFieldMapper(sourceFieldMapper);
Expand All @@ -379,32 +288,26 @@ public SourceFieldMapper build() {

}

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);
}
case TIME_SERIES:
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:
Expand All @@ -417,21 +320,19 @@ private static SourceFieldMapper resolveSourceMode(final IndexMode indexMode, fi

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)
)
);

Expand Down Expand Up @@ -484,16 +385,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
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;
Expand All @@ -506,7 +399,6 @@ private SourceFieldMapper(
}
this.complete = stored() && sourceFilter == null;
this.indexMode = indexMode;
this.enableRecoverySource = enableRecoverySource;
}

private static SourceFilter buildSourceFilter(String[] includes, String[] excludes) {
Expand Down Expand Up @@ -551,6 +443,7 @@ public void preParse(DocumentParserContext context) throws IOException {
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();
Expand Down Expand Up @@ -579,7 +472,7 @@ protected String contentType() {

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(indexMode, Settings.EMPTY, false, enableRecoverySource).init(this);
return new Builder(indexMode, Settings.EMPTY, false).init(this);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/mapper/DynamicFieldsBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void testCreateDynamicStringFieldAsKeywordForDimension() throws IOExcepti
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));
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ public void testSearchRequestRuntimeFieldsAndMultifieldDetection() {

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));
Expand Down

0 comments on commit c8d70e9

Please sign in to comment.