Only create MapperService in SyntheticSourceIndexSettingsProvider when required (elastic#116075)

In case the index.mapping.source.mode is specified, there is no need to create a mapper service to determine whether synthetic source is used.

In case of logsdb/tsdb there is also no reason to create a mapper service. If _source.mode attribute is specified, then it doesn't really matter whether what its value is for the SyntheticSourceIndexSettingsProvider. If it is synthetic, then that is the same as the index mode's default source mode. If it is stored, we just will add set index.mapping.source.mode to stored, which has no effect. And disabled source mode isn't allowed in the case of logsdb and tsdb.

Closes elastic#116070

Author: martijnvg

x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java

@@ -101,6 +101,20 @@ boolean newIndexHasSyntheticSourceUsage(
 
         try {
             var tmpIndexMetadata = buildIndexMetadataForMapperService(indexName, templateIndexMode, indexTemplateAndCreateRequestSettings);
+            var indexMode = tmpIndexMetadata.getIndexMode();
+            if (SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.exists(tmpIndexMetadata.getSettings())
+                || indexMode == IndexMode.LOGSDB
+                || indexMode == IndexMode.TIME_SERIES) {
+                // In case when index mode is tsdb or logsdb and only _source.mode mapping attribute is specified, then the default
+                // could be wrong. However, it doesn't really matter, because if the _source.mode mapping attribute is set to stored,
+                // then configuring the index.mapping.source.mode setting to stored has no effect. Additionally _source.mode can't be set
+                // to disabled, because that isn't allowed with logsdb/tsdb. In other words setting index.mapping.source.mode setting to
+                // stored when _source.mode mapping attribute is stored is fine as it has no effect, but avoids creating MapperService.
+                var sourceMode = SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.get(tmpIndexMetadata.getSettings());
+                return sourceMode == SourceFieldMapper.Mode.SYNTHETIC;
+            }
+
+            // TODO: remove this when _source.mode attribute has been removed:
             try (var mapperService = mapperServiceFactory.apply(tmpIndexMetadata)) {
                 // combinedTemplateMappings can be null when creating system indices
                 // combinedTemplateMappings can be empty when creating a normal index that doesn't match any template and without mapping.
@@ -112,7 +126,8 @@ boolean newIndexHasSyntheticSourceUsage(
             }
         } catch (AssertionError | Exception e) {
             // In case invalid mappings or setting are provided, then mapper service creation can fail.
-            // In that case it is ok to return false here. The index creation will fail anyway later, so need to fallback to stored source.
+            // In that case it is ok to return false here. The index creation will fail anyway later, so no need to fallback to stored
+            // source.
             LOGGER.info(() -> Strings.format("unable to create mapper service for index [%s]", indexName), e);
             return false;
         }

x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java

@@ -24,6 +24,7 @@
 import java.io.IOException;
 import java.time.Instant;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.elasticsearch.common.settings.Settings.builder;
 import static org.hamcrest.Matchers.equalTo;
@@ -35,6 +36,7 @@ public class SyntheticSourceIndexSettingsProviderTests extends ESTestCase {
 
     private SyntheticSourceLicenseService syntheticSourceLicenseService;
     private SyntheticSourceIndexSettingsProvider provider;
+    private final AtomicInteger newMapperServiceCounter = new AtomicInteger();
 
     private static LogsdbIndexModeSettingsProvider getLogsdbIndexModeSettingsProvider(boolean enabled) {
         return new LogsdbIndexModeSettingsProvider(Settings.builder().put("cluster.logsdb.enabled", enabled).build());
@@ -49,11 +51,11 @@ public void setup() {
         syntheticSourceLicenseService = new SyntheticSourceLicenseService(Settings.EMPTY);
         syntheticSourceLicenseService.setLicenseState(licenseState);
 
-        provider = new SyntheticSourceIndexSettingsProvider(
-            syntheticSourceLicenseService,
-            im -> MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), im.getSettings(), im.getIndex().getName()),
-            getLogsdbIndexModeSettingsProvider(false)
-        );
+        provider = new SyntheticSourceIndexSettingsProvider(syntheticSourceLicenseService, im -> {
+            newMapperServiceCounter.incrementAndGet();
+            return MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), im.getSettings(), im.getIndex().getName());
+        }, getLogsdbIndexModeSettingsProvider(false));
+        newMapperServiceCounter.set(0);
     }
 
     public void testNewIndexHasSyntheticSourceUsage() throws IOException {
@@ -77,6 +79,7 @@ public void testNewIndexHasSyntheticSourceUsage() throws IOException {
                 """;
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of(new CompressedXContent(mapping)));
             assertTrue(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(1));
         }
         {
             String mapping;
@@ -110,6 +113,7 @@ public void testNewIndexHasSyntheticSourceUsage() throws IOException {
             }
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of(new CompressedXContent(mapping)));
             assertFalse(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(2));
         }
     }
 
@@ -152,15 +156,18 @@ public void testNewIndexHasSyntheticSourceUsageLogsdbIndex() throws IOException
             Settings settings = Settings.builder().put("index.mode", "logsdb").build();
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of(new CompressedXContent(mapping)));
             assertTrue(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(0));
         }
         {
             Settings settings = Settings.builder().put("index.mode", "logsdb").build();
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of());
             assertTrue(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(0));
         }
         {
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, Settings.EMPTY, List.of());
             assertFalse(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(1));
         }
         {
             boolean result = provider.newIndexHasSyntheticSourceUsage(
@@ -170,6 +177,7 @@ public void testNewIndexHasSyntheticSourceUsageLogsdbIndex() throws IOException
                 List.of(new CompressedXContent(mapping))
             );
             assertFalse(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(2));
         }
     }
 
@@ -234,6 +242,7 @@ public void testNewIndexHasSyntheticSourceUsage_invalidSettings() throws IOExcep
                 """;
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of(new CompressedXContent(mapping)));
             assertFalse(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(1));
         }
         {
             String mapping = """
@@ -249,6 +258,7 @@ public void testNewIndexHasSyntheticSourceUsage_invalidSettings() throws IOExcep
                 """;
             boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, null, settings, List.of(new CompressedXContent(mapping)));
             assertFalse(result);
+            assertThat(newMapperServiceCounter.get(), equalTo(2));
         }
     }
 
@@ -278,6 +288,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSource() throws
             List.of()
         );
         assertThat(result.size(), equalTo(0));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
 
         syntheticSourceLicenseService.setSyntheticSourceFallback(true);
         result = provider.getAdditionalIndexSettings(
@@ -291,6 +302,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSource() throws
         );
         assertThat(result.size(), equalTo(1));
         assertEquals(SourceFieldMapper.Mode.STORED, SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.get(result));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
 
         result = provider.getAdditionalIndexSettings(
             DataStream.getDefaultBackingIndexName(dataStreamName, 2),
@@ -303,6 +315,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSource() throws
         );
         assertThat(result.size(), equalTo(1));
         assertEquals(SourceFieldMapper.Mode.STORED, SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.get(result));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
 
         result = provider.getAdditionalIndexSettings(
             DataStream.getDefaultBackingIndexName(dataStreamName, 2),
@@ -315,6 +328,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSource() throws
         );
         assertThat(result.size(), equalTo(1));
         assertEquals(SourceFieldMapper.Mode.STORED, SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.get(result));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
     }
 
     public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSourceFileMatch() throws IOException {
@@ -347,6 +361,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSourceFileMatch(
             List.of()
         );
         assertThat(result.size(), equalTo(0));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
 
         dataStreamName = "logs-app1-0";
         mb = Metadata.builder(
@@ -371,6 +386,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSourceFileMatch(
         );
         assertThat(result.size(), equalTo(1));
         assertEquals(SourceFieldMapper.Mode.STORED, SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.get(result));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
 
         result = provider.getAdditionalIndexSettings(
             DataStream.getDefaultBackingIndexName(dataStreamName, 2),
@@ -382,5 +398,6 @@ public void testGetAdditionalIndexSettingsDowngradeFromSyntheticSourceFileMatch(
             List.of()
         );
         assertThat(result.size(), equalTo(0));
+        assertThat(newMapperServiceCounter.get(), equalTo(0));
     }
 }