Skip to content

Commit be85bfb

Browse files
salvatore-campagnasalvatore-campagna
authored
[8.x] Use synthetic recovery source by default if synthetic source is enabled (elastic#119110) (elastic#121760)
* Use synthetic recovery source by default if synthetic source is enabled (elastic#119110) We experimented with using synthetic source for recovery and observed quite positive impact on indexing throughput by means of our nightly Rally benchmarks. As a result, here we enable it by default when synthetic source is used. To be more precise, if `index.mapping.source.mode` setting is `synthetic` we enable recovery source by means of synthetic source. Moreover, enabling synthetic source recovery is done behind a feature flag. That would allow us to enable it in snapshot builds which in turn will allow us to see performance results in Rally nightly benchmarks.
1 parent 9ba91e6 commit be85bfb

File tree

9 files changed

+356
-28
lines changed

9 files changed

+356
-28
lines changed

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/update/100_synthetic_source.yml‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ setup:
66
---
77
keyword:
88
- requires:
9-
cluster_features: ["gte_v8.4.0"]
10-
reason: introduced in 8.4.0
9+
cluster_features: [ "mapper.synthetic_recovery_source" ]
10+
reason: requires synthetic recovery source
1111

1212
- do:
1313
indices.create:
@@ -60,13 +60,14 @@ keyword:
6060
index: test
6161
run_expensive_tasks: true
6262
- is_false: test.fields._source
63-
- is_true: test.fields._recovery_source
63+
# When synthetic source is used there is no _recovery_source field
64+
- match: { test.fields._recovery_source: null }
6465

6566
---
6667
stored text:
6768
- requires:
68-
cluster_features: ["gte_v8.5.0"]
69-
reason: introduced in 8.5.0
69+
cluster_features: [ "mapper.synthetic_recovery_source" ]
70+
reason: requires synthetic recovery source
7071

7172
- do:
7273
indices.create:
@@ -121,4 +122,5 @@ stored text:
121122
index: test
122123
run_expensive_tasks: true
123124
- is_false: test.fields._source
124-
- is_true: test.fields._recovery_source
125+
# When synthetic source is used there is no _recovery_source field
126+
- match: { test.fields._recovery_source: null }

‎server/src/main/java/org/elasticsearch/common/settings/Setting.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,15 @@ public static Setting<Boolean> boolSetting(String key, boolean defaultValue, Val
15861586
return new Setting<>(key, Boolean.toString(defaultValue), b -> parseBoolean(b, key, isFiltered(properties)), validator, properties);
15871587
}
15881588

1589+
public static Setting<Boolean> boolSetting(
1590+
String key,
1591+
Function<Settings, String> defaultValueFn,
1592+
Validator<Boolean> validator,
1593+
Property... properties
1594+
) {
1595+
return new Setting<>(key, defaultValueFn, b -> parseBoolean(b, key, isFiltered(properties)), validator, properties);
1596+
}
1597+
15891598
public static Setting<Boolean> boolSetting(String key, Function<Settings, String> defaultValueFn, Property... properties) {
15901599
return new Setting<>(key, defaultValueFn, b -> parseBoolean(b, key, isFiltered(properties)), properties);
15911600
}

‎server/src/main/java/org/elasticsearch/index/IndexSettings.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.time.DateUtils;
2525
import org.elasticsearch.common.unit.ByteSizeUnit;
2626
import org.elasticsearch.common.unit.ByteSizeValue;
27+
import org.elasticsearch.common.util.FeatureFlag;
2728
import org.elasticsearch.core.TimeValue;
2829
import org.elasticsearch.features.NodeFeature;
2930
import org.elasticsearch.index.mapper.IgnoredSourceFieldMapper;
@@ -40,6 +41,7 @@
4041
import java.util.List;
4142
import java.util.Locale;
4243
import java.util.Map;
44+
import java.util.Objects;
4345
import java.util.concurrent.TimeUnit;
4446
import java.util.function.Consumer;
4547
import java.util.function.Function;
@@ -725,9 +727,19 @@ public Iterator<Setting<?>> settings() {
725727
Setting.Property.IndexScope
726728
);
727729

730+
public static final FeatureFlag RECOVERY_USE_SYNTHETIC_SOURCE = new FeatureFlag("index_recovery_use_synthetic_source");
728731
public static final Setting<Boolean> RECOVERY_USE_SYNTHETIC_SOURCE_SETTING = Setting.boolSetting(
729732
"index.recovery.use_synthetic_source",
730-
false,
733+
settings -> {
734+
boolean isSyntheticSourceRecoveryFeatureFlagEnabled = RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled();
735+
boolean isNewIndexVersion = SETTING_INDEX_VERSION_CREATED.get(settings)
736+
.onOrAfter(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT_BACKPORT);
737+
boolean useSyntheticRecoverySource = isSyntheticSourceRecoveryFeatureFlagEnabled && isNewIndexVersion;
738+
return String.valueOf(
739+
useSyntheticRecoverySource
740+
&& Objects.equals(INDEX_MAPPER_SOURCE_MODE_SETTING.get(settings), SourceFieldMapper.Mode.SYNTHETIC)
741+
);
742+
},
731743
new Setting.Validator<>() {
732744
@Override
733745
public void validate(Boolean value) {}
@@ -1088,7 +1100,8 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
10881100
skipIgnoredSourceRead = scopedSettings.get(IgnoredSourceFieldMapper.SKIP_IGNORED_SOURCE_READ_SETTING);
10891101
indexMappingSourceMode = scopedSettings.get(INDEX_MAPPER_SOURCE_MODE_SETTING);
10901102
recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings);
1091-
recoverySourceSyntheticEnabled = scopedSettings.get(RECOVERY_USE_SYNTHETIC_SOURCE_SETTING);
1103+
recoverySourceSyntheticEnabled = DiscoveryNode.isStateless(nodeSettings) == false
1104+
&& scopedSettings.get(RECOVERY_USE_SYNTHETIC_SOURCE_SETTING);
10921105
if (recoverySourceSyntheticEnabled) {
10931106
if (DiscoveryNode.isStateless(settings)) {
10941107
throw new IllegalArgumentException("synthetic recovery source is only allowed in stateful");

‎server/src/main/java/org/elasticsearch/index/IndexVersions.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ private static IndexVersion def(int id, Version luceneVersion) {
126126
public static final IndexVersion UPGRADE_TO_LUCENE_9_12_1 = def(8_523_0_00, Version.LUCENE_9_12_1);
127127
public static final IndexVersion INFERENCE_METADATA_FIELDS_BACKPORT = def(8_524_0_00, Version.LUCENE_9_12_1);
128128
public static final IndexVersion LOGSB_OPTIONAL_SORTING_ON_HOST_NAME_BACKPORT = def(8_525_0_00, Version.LUCENE_9_12_1);
129+
public static final IndexVersion USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT_BACKPORT = def(8_526_0_00, Version.LUCENE_9_12_1);
129130
/*
130131
* STOP! READ THIS FIRST! No, really,
131132
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _

‎server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2455,8 +2455,14 @@ protected void validateRoundTripReader(String syntheticSource, DirectoryReader r
24552455
// and since the copy is exact, contents of ignored source are different.
24562456
assertReaderEquals(
24572457
"round trip " + syntheticSource,
2458-
new FieldMaskingReader(Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, IgnoredSourceFieldMapper.NAME), reader),
2459-
new FieldMaskingReader(Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, IgnoredSourceFieldMapper.NAME), roundTripReader)
2458+
new FieldMaskingReader(
2459+
Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, IgnoredSourceFieldMapper.NAME, SourceFieldMapper.RECOVERY_SOURCE_SIZE_NAME),
2460+
reader
2461+
),
2462+
new FieldMaskingReader(
2463+
Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, IgnoredSourceFieldMapper.NAME, SourceFieldMapper.RECOVERY_SOURCE_SIZE_NAME),
2464+
roundTripReader
2465+
)
24602466
);
24612467
}
24622468
}

‎server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,13 @@ public void testRecoverySourceWithSyntheticSource() throws IOException {
475475
MapperService mapperService = createMapperService(settings, topMapping(b -> {}));
476476
DocumentMapper docMapper = mapperService.documentMapper();
477477
ParsedDocument doc = docMapper.parse(source(b -> b.field("field1", "value1")));
478-
assertNotNull(doc.rootDoc().getField("_recovery_source"));
479-
assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"field1\":\"value1\"}")));
478+
if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
479+
// TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
480+
assertNotNull(doc.rootDoc().getField("_recovery_source"));
481+
assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"field1\":\"value1\"}")));
482+
} else {
483+
assertNull(doc.rootDoc().getField("_recovery_source"));
484+
}
480485
}
481486
{
482487
Settings settings = Settings.builder()
@@ -507,8 +512,16 @@ public void testRecoverySourceWithLogs() throws IOException {
507512
MapperService mapperService = createMapperService(settings, mapping(b -> {}));
508513
DocumentMapper docMapper = mapperService.documentMapper();
509514
ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
510-
assertNotNull(doc.rootDoc().getField("_recovery_source"));
511-
assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}")));
515+
if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
516+
// TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
517+
assertNotNull(doc.rootDoc().getField("_recovery_source"));
518+
assertThat(
519+
doc.rootDoc().getField("_recovery_source").binaryValue(),
520+
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}"))
521+
);
522+
} else {
523+
assertNull(doc.rootDoc().getField("_recovery_source"));
524+
}
512525
}
513526
{
514527
Settings settings = Settings.builder()
@@ -701,8 +714,16 @@ public void testRecoverySourceWithLogsCustom() throws IOException {
701714
MapperService mapperService = createMapperService(settings, mappings);
702715
DocumentMapper docMapper = mapperService.documentMapper();
703716
ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
704-
assertNotNull(doc.rootDoc().getField("_recovery_source"));
705-
assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}")));
717+
if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
718+
// TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
719+
assertNotNull(doc.rootDoc().getField("_recovery_source"));
720+
assertThat(
721+
doc.rootDoc().getField("_recovery_source").binaryValue(),
722+
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}"))
723+
);
724+
} else {
725+
assertNull(doc.rootDoc().getField("_recovery_source"));
726+
}
706727
}
707728
{
708729
Settings settings = Settings.builder()
@@ -728,11 +749,16 @@ public void testRecoverySourceWithTimeSeries() throws IOException {
728749
}));
729750
DocumentMapper docMapper = mapperService.documentMapper();
730751
ParsedDocument doc = docMapper.parse(source("123", b -> b.field("@timestamp", "2012-02-13").field("field", "value1"), null));
731-
assertNotNull(doc.rootDoc().getField("_recovery_source"));
732-
assertThat(
733-
doc.rootDoc().getField("_recovery_source").binaryValue(),
734-
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
735-
);
752+
if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
753+
// TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
754+
assertNotNull(doc.rootDoc().getField("_recovery_source"));
755+
assertThat(
756+
doc.rootDoc().getField("_recovery_source").binaryValue(),
757+
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
758+
);
759+
} else {
760+
assertNull(doc.rootDoc().getField("_recovery_source"));
761+
}
736762
}
737763
{
738764
Settings settings = Settings.builder()
@@ -776,11 +802,16 @@ public void testRecoverySourceWithTimeSeriesCustom() throws IOException {
776802
MapperService mapperService = createMapperService(settings, mappings);
777803
DocumentMapper docMapper = mapperService.documentMapper();
778804
ParsedDocument doc = docMapper.parse(source("123", b -> b.field("@timestamp", "2012-02-13").field("field", "value1"), null));
779-
assertNotNull(doc.rootDoc().getField("_recovery_source"));
780-
assertThat(
781-
doc.rootDoc().getField("_recovery_source").binaryValue(),
782-
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
783-
);
805+
if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
806+
// TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
807+
assertNotNull(doc.rootDoc().getField("_recovery_source"));
808+
assertThat(
809+
doc.rootDoc().getField("_recovery_source").binaryValue(),
810+
equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
811+
);
812+
} else {
813+
assertNull(doc.rootDoc().getField("_recovery_source"));
814+
}
784815
}
785816
{
786817
Settings settings = Settings.builder()

‎test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -884,8 +884,11 @@ protected void validateRoundTripReader(String syntheticSource, DirectoryReader r
884884
throws IOException {
885885
assertReaderEquals(
886886
"round trip " + syntheticSource,
887-
new FieldMaskingReader(SourceFieldMapper.RECOVERY_SOURCE_NAME, reader),
888-
new FieldMaskingReader(SourceFieldMapper.RECOVERY_SOURCE_NAME, roundTripReader)
887+
new FieldMaskingReader(Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, SourceFieldMapper.RECOVERY_SOURCE_SIZE_NAME), reader),
888+
new FieldMaskingReader(
889+
Set.of(SourceFieldMapper.RECOVERY_SOURCE_NAME, SourceFieldMapper.RECOVERY_SOURCE_SIZE_NAME),
890+
roundTripReader
891+
)
889892
);
890893
}
891894

‎x-pack/plugin/logsdb/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,5 @@ tasks.named("javaRestTest").configure {
4444
tasks.named('yamlRestTest') {
4545
usesDefaultDistribution()
4646
}
47+
48+

0 commit comments

Comments
 (0)