From 2fc2d0546676b30acc3c31bc770482133c4689d3 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 28 Sep 2024 22:49:59 -0700 Subject: [PATCH] Revert "synthetic source index setting provider should check source field mapper (#113522) (#113717)" (#113746) Revert "synthetic source index setting provider should check source field mapper (#113522) (#113717)" This reverts commit c7e2b28. --- .../xpack/logsdb/LogsdbRestIT.java | 26 +-- .../xpack/logsdb/LogsdbRestIT.java | 26 +-- .../xpack/logsdb/LogsDBPlugin.java | 5 +- .../SyntheticSourceIndexSettingsProvider.java | 82 +------- .../logsdb/SyntheticSourceLicenseService.java | 4 +- ...heticSourceIndexSettingsProviderTests.java | 190 ------------------ 6 files changed, 15 insertions(+), 318 deletions(-) delete mode 100644 x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java diff --git a/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java b/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java index 813a181045f2e..e7d267810424c 100644 --- a/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java +++ b/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java @@ -40,31 +40,7 @@ public void testFeatureUsageWithLogsdbIndex() throws IOException { assertThat(features, Matchers.empty()); } { - if (randomBoolean()) { - createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); - } else if (randomBoolean()) { - String mapping = """ - { - "properties": { - "field1": { - "type": "keyword", - "time_series_dimension": true - } - } - } - """; - var settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "field1").build(); - createIndex("test-index", settings, mapping); - } else { - String mapping = """ - { - "_source": { - "mode": "synthetic" - } - } - """; - createIndex("test-index", Settings.EMPTY, mapping); - } + createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); var response = getAsMap("/_license/feature_usage"); @SuppressWarnings("unchecked") List> features = (List>) response.get("features"); diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java index b2d2978a254df..efff6d0579838 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java @@ -42,31 +42,7 @@ public void testFeatureUsageWithLogsdbIndex() throws IOException { assertThat(features, Matchers.empty()); } { - if (randomBoolean()) { - createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); - } else if (randomBoolean()) { - String mapping = """ - { - "properties": { - "field1": { - "type": "keyword", - "time_series_dimension": true - } - } - } - """; - var settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "field1").build(); - createIndex("test-index", settings, mapping); - } else { - String mapping = """ - { - "_source": { - "mode": "synthetic" - } - } - """; - createIndex("test-index", Settings.EMPTY, mapping); - } + createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); var response = getAsMap("/_license/feature_usage"); @SuppressWarnings("unchecked") List> features = (List>) response.get("features"); diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java index 501cb8536ee7e..5cb7bf9e75252 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java @@ -51,10 +51,7 @@ public Collection getAdditionalIndexSettingProviders(Index if (DiscoveryNode.isStateless(settings)) { return List.of(logsdbIndexModeSettingsProvider); } - return List.of( - new SyntheticSourceIndexSettingsProvider(licenseService, parameters.mapperServiceFactory()), - logsdbIndexModeSettingsProvider - ); + return List.of(new SyntheticSourceIndexSettingsProvider(licenseService), logsdbIndexModeSettingsProvider); } @Override 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 a9f060528f05b..5b7792de0622a 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 @@ -9,41 +9,28 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; -import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettings; -import org.elasticsearch.index.IndexVersion; -import org.elasticsearch.index.mapper.MapperService; -import java.io.IOException; -import java.io.UncheckedIOException; import java.time.Instant; import java.util.List; - -import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_ROUTING_PATH; +import java.util.Locale; /** * An index setting provider that overwrites the source mode from synthetic to stored if synthetic source isn't allowed to be used. */ -final class SyntheticSourceIndexSettingsProvider implements IndexSettingProvider { +public class SyntheticSourceIndexSettingsProvider implements IndexSettingProvider { private static final Logger LOGGER = LogManager.getLogger(SyntheticSourceIndexSettingsProvider.class); private final SyntheticSourceLicenseService syntheticSourceLicenseService; - private final CheckedFunction mapperServiceFactory; - SyntheticSourceIndexSettingsProvider( - SyntheticSourceLicenseService syntheticSourceLicenseService, - CheckedFunction mapperServiceFactory - ) { + public SyntheticSourceIndexSettingsProvider(SyntheticSourceLicenseService syntheticSourceLicenseService) { this.syntheticSourceLicenseService = syntheticSourceLicenseService; - this.mapperServiceFactory = mapperServiceFactory; } @Override @@ -56,7 +43,7 @@ public Settings getAdditionalIndexSettings( Settings indexTemplateAndCreateRequestSettings, List combinedTemplateMappings ) { - if (newIndexHasSyntheticSourceUsage(indexName, isTimeSeries, indexTemplateAndCreateRequestSettings, combinedTemplateMappings) + if (newIndexHasSyntheticSourceUsage(indexTemplateAndCreateRequestSettings) && syntheticSourceLicenseService.fallbackToStoredSource()) { LOGGER.debug("creation of index [{}] with synthetic source without it being allowed", indexName); // TODO: handle falling back to stored source @@ -64,60 +51,11 @@ public Settings getAdditionalIndexSettings( return Settings.EMPTY; } - boolean newIndexHasSyntheticSourceUsage( - String indexName, - boolean isTimeSeries, - Settings indexTemplateAndCreateRequestSettings, - List combinedTemplateMappings - ) { - if ("validate-index-name".equals(indexName)) { - // This index name is used when validating component and index templates, we should skip this check in that case. - // (See MetadataIndexTemplateService#validateIndexTemplateV2(...) method) - return false; - } - - var tmpIndexMetadata = buildIndexMetadataForMapperService(indexName, isTimeSeries, indexTemplateAndCreateRequestSettings); - 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. - if (combinedTemplateMappings == null || combinedTemplateMappings.isEmpty()) { - combinedTemplateMappings = List.of(new CompressedXContent("{}")); - } - mapperService.merge(MapperService.SINGLE_MAPPING_NAME, combinedTemplateMappings, MapperService.MergeReason.INDEX_TEMPLATE); - return mapperService.documentMapper().sourceMapper().isSynthetic(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - // Create a dummy IndexMetadata instance that can be used to create a MapperService in order to check whether synthetic source is used: - private IndexMetadata buildIndexMetadataForMapperService( - String indexName, - boolean isTimeSeries, - Settings indexTemplateAndCreateRequestSettings - ) { - var tmpIndexMetadata = IndexMetadata.builder(indexName); - - int dummyPartitionSize = IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(indexTemplateAndCreateRequestSettings); - int dummyShards = indexTemplateAndCreateRequestSettings.getAsInt( - IndexMetadata.SETTING_NUMBER_OF_SHARDS, - dummyPartitionSize == 1 ? 1 : dummyPartitionSize + 1 - ); - int shardReplicas = indexTemplateAndCreateRequestSettings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0); - var finalResolvedSettings = Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) - .put(indexTemplateAndCreateRequestSettings) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, dummyShards) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, shardReplicas) - .put(IndexMetadata.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()); - - if (isTimeSeries) { - finalResolvedSettings.put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES); - // Avoid failing because index.routing_path is missing (in case fields are marked as dimension) - finalResolvedSettings.putList(INDEX_ROUTING_PATH.getKey(), List.of("path")); - } - - tmpIndexMetadata.settings(finalResolvedSettings); - return tmpIndexMetadata.build(); + boolean newIndexHasSyntheticSourceUsage(Settings indexTemplateAndCreateRequestSettings) { + // TODO: build tmp MapperService and check whether SourceFieldMapper#isSynthetic() to determine synthetic source usage. + // Not using IndexSettings.MODE.get() to avoid validation that may fail at this point. + var rawIndexMode = indexTemplateAndCreateRequestSettings.get(IndexSettings.MODE.getKey()); + IndexMode indexMode = rawIndexMode != null ? Enum.valueOf(IndexMode.class, rawIndexMode.toUpperCase(Locale.ROOT)) : null; + return indexMode != null && indexMode.isSyntheticSourceEnabled(); } } diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java index 89733c9ef1f2b..4e3e916762fab 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java @@ -16,7 +16,7 @@ /** * Determines based on license and fallback setting whether synthetic source usages should fallback to stored source. */ -final class SyntheticSourceLicenseService { +public final class SyntheticSourceLicenseService { private static final String MAPPINGS_FEATURE_FAMILY = "mappings"; @@ -39,7 +39,7 @@ final class SyntheticSourceLicenseService { private XPackLicenseState licenseState; private volatile boolean syntheticSourceFallback; - SyntheticSourceLicenseService(Settings settings) { + public SyntheticSourceLicenseService(Settings settings) { syntheticSourceFallback = FALLBACK_SETTING.get(settings); } 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 deleted file mode 100644 index 9a46b8de8b662..0000000000000 --- a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java +++ /dev/null @@ -1,190 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.logsdb; - -import org.elasticsearch.cluster.metadata.DataStream; -import org.elasticsearch.common.compress.CompressedXContent; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.MapperTestUtils; -import org.elasticsearch.test.ESTestCase; -import org.junit.Before; - -import java.io.IOException; -import java.util.List; - -public class SyntheticSourceIndexSettingsProviderTests extends ESTestCase { - - private SyntheticSourceIndexSettingsProvider provider; - - @Before - public void setup() { - SyntheticSourceLicenseService syntheticSourceLicenseService = new SyntheticSourceLicenseService(Settings.EMPTY); - provider = new SyntheticSourceIndexSettingsProvider( - syntheticSourceLicenseService, - im -> MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), im.getSettings(), im.getIndex().getName()) - ); - } - - public void testNewIndexHasSyntheticSourceUsage() throws IOException { - String dataStreamName = "logs-app1"; - String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); - Settings settings = Settings.EMPTY; - { - String mapping = """ - { - "_doc": { - "_source": { - "mode": "synthetic" - }, - "properties": { - "my_field": { - "type": "keyword" - } - } - } - } - """; - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); - assertTrue(result); - } - { - String mapping; - if (randomBoolean()) { - mapping = """ - { - "_doc": { - "_source": { - "mode": "stored" - }, - "properties": { - "my_field": { - "type": "keyword" - } - } - } - } - """; - } else { - mapping = """ - { - "_doc": { - "properties": { - "my_field": { - "type": "keyword" - } - } - } - } - """; - } - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); - assertFalse(result); - } - } - - public void testValidateIndexName() throws IOException { - String indexName = "validate-index-name"; - String mapping = """ - { - "_doc": { - "_source": { - "mode": "synthetic" - }, - "properties": { - "my_field": { - "type": "keyword" - } - } - } - } - """; - Settings settings = Settings.EMPTY; - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); - assertFalse(result); - } - - public void testNewIndexHasSyntheticSourceUsageLogsdbIndex() throws IOException { - String dataStreamName = "logs-app1"; - String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); - String mapping = """ - { - "_doc": { - "properties": { - "my_field": { - "type": "keyword" - } - } - } - } - """; - { - Settings settings = Settings.builder().put("index.mode", "logsdb").build(); - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); - assertTrue(result); - } - { - Settings settings = Settings.builder().put("index.mode", "logsdb").build(); - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of()); - assertTrue(result); - } - { - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, Settings.EMPTY, List.of()); - assertFalse(result); - } - { - boolean result = provider.newIndexHasSyntheticSourceUsage( - indexName, - false, - Settings.EMPTY, - List.of(new CompressedXContent(mapping)) - ); - assertFalse(result); - } - } - - public void testNewIndexHasSyntheticSourceUsageTimeSeries() throws IOException { - String dataStreamName = "logs-app1"; - String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); - String mapping = """ - { - "_doc": { - "properties": { - "my_field": { - "type": "keyword", - "time_series_dimension": true - } - } - } - } - """; - { - Settings settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "my_field").build(); - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); - assertTrue(result); - } - { - Settings settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "my_field").build(); - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of()); - assertTrue(result); - } - { - boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, Settings.EMPTY, List.of()); - assertFalse(result); - } - { - boolean result = provider.newIndexHasSyntheticSourceUsage( - indexName, - false, - Settings.EMPTY, - List.of(new CompressedXContent(mapping)) - ); - assertFalse(result); - } - } - -}