Skip to content

Deprecate _source.mode in mappings#116689

Merged
dnhatn merged 21 commits intoelastic:mainfrom
dnhatn:deprecate-synthetic-source
Nov 20, 2024
Merged

Deprecate _source.mode in mappings#116689
dnhatn merged 21 commits intoelastic:mainfrom
dnhatn:deprecate-synthetic-source

Conversation

@dnhatn
Copy link
Copy Markdown
Member

@dnhatn dnhatn commented Nov 12, 2024

This change deprecates _source.mode in mappings, replacing it with the index.mapping.source.mode index setting.

@dnhatn dnhatn force-pushed the deprecate-synthetic-source branch 12 times, most recently from c8f9a22 to 63cfc41 Compare November 13, 2024 05:41
@dnhatn dnhatn changed the title Deprecate Deprecate _source.mode in mappings Nov 13, 2024
@dnhatn dnhatn force-pushed the deprecate-synthetic-source branch 2 times, most recently from 496f8d8 to d2b8d68 Compare November 13, 2024 06:31
@dnhatn dnhatn force-pushed the deprecate-synthetic-source branch from d2b8d68 to 1331464 Compare November 13, 2024 16:18
@dnhatn dnhatn added v8.17.0 auto-backport Automatically create backport pull requests when merged :StorageEngine/Mapping The storage related side of mappings >deprecation labels Nov 13, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @dnhatn, I've created a changelog YAML for you. Note that since this PR is labelled >deprecation, you need to update the changelog YAML to fill out the extended information sections.

@dnhatn dnhatn marked this pull request as ready for review November 13, 2024 16:21
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

Copy link
Copy Markdown
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nhat, I left questions are the deprecation logic in MetadataCreateIndexService.

I wonder whether the following is perhaps a better mechanism to emit depreciation warning in case _source.mode is used:

Subject: [PATCH] Adjust SyntheticSourceLicenseService 

Allow gold and platinum license to use synthetic source for a limited time.
If the start time of a license if before a cut off date, then gold and platinum licenses will not fallback to stored source if synthetic source is used.
---
Index: server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java
--- a/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java	(revision bd091d3d96b33adb4121f980dc4f9f7a2a87b043)
+++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java	(date 1731573325979)
@@ -10,6 +10,8 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
@@ -27,6 +29,9 @@
  * Parser for {@link Mapping} provided in {@link CompressedXContent} format
  */
 public final class MappingParser {
+
+    private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(MappingParser.class);
+
     private final Supplier<MappingParserContext> mappingParserContextSupplier;
     private final Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier;
     private final Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers;
@@ -144,6 +149,14 @@
                 }
                 @SuppressWarnings("unchecked")
                 Map<String, Object> fieldNodeMap = (Map<String, Object>) fieldNode;
+
+                // iirc after parsing fieldNodeMap, keys are removed
+                if (reason == MergeReason.MAPPING_UPDATE && SourceFieldMapper.CONTENT_TYPE.equals(fieldName)) {
+                    if (fieldNodeMap.containsKey("mode")) {
+                        DEPRECATION_LOGGER.critical(DeprecationCategory.MAPPINGS, "mapping_source_mode", SourceFieldMapper.DEPRECATION_WARNING);
+                    }
+                }
+
                 MetadataFieldMapper metadataFieldMapper = typeParser.parse(fieldName, fieldNodeMap, mappingParserContext).build();
                 metadataMappers.put(metadataFieldMapper.getClass(), metadataFieldMapper);
                 assert fieldNodeMap.isEmpty();

I think this also returns warning when mapping gets updated, maybe that isn't too bad? But this should only return a warning when _source.mode attribute is really used.

final IndexSettings indexSettings = mapperService.getIndexSettings();
if (indexSettings != null
&& indexSettings.getIndexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)
&& SourceFieldMapper.isSynthetic(indexSettings) == false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to check whether index.mapping.source.mode index setting has not be defined here? Other we only check whether this setting has been set to synthetic?

&& indexSettings.getIndexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)
&& SourceFieldMapper.isSynthetic(indexSettings) == false
&& mapperService.documentMapper() != null
&& mapperService.documentMapper().sourceMapper().isSynthetic()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we check whether index.mapping.source.mode has not be defined, then I think mapperService.documentMapper().sourceMapper().isSynthetic() can return true if index.mode=logsdb|time_series and no _source.mode has been defined?

elasticsearchmachine pushed a commit that referenced this pull request Nov 20, 2024
This change deprecates _source.mode in mappings, replacing it with the
index.mapping.source.mode index setting.
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Nov 20, 2024
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Nov 20, 2024
martijnvg added a commit that referenced this pull request Nov 20, 2024
This reverts commit 0d7b90e, because of bwc testing failures.
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Nov 20, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 20, 2024
Re-introduce elastic#116689 using cluster features
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 20, 2024
dnhatn added a commit that referenced this pull request Nov 20, 2024
rjernst pushed a commit to rjernst/elasticsearch that referenced this pull request Nov 20, 2024
This change deprecates _source.mode in mappings, replacing it with the 
index.mapping.source.mode index setting.
rjernst pushed a commit to rjernst/elasticsearch that referenced this pull request Nov 20, 2024
rjernst pushed a commit to rjernst/elasticsearch that referenced this pull request Nov 20, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 21, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 25, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 25, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 25, 2024
dnhatn added a commit that referenced this pull request Nov 25, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 25, 2024
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Nov 25, 2024
dnhatn added a commit that referenced this pull request Nov 26, 2024
Backport of #116689 to 8.17

This change deprecates _source.mode in mappings, replacing it with
the index.mapping.source.mode index setting.
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
This change deprecates _source.mode in mappings, replacing it with the 
index.mapping.source.mode index setting.
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >deprecation :StorageEngine/Mapping The storage related side of mappings Team:StorageEngine test-full-bwc Trigger full BWC version matrix tests v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants