Browse Source

Remove synthetic recovery source feature flag. (#122615)

This feature flag controls whether synthetic recovery source is enabled by default when the source mode is synthetic.

The synthetic recovery source feature itself is already available via the index.recovery.use_synthetic_source index setting and can be enabled by anyone using synthetic source.

The default value of index.recovery.use_synthetic_source setting defaults to true when index.mapping.source.mode is enabled. The index.mapping.source.mode default to true if index.mode is logsdb or time_series.

In other words, with this change synthetic recovery source will be enabled by default for logsdb and tsdb.

Closes #116726
Martijn van Groningen 7 months ago
parent
commit
26de5343a2

+ 8 - 0
docs/changelog/122615.yaml

@@ -0,0 +1,8 @@
+pr: 122615
+summary: Enable synthetic recovery source by default when synthetic source is enabled.
+  Using synthetic recovery source significantly improves indexing performance compared
+  to regular recovery source.
+area: Mapping
+type: enhancement
+issues:
+ - 116726

+ 0 - 1
qa/smoke-test-multinode/src/yamlRestTest/java/org/elasticsearch/smoketest/SmokeTestMultiNodeClientYamlTestSuiteIT.java

@@ -36,7 +36,6 @@ public class SmokeTestMultiNodeClientYamlTestSuiteIT extends ESClientYamlSuiteTe
         .node(0, n -> n.setting("node.roles", "[master,data,ml,remote_cluster_client,transform]"))
         .feature(FeatureFlag.TIME_SERIES_MODE)
         .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED)
-        .feature(FeatureFlag.INDEX_RECOVERY_USE_SYNTHETIC_SOURCE)
         .feature(FeatureFlag.DOC_VALUES_SKIPPER)
         .build();
 

+ 0 - 1
rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java

@@ -36,7 +36,6 @@ public class ClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase {
         .module("data-streams")
         .feature(FeatureFlag.TIME_SERIES_MODE)
         .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED)
-        .feature(FeatureFlag.INDEX_RECOVERY_USE_SYNTHETIC_SOURCE)
         .feature(FeatureFlag.DOC_VALUES_SKIPPER)
         .build();
 

+ 1 - 5
server/src/main/java/org/elasticsearch/index/IndexSettings.java

@@ -732,19 +732,15 @@ public final class IndexSettings {
         Setting.Property.ServerlessPublic
     );
 
-    public static final FeatureFlag RECOVERY_USE_SYNTHETIC_SOURCE = new FeatureFlag("index_recovery_use_synthetic_source");
     public static final Setting<Boolean> RECOVERY_USE_SYNTHETIC_SOURCE_SETTING = Setting.boolSetting(
         "index.recovery.use_synthetic_source",
         settings -> {
-            boolean isSyntheticSourceRecoveryFeatureFlagEnabled = RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled();
             boolean isNewIndexVersion = SETTING_INDEX_VERSION_CREATED.get(settings)
                 .onOrAfter(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT);
             boolean isIndexVersionInBackportRange = SETTING_INDEX_VERSION_CREATED.get(settings)
                 .between(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT_BACKPORT, IndexVersions.UPGRADE_TO_LUCENE_10_0_0);
 
-            boolean useSyntheticRecoverySource = isSyntheticSourceRecoveryFeatureFlagEnabled
-                && (isNewIndexVersion || isIndexVersionInBackportRange);
-
+            boolean useSyntheticRecoverySource = isNewIndexVersion || isIndexVersionInBackportRange;
             return String.valueOf(
                 useSyntheticRecoverySource
                     && Objects.equals(INDEX_MAPPER_SOURCE_MODE_SETTING.get(settings), SourceFieldMapper.Mode.SYNTHETIC)

+ 3 - 20
server/src/test/java/org/elasticsearch/index/mapper/NativeArrayIntegrationTestCase.java

@@ -20,7 +20,6 @@ import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.support.WriteRequest;
 import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.query.IdsQueryBuilder;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.xcontent.XContentBuilder;
@@ -35,7 +34,6 @@ import java.util.Set;
 
 import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
@@ -136,14 +134,7 @@ public abstract class NativeArrayIntegrationTestCase extends ESSingleNodeTestCas
                 var document = reader.storedFields().document(i);
                 // Verify that there is no ignored source:
                 Set<String> storedFieldNames = new LinkedHashSet<>(document.getFields().stream().map(IndexableField::name).toList());
-                if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled()) {
-                    assertThat(storedFieldNames, contains(expectedStoredFields));
-                } else {
-                    var copyExpectedStoredFields = new String[expectedStoredFields.length + 1];
-                    System.arraycopy(expectedStoredFields, 0, copyExpectedStoredFields, 0, expectedStoredFields.length);
-                    copyExpectedStoredFields[copyExpectedStoredFields.length - 1] = "_recovery_source";
-                    assertThat(storedFieldNames, containsInAnyOrder(copyExpectedStoredFields));
-                }
+                assertThat(storedFieldNames, contains(expectedStoredFields));
             }
             var fieldInfo = FieldInfos.getMergedFieldInfos(reader).fieldInfo("field.offsets");
             assertThat(fieldInfo.getDocValuesType(), equalTo(DocValuesType.SORTED));
@@ -208,11 +199,7 @@ public abstract class NativeArrayIntegrationTestCase extends ESSingleNodeTestCas
                 var document = reader.storedFields().document(i);
                 // Verify that there is ignored source because of leaf array being wrapped by object array:
                 List<String> storedFieldNames = document.getFields().stream().map(IndexableField::name).toList();
-                if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled()) {
-                    assertThat(storedFieldNames, contains("_id", "_ignored_source"));
-                } else {
-                    assertThat(storedFieldNames, containsInAnyOrder("_id", "_ignored_source", "_recovery_source"));
-                }
+                assertThat(storedFieldNames, contains("_id", "_ignored_source"));
 
                 // Verify that there is no offset field:
                 LeafReader leafReader = reader.leaves().get(0).reader();
@@ -285,11 +272,7 @@ public abstract class NativeArrayIntegrationTestCase extends ESSingleNodeTestCas
                 var document = reader.storedFields().document(i);
                 // Verify that there is no ignored source:
                 Set<String> storedFieldNames = new LinkedHashSet<>(document.getFields().stream().map(IndexableField::name).toList());
-                if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled()) {
-                    assertThat(storedFieldNames, contains("_id"));
-                } else {
-                    assertThat(storedFieldNames, containsInAnyOrder("_id", "_recovery_source"));
-                }
+                assertThat(storedFieldNames, contains("_id"));
             }
             var fieldInfo = FieldInfos.getMergedFieldInfos(reader).fieldInfo("object.field.offsets");
             assertThat(fieldInfo.getDocValuesType(), equalTo(DocValuesType.SORTED));

+ 6 - 47
server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java

@@ -489,13 +489,7 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             MapperService mapperService = createMapperService(settings, topMapping(b -> {}));
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> b.field("field1", "value1")));
-            if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
-                // TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
-                assertNotNull(doc.rootDoc().getField("_recovery_source"));
-                assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"field1\":\"value1\"}")));
-            } else {
-                assertNull(doc.rootDoc().getField("_recovery_source"));
-            }
+            assertNull(doc.rootDoc().getField("_recovery_source"));
         }
         {
             Settings settings = Settings.builder()
@@ -526,16 +520,8 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             MapperService mapperService = createMapperService(settings, mapping(b -> {}));
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
-            if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
-                // TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
-                assertNotNull(doc.rootDoc().getField("_recovery_source"));
-                assertThat(
-                    doc.rootDoc().getField("_recovery_source").binaryValue(),
-                    equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}"))
-                );
-            } else {
-                assertNull(doc.rootDoc().getField("_recovery_source"));
-            }
+            assertNotNull(doc.rootDoc().getField("_recovery_source_size"));
+            assertThat(doc.rootDoc().getField("_recovery_source_size").numericValue(), equalTo(27L));
         }
         {
             Settings settings = Settings.builder()
@@ -728,16 +714,7 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             MapperService mapperService = createMapperService(settings, mappings);
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
-            if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
-                // TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
-                assertNotNull(doc.rootDoc().getField("_recovery_source"));
-                assertThat(
-                    doc.rootDoc().getField("_recovery_source").binaryValue(),
-                    equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}"))
-                );
-            } else {
-                assertNull(doc.rootDoc().getField("_recovery_source"));
-            }
+            assertNull(doc.rootDoc().getField("_recovery_source"));
         }
         {
             Settings settings = Settings.builder()
@@ -763,16 +740,7 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             }));
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source("123", b -> b.field("@timestamp", "2012-02-13").field("field", "value1"), null));
-            if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
-                // TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
-                assertNotNull(doc.rootDoc().getField("_recovery_source"));
-                assertThat(
-                    doc.rootDoc().getField("_recovery_source").binaryValue(),
-                    equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
-                );
-            } else {
-                assertNull(doc.rootDoc().getField("_recovery_source"));
-            }
+            assertNull(doc.rootDoc().getField("_recovery_source"));
         }
         {
             Settings settings = Settings.builder()
@@ -816,16 +784,7 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             MapperService mapperService = createMapperService(settings, mappings);
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source("123", b -> b.field("@timestamp", "2012-02-13").field("field", "value1"), null));
-            if (IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE.isEnabled() == false) {
-                // TODO: remove this if branch when removing the 'index_recovery_use_synthetic_source' feature flag
-                assertNotNull(doc.rootDoc().getField("_recovery_source"));
-                assertThat(
-                    doc.rootDoc().getField("_recovery_source").binaryValue(),
-                    equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
-                );
-            } else {
-                assertNull(doc.rootDoc().getField("_recovery_source"));
-            }
+            assertNull(doc.rootDoc().getField("_recovery_source"));
         }
         {
             Settings settings = Settings.builder()

+ 0 - 5
test/test-clusters/src/main/java/org/elasticsearch/test/cluster/FeatureFlag.java

@@ -19,11 +19,6 @@ public enum FeatureFlag {
     TIME_SERIES_MODE("es.index_mode_feature_flag_registered=true", Version.fromString("8.0.0"), null),
     FAILURE_STORE_ENABLED("es.failure_store_feature_flag_enabled=true", Version.fromString("8.12.0"), null),
     SUB_OBJECTS_AUTO_ENABLED("es.sub_objects_auto_feature_flag_enabled=true", Version.fromString("8.16.0"), null),
-    INDEX_RECOVERY_USE_SYNTHETIC_SOURCE(
-        "es.index_recovery_use_synthetic_source_feature_flag_enabled=true",
-        Version.fromString("8.18.0"),
-        null
-    ),
     DOC_VALUES_SKIPPER("es.doc_values_skipper_feature_flag_enabled=true", Version.fromString("8.18.1"), null);
 
     public final String systemProperty;

+ 0 - 1
x-pack/plugin/logsdb/src/yamlRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbTestSuiteIT.java

@@ -24,7 +24,6 @@ public class LogsdbTestSuiteIT extends ESClientYamlSuiteTestCase {
         .distribution(DistributionType.DEFAULT)
         .setting("xpack.security.enabled", "false")
         .setting("xpack.license.self_generated.type", "trial")
-        .feature(FeatureFlag.INDEX_RECOVERY_USE_SYNTHETIC_SOURCE)
         .feature(FeatureFlag.DOC_VALUES_SKIPPER)
         .build();
 

+ 0 - 1
x-pack/qa/core-rest-tests-with-security/src/yamlRestTest/java/org/elasticsearch/xpack/security/CoreWithSecurityClientYamlTestSuiteIT.java

@@ -50,7 +50,6 @@ public class CoreWithSecurityClientYamlTestSuiteIT extends ESClientYamlSuiteTest
         .user(USER, PASS)
         .feature(FeatureFlag.TIME_SERIES_MODE)
         .feature(FeatureFlag.SUB_OBJECTS_AUTO_ENABLED)
-        .feature(FeatureFlag.INDEX_RECOVERY_USE_SYNTHETIC_SOURCE)
         .feature(FeatureFlag.DOC_VALUES_SKIPPER)
         .build();