Skip to content

Commit bd3f8dd

Browse files
author
Madhan
committed
no-mistakes(review): Reject generation-conflicting backing index name; use value equality in mapper
1 parent c475eb8 commit bd3f8dd

3 files changed

Lines changed: 36 additions & 1 deletion

File tree

server/src/main/java/org/opensearch/action/admin/indices/datastream/ModifyDataStreamsAction.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,26 @@ private static void addBackingIndex(
313313
CheckedFunction<IndexMetadata, MapperService, IOException> mapperServiceFactory
314314
) throws IOException {
315315
final DataStream dataStream = validateDataStream(builder, dataStreamName);
316+
317+
// an index whose name matches this data stream's backing-index naming pattern with a generation counter greater than
318+
// the stream's current generation cannot be added: it would collide with a future backing index produced by a rollover
319+
// and make the metadata invalid (Metadata.Builder.validateDataStreams would later throw IllegalStateException -> HTTP 500)
320+
final String backingIndexPrefix = DataStream.BACKING_INDEX_PREFIX + dataStreamName + "-";
321+
if (indexName.startsWith(backingIndexPrefix)
322+
&& Metadata.NUMBER_PATTERN.matcher(indexName.substring(backingIndexPrefix.length())).matches()
323+
&& IndexMetadata.parseIndexNameCounter(indexName) > dataStream.getGeneration()) {
324+
throw new IllegalArgumentException(
325+
"cannot add index ["
326+
+ indexName
327+
+ "] to data stream ["
328+
+ dataStreamName
329+
+ "] because its name conflicts with a future backing index of the data stream "
330+
+ "(current generation ["
331+
+ dataStream.getGeneration()
332+
+ "])"
333+
);
334+
}
335+
316336
final IndexMetadata index = validateIndex(builder, indexName);
317337

318338
// an index that is already a backing index of this data stream cannot be added again

server/src/main/java/org/opensearch/index/mapper/DataStreamFieldMapper.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Collections;
2222
import java.util.List;
2323
import java.util.Map;
24+
import java.util.Objects;
2425

2526
/**
2627
* Field mapper for a datastream field
@@ -51,7 +52,7 @@ public static final class Builder extends MetadataFieldMapper.Builder {
5152
// The _data_stream_timestamp meta field can be enabled on a mapping update (e.g. when an existing index is
5253
// adapted into a data stream backing index) but, once enabled, it cannot be disabled again.
5354
final Parameter<Boolean> enabledParam = Parameter.boolParam("enabled", false, mapper -> toType(mapper).enabled, Defaults.ENABLED)
54-
.setMergeValidator((previous, current) -> previous == current || (previous == false && current));
55+
.setMergeValidator((previous, current) -> Objects.equals(previous, current) || (previous == false && current));
5556

5657
final Parameter<TimestampField> timestampFieldParam = new Parameter<>(
5758
"timestamp_field",

server/src/test/java/org/opensearch/action/admin/indices/datastream/ModifyDataStreamsActionTests.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,20 @@ public void testAddIndexBelongingToAnotherDataStreamFails() {
248248
assertThat(e.getMessage(), containsString("is already a backing index of data stream [other-data-stream]"));
249249
}
250250

251+
public void testAddIndexWithGenerationConflictNameFails() {
252+
ClusterState cs = getClusterStateWithDataStreams(List.of(new Tuple<>(DATA_STREAM, 2)), List.of());
253+
// an index whose name matches the backing-index naming pattern but with a counter greater than the stream's
254+
// current generation would collide with a future rollover-created backing index; the request must be rejected with a
255+
// 400 (IllegalArgumentException) up front rather than failing the cluster state build with a 500 (IllegalStateException)
256+
String conflictingName = DataStream.getDefaultBackingIndexName(DATA_STREAM, 99);
257+
258+
IllegalArgumentException e = expectThrows(
259+
IllegalArgumentException.class,
260+
() -> modify(cs, List.of(DataStreamAction.addBackingIndex(DATA_STREAM, conflictingName)))
261+
);
262+
assertThat(e.getMessage(), containsString("conflicts with a future backing index"));
263+
}
264+
251265
public void testMultipleActionsAppliedAtomically() {
252266
ClusterState cs = getClusterStateWithDataStreams(List.of(new Tuple<>(DATA_STREAM, 3)), List.of("standalone-index"));
253267
String indexToRemove = DataStream.getDefaultBackingIndexName(DATA_STREAM, 1);

0 commit comments

Comments
 (0)