Handle empty index case in LuceneSyntheticSourceChangesSnapshot (#118996

) In case of synthetic recovery source when the mapping is empty. A test that reproduces failure in #118955 consistently with a potential fix. `MapperService#updateMapping(...)` doesn't set the mapper field if a mapping has no fields, which is what is used in InternalEngine#newChangesSnapshot(...) . This happens when `newMappingMetadata` variable in `MapperService updateMapping(...)` is `null`. Causing an assertion to trip. This change adjusts that assertion to handle an empty index. Closes #118955
elastic · Dec 19, 2024 · 252f66d · 252f66d
1 parent e15b9b9
commit 252f66d
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 11 deletions.
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -288,9 +288,6 @@ tests:
 - class: org.elasticsearch.cluster.service.MasterServiceTests
   method: testThreadContext
   issue: https://github.com/elastic/elasticsearch/issues/118914
-- class: org.elasticsearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT
-  method: test {yaml=indices.create/20_synthetic_source/create index with use_synthetic_source}
-  issue: https://github.com/elastic/elasticsearch/issues/118955
 - class: org.elasticsearch.repositories.blobstore.testkit.analyze.SecureHdfsRepositoryAnalysisRestIT
   issue: https://github.com/elastic/elasticsearch/issues/118970
 - class: org.elasticsearch.xpack.security.authc.AuthenticationServiceTests

diff --git a/...spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml b/...spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml
@@ -2036,14 +2036,12 @@ create index with use_synthetic_source:
   - is_true: test.settings.index.recovery.use_synthetic_source
 
   - do:
-      bulk:
+      index:
         index: test
+        id: 1
         refresh: true
-        body:
-          - '{ "create": { } }'
-          - '{ "field": "aaaa" }'
-          - '{ "create": { } }'
-          - '{ "field": "bbbb" }'
+        body: { foo: bar }
+  - match: { _version: 1 }
 
   - do:
       indices.disk_usage:

diff --git a/...ec/src/yamlRestTest/resources/rest-api-spec/test/indices.recovery/20_synthetic_source.yml b/...ec/src/yamlRestTest/resources/rest-api-spec/test/indices.recovery/20_synthetic_source.yml
@@ -0,0 +1,33 @@
+---
+test recovery empty index with use_synthetic_source:
+  - requires:
+      cluster_features: ["mapper.synthetic_recovery_source"]
+      reason: requires synthetic recovery source
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          settings:
+            index:
+              number_of_replicas: 0
+              recovery:
+                use_synthetic_source: true
+              mapping:
+                source:
+                  mode: synthetic
+
+  - do:
+      indices.get_settings: {}
+  - match: { test.settings.index.mapping.source.mode: synthetic}
+  - is_true: test.settings.index.recovery.use_synthetic_source
+
+  - do:
+      indices.put_settings:
+        index: test
+        body:
+          index.number_of_replicas: 1
+
+  - do:
+      cluster.health:
+        wait_for_events: languid
diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java
@@ -1049,6 +1049,9 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings);
         recoverySourceSyntheticEnabled = scopedSettings.get(RECOVERY_USE_SYNTHETIC_SOURCE_SETTING);
         if (recoverySourceSyntheticEnabled) {
+            if (DiscoveryNode.isStateless(settings)) {
+                throw new IllegalArgumentException("synthetic recovery source is only allowed in stateful");
+            }
             // Verify that all nodes can handle this setting
             if (version.before(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY)
                 && version.between(

diff --git a/...er/src/main/java/org/elasticsearch/index/engine/LuceneSyntheticSourceChangesSnapshot.java b/...er/src/main/java/org/elasticsearch/index/engine/LuceneSyntheticSourceChangesSnapshot.java
@@ -77,12 +77,13 @@ public LuceneSyntheticSourceChangesSnapshot(
         IndexVersion indexVersionCreated
     ) throws IOException {
         super(engineSearcher, searchBatchSize, fromSeqNo, toSeqNo, requiredFullRange, accessStats, indexVersionCreated);
-        assert mappingLookup.isSourceSynthetic();
+        // a MapperService#updateMapping(...) of empty index may not have been invoked and then mappingLookup is empty
+        assert engineSearcher.getDirectoryReader().maxDoc() == 0 || mappingLookup.isSourceSynthetic()
+            : "either an empty index or synthetic source must be enabled for proper functionality.";
         // ensure we can buffer at least one document
         this.maxMemorySizeInBytes = maxMemorySizeInBytes > 0 ? maxMemorySizeInBytes : 1;
         this.sourceLoader = mappingLookup.newSourceLoader(null, SourceFieldMetrics.NOOP);
         Set<String> storedFields = sourceLoader.requiredStoredFields();
-        assert mappingLookup.isSourceSynthetic() : "synthetic source must be enabled for proper functionality.";
         this.storedFieldLoader = StoredFieldLoader.create(false, storedFields);
         this.lastSeenSeqNo = fromSeqNo - 1;
     }