From 33b0a22794f54fec2346ea01621e53100274449b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 26 Sep 2025 10:45:41 +0200 Subject: [PATCH 1/4] Avoid changing tsid creating strategy for an index This avoids that the tsid and routing is calculated differently on the same index. Doing so would have the following consequences: * De-duplication would not work, for example, when a client re-transmits a batch as the id (which is based on the tsid) will be different. * When replaying a translog operation, the re-computed tsid and id will differ from the id stored in the translog. This would lead to a failure of the index operation. --- .../DataStreamIndexSettingsProvider.java | 12 +++++++----- .../DataStreamIndexSettingsProviderTests.java | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java index 268ef3455cd41..4da25409da105 100644 --- a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java +++ b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java @@ -172,11 +172,13 @@ public void onUpdateMappings(IndexMetadata indexMetadata, DocumentMapper documen boolean hasChanges = indexDimensions.size() != newIndexDimensions.size() && new HashSet<>(indexDimensions).equals(new HashSet<>(newIndexDimensions)) == false; if (matchesAllDimensions == false) { - // If the new dimensions don't match all potential dimension fields, we need to unset index.dimensions - // and set index.routing_path instead. - // This can happen if a new dynamic template with time_series_dimension: true is added to an existing index. - additionalSettings.putList(INDEX_DIMENSIONS.getKey(), List.of()); - additionalSettings.putList(INDEX_ROUTING_PATH.getKey(), newIndexDimensions); + throw new IllegalArgumentException( + "Cannot add dimension fields via dynamic templates or mappings for an index with " + + INDEX_DIMENSIONS.getKey() + + ". " + + "Please change the index template and roll over the data stream " + + "instead of modifying the mappings of the backing indices." + ); } else if (hasChanges) { additionalSettings.putList(INDEX_DIMENSIONS.getKey(), newIndexDimensions); } diff --git a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java index d896f6c7afa64..ba12c515032ba 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java +++ b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java @@ -956,10 +956,18 @@ public void testAddDynamicTemplate() throws Exception { } """; // we don't support index.dimensions with dynamic templates so we'll unset index.dimensions - Settings result = onUpdateMappings(null, "labels.*", mapping); - assertThat(result.size(), equalTo(2)); - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); - assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("labels.*")); + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> onUpdateMappings(null, "labels.*", mapping) + ); + assertThat( + exception.getMessage(), + equalTo( + "Cannot add dimension fields via dynamic templates or mappings for an index with index.dimensions. " + + "Please change the index template and roll over the data stream " + + "instead of modifying the mappings of the backing indices." + ) + ); } private Settings generateTsdbSettings(String mapping, Instant now) throws IOException { From 2e64e34e8a0fdcd2123966892ecc9ca472218055 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 26 Sep 2025 13:47:40 +0200 Subject: [PATCH 2/4] Fix TSDBIndexingIT --- .../datastreams/TSDBIndexingIT.java | 17 ++++++++++++----- .../DataStreamIndexSettingsProvider.java | 2 +- .../DataStreamIndexSettingsProviderTests.java | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java index 194737040455c..8dd48800e7451 100644 --- a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java +++ b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java @@ -8,6 +8,7 @@ */ package org.elasticsearch.datastreams; +import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.admin.indices.diskusage.AnalyzeIndexDiskUsageRequest; import org.elasticsearch.action.admin.indices.diskusage.TransportAnalyzeIndexDiskUsageAction; @@ -31,6 +32,7 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.metadata.ComponentTemplate; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.metadata.IndexMetadata; @@ -709,17 +711,22 @@ public void testAddDimensionToMapping() throws Exception { ] } """, XContentType.JSON); - assertAcked(client().execute(TransportPutMappingAction.TYPE, putMappingRequest).actionGet()); + ActionFuture putMappingFuture = client().execute(TransportPutMappingAction.TYPE, putMappingRequest); if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, putMappingFuture::actionGet); + assertThat(exception.getMessage(), containsString( + "Cannot add dynamic templates that define dimension fields on an existing index with index.dimensions") + ); assertThat( - getSetting(dataStreamName, IndexMetadata.INDEX_ROUTING_PATH), - containsInAnyOrder("metricset", "labels.*", "k8s.pod.name") + getSetting(dataStreamName, IndexMetadata.INDEX_DIMENSIONS), + containsInAnyOrder("metricset", "k8s.pod.name") ); + assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_ROUTING_PATH), empty()); } else { + assertAcked(putMappingFuture); assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_ROUTING_PATH), containsInAnyOrder("metricset")); + assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_DIMENSIONS), empty()); } - assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_DIMENSIONS), empty()); - indexWithPodNames(dataStreamName, Instant.now(), Map.of(), "dog", "cat"); } diff --git a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java index 4da25409da105..ad45c26815d9f 100644 --- a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java +++ b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java @@ -173,7 +173,7 @@ public void onUpdateMappings(IndexMetadata indexMetadata, DocumentMapper documen && new HashSet<>(indexDimensions).equals(new HashSet<>(newIndexDimensions)) == false; if (matchesAllDimensions == false) { throw new IllegalArgumentException( - "Cannot add dimension fields via dynamic templates or mappings for an index with " + "Cannot add dynamic templates that define dimension fields on an existing index with " + INDEX_DIMENSIONS.getKey() + ". " + "Please change the index template and roll over the data stream " diff --git a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java index ba12c515032ba..609cd6a26ab5c 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java +++ b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java @@ -963,7 +963,7 @@ public void testAddDynamicTemplate() throws Exception { assertThat( exception.getMessage(), equalTo( - "Cannot add dimension fields via dynamic templates or mappings for an index with index.dimensions. " + "Cannot add dynamic templates that define dimension fields on an existing index with index.dimensions. " + "Please change the index template and roll over the data stream " + "instead of modifying the mappings of the backing indices." ) From 3b1117f6f9e0aac8aa31e3b23ed55e1c70564ca9 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 26 Sep 2025 11:55:23 +0000 Subject: [PATCH 3/4] [CI] Auto commit changes from spotless --- .../org/elasticsearch/datastreams/TSDBIndexingIT.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java index 8dd48800e7451..aafc15576bb4b 100644 --- a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java +++ b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java @@ -714,13 +714,11 @@ public void testAddDimensionToMapping() throws Exception { ActionFuture putMappingFuture = client().execute(TransportPutMappingAction.TYPE, putMappingRequest); if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, putMappingFuture::actionGet); - assertThat(exception.getMessage(), containsString( - "Cannot add dynamic templates that define dimension fields on an existing index with index.dimensions") - ); assertThat( - getSetting(dataStreamName, IndexMetadata.INDEX_DIMENSIONS), - containsInAnyOrder("metricset", "k8s.pod.name") + exception.getMessage(), + containsString("Cannot add dynamic templates that define dimension fields on an existing index with index.dimensions") ); + assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_DIMENSIONS), containsInAnyOrder("metricset", "k8s.pod.name")); assertThat(getSetting(dataStreamName, IndexMetadata.INDEX_ROUTING_PATH), empty()); } else { assertAcked(putMappingFuture); From 28afa2fda43f4ef26ee883c6cc41a2cf94af4597 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 29 Sep 2025 09:33:05 +0200 Subject: [PATCH 4/4] Update JavaDoc --- .../DataStreamIndexSettingsProvider.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java index ad45c26815d9f..31866a19fb43a 100644 --- a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java +++ b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java @@ -154,11 +154,15 @@ public void provideAdditionalSettings( } /** - * This is called when mappings are updated, so that the {@link IndexMetadata#getTimeSeriesDimensions()} - * and {@link IndexMetadata#INDEX_ROUTING_PATH} settings are updated to match the new mappings. - * Updates {@link IndexMetadata#getTimeSeriesDimensions} if a new dimension field is added to the mappings, - * or sets {@link IndexMetadata#INDEX_ROUTING_PATH} if a new dimension field is added that doesn't allow for matching all - * dimension fields via a wildcard pattern. + * This is called when mappings are updated, so that the {@link IndexMetadata#INDEX_DIMENSIONS} + * setting is updated if a new dimension field is added to the mappings. + * + * @throws IllegalArgumentException If a dynamic template that defines dimension fields is added to an existing index with + * {@link IndexMetadata#getTimeSeriesDimensions()}. + * Changing fom {@link IndexMetadata#INDEX_DIMENSIONS} to {@link IndexMetadata#INDEX_ROUTING_PATH} + * is not allowed because it would violate the invariant that the same input document always results + * in the same _id and _tsid. + * Otherwise, data duplication or translog replay issues could occur. */ @Override public void onUpdateMappings(IndexMetadata indexMetadata, DocumentMapper documentMapper, Settings.Builder additionalSettings) {