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..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 @@ -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,20 @@ 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( - getSetting(dataStreamName, IndexMetadata.INDEX_ROUTING_PATH), - containsInAnyOrder("metricset", "labels.*", "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); 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 268ef3455cd41..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) { @@ -172,11 +176,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 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 " + + "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..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 @@ -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 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." + ) + ); } private Settings generateTsdbSettings(String mapping, Instant now) throws IOException {