From f1eac4c685a60459d5d65149b1399a0dceddaf0c Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 18 Dec 2024 16:00:18 +0100 Subject: [PATCH] [8.x] Fix synthetic recovery source version validation. (#118942) * Fix synthetic recovery source version validation. (#118924) The validation didn't work as expected, because created version was always 0 (which is created version default value). Moving the validation to a place that does have access to the index version. * fixed compile error after cherry picking * iter --- .../indices.create/20_synthetic_source.yml | 40 +++++++++++++++++++ .../elasticsearch/index/IndexSettings.java | 30 +++++++------- .../index/mapper/MapperFeatures.java | 3 +- .../index/mapper/SourceFieldMapper.java | 1 + .../index/mapper/SourceFieldMapperTests.java | 25 ------------ 5 files changed, 58 insertions(+), 41 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml index 962e10c0a6a70..cd2471eda0a0a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml @@ -2012,3 +2012,43 @@ synthetic_source with copy_to pointing inside dynamic object: hits.hits.2.fields: c.copy.keyword: [ "hello", "zap" ] +--- +create 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: + 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: + bulk: + index: test + refresh: true + body: + - '{ "create": { } }' + - '{ "field": "aaaa" }' + - '{ "create": { } }' + - '{ "field": "bbbb" }' + + - do: + indices.disk_usage: + index: test + run_expensive_tasks: true + flush: false + - gt: { test.store_size_in_bytes: 0 } + - is_false: test.fields._recovery_source diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index 751b194af3d50..02617b7dc376d 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -688,25 +688,11 @@ public void validate(Boolean enabled, Map, Object> settings) { ); } } - - // Verify that all nodes can handle this setting - var version = (IndexVersion) settings.get(SETTING_INDEX_VERSION_CREATED); - if (version.before(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BACKPORT)) { - throw new IllegalArgumentException( - String.format( - Locale.ROOT, - "The setting [%s] is unavailable on this cluster because some nodes are running older " - + "versions that do not support it. Please upgrade all nodes to the latest version " - + "and try again.", - RECOVERY_USE_SYNTHETIC_SOURCE_SETTING.getKey() - ) - ); - } } @Override public Iterator> settings() { - List> res = List.of(INDEX_MAPPER_SOURCE_MODE_SETTING, SETTING_INDEX_VERSION_CREATED, MODE); + List> res = List.of(INDEX_MAPPER_SOURCE_MODE_SETTING, MODE); return res.iterator(); } }, @@ -1049,6 +1035,20 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti indexMappingSourceMode = scopedSettings.get(INDEX_MAPPER_SOURCE_MODE_SETTING); recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings); recoverySourceSyntheticEnabled = scopedSettings.get(RECOVERY_USE_SYNTHETIC_SOURCE_SETTING); + if (recoverySourceSyntheticEnabled) { + // Verify that all nodes can handle this setting + if (version.before(IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BACKPORT)) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "The setting [%s] is unavailable on this cluster because some nodes are running older " + + "versions that do not support it. Please upgrade all nodes to the latest version " + + "and try again.", + RECOVERY_USE_SYNTHETIC_SOURCE_SETTING.getKey() + ) + ); + } + } scopedSettings.addSettingsUpdateConsumer( MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java index 193312477dd08..c32a1fc2d58d7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java @@ -70,7 +70,8 @@ public Set getTestFeatures() { DocumentParser.FIX_PARSING_SUBOBJECTS_FALSE_DYNAMIC_FALSE, CONSTANT_KEYWORD_SYNTHETIC_SOURCE_WRITE_FIX, META_FETCH_FIELDS_ERROR_CODE_CHANGED, - SPARSE_VECTOR_STORE_SUPPORT + SPARSE_VECTOR_STORE_SUPPORT, + SourceFieldMapper.SYNTHETIC_RECOVERY_SOURCE ); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index 848e0178269fc..648dcd304f7b7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -55,6 +55,7 @@ public class SourceFieldMapper extends MetadataFieldMapper { "mapper.source.remove_synthetic_source_only_validation" ); public static final NodeFeature SOURCE_MODE_FROM_INDEX_SETTING = new NodeFeature("mapper.source.mode_from_index_setting"); + public static final NodeFeature SYNTHETIC_RECOVERY_SOURCE = new NodeFeature("mapper.synthetic_recovery_source"); public static final String NAME = "_source"; public static final String RECOVERY_SOURCE_NAME = "_recovery_source"; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java index e844edd27f1ab..b7693513a434d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java @@ -465,31 +465,6 @@ public void testRecoverySourceWitInvalidSettings() { ) ); } - { - Settings settings = Settings.builder() - .put(SourceFieldMapper.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), SourceFieldMapper.Mode.SYNTHETIC.toString()) - .put(IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE_SETTING.getKey(), true) - .build(); - IllegalArgumentException exc = expectThrows( - IllegalArgumentException.class, - () -> createMapperService( - IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersions.USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BACKPORT), - settings, - () -> false, - topMapping(b -> {}) - ) - ); - assertThat( - exc.getMessage(), - containsString( - String.format( - Locale.ROOT, - "The setting [%s] is unavailable on this cluster", - IndexSettings.RECOVERY_USE_SYNTHETIC_SOURCE_SETTING.getKey() - ) - ) - ); - } } public void testRecoverySourceWithSyntheticSource() throws IOException {