From d6ca8a03842c2b22302c67e9cf00f57cff29f73c Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 5 Nov 2024 10:32:14 +0100 Subject: [PATCH] Only create MapperService in SyntheticSourceIndexSettingsProvider when required (#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 #116070 --- .../SyntheticSourceIndexSettingsProvider.java | 17 +++++++++++- ...heticSourceIndexSettingsProviderTests.java | 27 +++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java index e7572d6a646e1..e87f10ec19916 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java +++ b/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; } diff --git a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java index 2ab77b38b3373..2d8723a0d8c25 100644 --- a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java +++ b/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)); } }