synthetic source index setting provider should check source field mapper #113522

martijnvg · 2024-09-25T10:58:43Z

Adds logic to SyntheticSourceIndexSettingsProvider to check source field mapper to whether synthetic source is enabled.

Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

…eld mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

elasticsearchmachine · 2024-09-25T10:59:07Z

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

martijnvg · 2024-09-25T10:59:55Z

...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java

-        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();
+    boolean newIndexHasSyntheticSourceUsage(


this is the core of the change: instead of checking the index.mode setting, this now uses SourceFieldMapper#isSynthetic() to determine synthetic source usage..

salvatore-campagna · 2024-09-27T12:36:57Z

...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java

+        }
+
+        tmpIndexMetadata.settings(finalResolvedSettings);
+        // Create MapperService just to extract keyword dimension fields:


I don't understand this comment...I don't see we are extracting dimensions...

that comment makes no sense in this context, I copied it initially from DataStreamIndexSettingsProvider. I will update this.

salvatore-campagna · 2024-09-27T14:04:06Z

...ogsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java

+
+        var tmpIndexMetadata = IndexMetadata.builder(indexName);
+
+        int dummyPartitionSize = IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(indexTemplateAndCreateRequestSettings);


I would probably create a method like buildDummySettingsForMerge just to extract this logic and explain (maybe in Javadoc)that we do this just as a way to create aMapperService

👍 makes sense

…ngsProvider_check_mapping

elasticsearchmachine · 2024-09-27T18:09:14Z

💚 Backport successful

Status	Branch	Result
✅	8.x

…per (elastic#113522) Adds logic to SyntheticSourceIndexSettingsProvider to check source field mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

…per (#113522) (#113717) Adds logic to SyntheticSourceIndexSettingsProvider to check source field mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

…ield mapper (elastic#113522)" This reverts commit 0cdcc8c.

…ield mapper (elastic#113522) (elastic#113717)" This reverts commit c7e2b28.

…ield mapper (#113522) (#113717)" (#113746) Revert "synthetic source index setting provider should check source field mapper (#113522) (#113717)" This reverts commit c7e2b28.

…ield mapper (#113522)" (#113745) Revert synthetic source index setting provider should check source field mapper introduced in #113522 This reverts commit 0cdcc8c.

…field mapper (elastic#113522)" (elastic#113745) This reverts commit e0a2d61.

…field mapper" (#113759) Originally added via #113522, but then reverted via #113745, because of mixed cluster test failures (#113730). This PR is a clean revert of the commit the reverted #113522 and one additional commit that should address the build failures report in #113730 : c7bd242 Basically create index invocation that would fail anyway should be ignored. If mapper service creation now fails, then we just assume that there is no synthetic source usage. This is ok, because the index creation would fail anyway later one. Closes #113730

…field mapper" (elastic#113759) Originally added via elastic#113522, but then reverted via elastic#113745, because of mixed cluster test failures (elastic#113730). This PR is a clean revert of the commit the reverted elastic#113522 and one additional commit that should address the build failures report in elastic#113730 : c7bd242 Basically create index invocation that would fail anyway should be ignored. If mapper service creation now fails, then we just assume that there is no synthetic source usage. This is ok, because the index creation would fail anyway later one. Closes elastic#113730

…ource field mapper" (#113759) (#113828) Backporting #113759 to 8.x branch. Originally added via #113522, but then reverted via #113745, because of mixed cluster test failures (#113730). This PR is a clean revert of the commit the reverted #113522 and one additional commit that should address the build failures report in #113730 : c7bd242 Basically create index invocation that would fail anyway should be ignored. If mapper service creation now fails, then we just assume that there is no synthetic source usage. This is ok, because the index creation would fail anyway later one. Closes #113730

…per (elastic#113522) Adds logic to SyntheticSourceIndexSettingsProvider to check source field mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

…ield mapper (elastic#113522)" (elastic#113745) Revert synthetic source index setting provider should check source field mapper introduced in elastic#113522 This reverts commit 0cdcc8c.

…field mapper" (elastic#113759) Originally added via elastic#113522, but then reverted via elastic#113745, because of mixed cluster test failures (elastic#113730). This PR is a clean revert of the commit the reverted elastic#113522 and one additional commit that should address the build failures report in elastic#113730 : c7bd242 Basically create index invocation that would fail anyway should be ignored. If mapper service creation now fails, then we just assume that there is no synthetic source usage. This is ok, because the index creation would fail anyway later one. Closes elastic#113730

Adds logic to SyntheticSourceIndexSettingsProvider to check source fi…

727e359

…eld mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source.

martijnvg added >non-issue :StorageEngine/Logs You know, for Logs v8.16.0 v9.0.0 labels Sep 25, 2024

elasticsearchmachine added the Team:StorageEngine label Sep 25, 2024

martijnvg commented Sep 25, 2024

View reviewed changes

martijnvg added 3 commits September 25, 2024 13:06

added missing unit test

e426d0e

adjusted class visibility

0e7422b

updated comment

6bd4d67

martijnvg changed the title ~~SyntheticSourceIndexSettingsProvider should check source field mapper~~ synthetic source index setting provider should check source field mapper Sep 25, 2024

martijnvg added 3 commits September 25, 2024 13:34

iter

348352a

iter

fa39345

update comment

aaec9f8

salvatore-campagna approved these changes Sep 27, 2024

View reviewed changes

salvatore-campagna reviewed Sep 27, 2024

View reviewed changes

martijnvg added 2 commits September 27, 2024 16:48

Merge remote-tracking branch 'es/main' into SyntheticSourceIndexSetti…

e685c6c

…ngsProvider_check_mapping

iter

087aa69

martijnvg added the auto-backport-and-merge label Sep 27, 2024

martijnvg merged commit 0cdcc8c into elastic:main Sep 27, 2024
15 checks passed

martijnvg mentioned this pull request Sep 27, 2024

[8.x] synthetic source index setting provider should check source field mapper (#113522) #113717

Merged

dnhatn mentioned this pull request Sep 28, 2024

[CI] MixedClusterClientYamlTestSuiteIT class failing #113730

Closed

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Sep 29, 2024

Revert "synthetic source index setting provider should check source f…

596133a

…ield mapper (elastic#113522)" This reverts commit 0cdcc8c.

dnhatn mentioned this pull request Sep 29, 2024

Revert #113522 #113745

Merged

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Sep 29, 2024

Revert "synthetic source index setting provider should check source f…

e7577d8

…ield mapper (elastic#113522) (elastic#113717)" This reverts commit c7e2b28.

dnhatn mentioned this pull request Sep 29, 2024

Revert #113522 #113746

Merged

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 30, 2024

Reapply "synthetic source index setting provider should check source …

6c19be7

…field mapper (elastic#113522)" (elastic#113745) This reverts commit e0a2d61.

martijnvg mentioned this pull request Sep 30, 2024

Reapply "synthetic source index setting provider should check source field mapper" #113759

Merged

martijnvg added test-full-bwc Trigger full BWC version matrix tests and removed test-full-bwc Trigger full BWC version matrix tests labels Sep 30, 2024

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 30, 2024

Reapply "synthetic source index setting provider should check source …

b2f1541

…field mapper (elastic#113522)" (elastic#113745) This reverts commit e0a2d61.

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 30, 2024

Reapply "synthetic source index setting provider should check source …

60ae0ca

…field mapper (elastic#113522)" (elastic#113745) This reverts commit e0a2d61.

martijnvg mentioned this pull request Oct 1, 2024

[8.x] Reapply "synthetic source index setting provider should check source field mapper" #113828

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

synthetic source index setting provider should check source field mapper #113522

synthetic source index setting provider should check source field mapper #113522

Uh oh!

martijnvg commented Sep 25, 2024 •

edited

Loading

Uh oh!

elasticsearchmachine commented Sep 25, 2024

Uh oh!

martijnvg Sep 25, 2024

Uh oh!

salvatore-campagna Sep 27, 2024

Uh oh!

martijnvg Sep 27, 2024

Uh oh!

salvatore-campagna Sep 27, 2024

Uh oh!

martijnvg Sep 27, 2024

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 27, 2024

Uh oh!

Uh oh!


		var tmpIndexMetadata = IndexMetadata.builder(indexName);

		int dummyPartitionSize = IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(indexTemplateAndCreateRequestSettings);

synthetic source index setting provider should check source field mapper #113522

synthetic source index setting provider should check source field mapper #113522

Uh oh!

Conversation

martijnvg commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 25, 2024

Uh oh!

martijnvg Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

salvatore-campagna Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

martijnvg Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

salvatore-campagna Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

martijnvg Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 27, 2024

💚 Backport successful

Uh oh!

Uh oh!

martijnvg commented Sep 25, 2024 •

edited

Loading