Skip to content

Commit bcf8600

Browse files
danhermanndanhermann
authored
Delete index API properly handles backing indices for data streams (#55690)
1 parent 1955b46 commit bcf8600

File tree

7 files changed

+211
-10
lines changed

7 files changed

+211
-10
lines changed

‎rest-api-spec/src/main/resources/rest-api-spec/test/indices.delete/20_backing_indices.yml

+99
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
---
2+
"Delete backing index on data stream":
3+
- skip:
4+
version: " - 7.99.99"
5+
reason: "enable in 7.8+ after backporting"
6+
7+
- do:
8+
indices.create_data_stream:
9+
name: simple-data-stream
10+
body:
11+
timestamp_field: "@timestamp"
12+
- is_true: acknowledged
13+
14+
# rollover data stream to create new backing index
15+
- do:
16+
indices.rollover:
17+
alias: "simple-data-stream"
18+
19+
- match: { old_index: simple-data-stream-000001 }
20+
- match: { new_index: simple-data-stream-000002 }
21+
- match: { rolled_over: true }
22+
- match: { dry_run: false }
23+
24+
# ensure new index is created
25+
- do:
26+
indices.exists:
27+
index: simple-data-stream-000002
28+
29+
- is_true: ''
30+
31+
- do:
32+
indices.delete:
33+
index: simple-data-stream-000001
34+
35+
- do:
36+
indices.exists:
37+
index: simple-data-stream-000001
38+
39+
- is_false: ''
40+
41+
- do:
42+
indices.get_data_streams:
43+
name: "*"
44+
- match: { 0.name: simple-data-stream }
45+
- match: { 0.timestamp_field: '@timestamp' }
46+
- match: { 0.generation: 2 }
47+
- length: { 0.indices: 1 }
48+
- match: { 0.indices.0.index_name: 'simple-data-stream-000002' }
49+
50+
- do:
51+
indices.delete_data_stream:
52+
name: simple-data-stream
53+
- is_true: acknowledged
54+
55+
---
56+
"Attempt to delete write index on data stream is rejected":
57+
- skip:
58+
version: " - 7.99.99"
59+
reason: "enable in 7.8+ after backporting"
60+
61+
- do:
62+
indices.create_data_stream:
63+
name: simple-data-stream
64+
body:
65+
timestamp_field: "@timestamp"
66+
- is_true: acknowledged
67+
68+
# rollover data stream to create new backing index
69+
- do:
70+
indices.rollover:
71+
alias: "simple-data-stream"
72+
73+
- match: { old_index: simple-data-stream-000001 }
74+
- match: { new_index: simple-data-stream-000002 }
75+
- match: { rolled_over: true }
76+
- match: { dry_run: false }
77+
78+
# ensure new index is created
79+
- do:
80+
indices.exists:
81+
index: simple-data-stream-000002
82+
83+
- is_true: ''
84+
85+
- do:
86+
catch: bad_request
87+
indices.delete:
88+
index: simple-data-stream-000002
89+
90+
- do:
91+
indices.exists:
92+
index: simple-data-stream-000002
93+
94+
- is_true: ''
95+
96+
- do:
97+
indices.delete_data_stream:
98+
name: simple-data-stream
99+
- is_true: acknowledged

‎server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java

+15
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public DataStream(String name, String timeStampField, List<Index> indices, long
4747
this.timeStampField = timeStampField;
4848
this.indices = indices;
4949
this.generation = generation;
50+
assert indices.size() > 0;
51+
assert indices.get(indices.size() - 1).getName().equals(getBackingIndexName(name, generation));
5052
}
5153

5254
public DataStream(String name, String timeStampField, List<Index> indices) {
@@ -84,6 +86,19 @@ public DataStream rollover(Index newWriteIndex) {
8486
return new DataStream(name, timeStampField, backingIndices, generation + 1);
8587
}
8688

89+
/**
90+
* Removes the specified backing index and returns a new {@code DataStream} instance with
91+
* the remaining backing indices.
92+
*
93+
* @param index the backing index to remove
94+
* @return new {@code DataStream} instance with the remaining backing indices
95+
*/
96+
public DataStream removeBackingIndex(Index index) {
97+
List<Index> backingIndices = new ArrayList<>(indices);
98+
backingIndices.remove(index);
99+
return new DataStream(name, timeStampField, backingIndices, generation);
100+
}
101+
87102
/**
88103
* Generates the name of the index that conforms to the naming convention for backing indices
89104
* on data streams given the specified data stream name and generation.

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexService.java

+22-3
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@
4242
import org.elasticsearch.snapshots.SnapshotsService;
4343

4444
import java.util.Arrays;
45+
import java.util.HashMap;
46+
import java.util.HashSet;
47+
import java.util.Map;
4548
import java.util.Set;
4649

47-
import static java.util.stream.Collectors.toSet;
48-
4950
/**
5051
* Deletes indices.
5152
*/
@@ -91,7 +92,21 @@ public ClusterState execute(final ClusterState currentState) {
9192
*/
9293
public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices) {
9394
final Metadata meta = currentState.metadata();
94-
final Set<Index> indicesToDelete = indices.stream().map(i -> meta.getIndexSafe(i).getIndex()).collect(toSet());
95+
final Set<Index> indicesToDelete = new HashSet<>();
96+
final Map<Index, DataStream> backingIndices = new HashMap<>();
97+
for (Index index : indices) {
98+
IndexMetadata im = meta.getIndexSafe(index);
99+
IndexAbstraction.DataStream parent = meta.getIndicesLookup().get(im.getIndex().getName()).getParentDataStream();
100+
if (parent != null) {
101+
if (parent.getWriteIndex().equals(im)) {
102+
throw new IllegalArgumentException("index [" + index.getName() + "] is the write index for data stream [" +
103+
parent.getName() + "] and cannot be deleted");
104+
} else {
105+
backingIndices.put(index, parent.getDataStream());
106+
}
107+
}
108+
indicesToDelete.add(im.getIndex());
109+
}
95110

96111
// Check if index deletion conflicts with any running snapshots
97112
Set<Index> snapshottingIndices = SnapshotsService.snapshottingIndices(currentState, indicesToDelete);
@@ -112,6 +127,10 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
112127
routingTableBuilder.remove(indexName);
113128
clusterBlocksBuilder.removeIndexBlocks(indexName);
114129
metadataBuilder.remove(indexName);
130+
if (backingIndices.containsKey(index)) {
131+
DataStream parent = backingIndices.get(index);
132+
metadataBuilder.put(parent.removeBackingIndex(index));
133+
}
115134
}
116135
// add tombstones to the cluster state for each deleted index
117136
final IndexGraveyard currentGraveyard = graveyardBuilder.addTombstones(indices).build(settings);

‎server/src/test/java/org/elasticsearch/action/admin/indices/datastream/DeleteDataStreamRequestTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public void testDeleteDataStream() {
7575
final String dataStreamName = "my-data-stream";
7676
final List<String> otherIndices = randomSubsetOf(List.of("foo", "bar", "baz"));
7777

78-
ClusterState cs = getClusterState(List.of(new Tuple<>(dataStreamName, 2)), otherIndices);
78+
ClusterState cs = getClusterStateWithDataStreams(List.of(new Tuple<>(dataStreamName, 2)), otherIndices);
7979
DeleteDataStreamAction.Request req = new DeleteDataStreamAction.Request(dataStreamName);
8080
ClusterState newState = DeleteDataStreamAction.TransportAction.removeDataStream(getMetadataDeleteIndexService(), cs, req);
8181
assertThat(newState.metadata().dataStreams().size(), equalTo(0));
@@ -119,7 +119,7 @@ private static MetadataDeleteIndexService getMetadataDeleteIndexService() {
119119
* @param dataStreams The names of the data streams to create with their respective number of backing indices
120120
* @param indexNames The names of indices to create that do not back any data streams
121121
*/
122-
private static ClusterState getClusterState(List<Tuple<String, Integer>> dataStreams, List<String> indexNames) {
122+
public static ClusterState getClusterStateWithDataStreams(List<Tuple<String, Integer>> dataStreams, List<String> indexNames) {
123123
Metadata.Builder builder = Metadata.builder();
124124

125125
List<IndexMetadata> allIndices = new ArrayList<>();

‎server/src/test/java/org/elasticsearch/action/admin/indices/datastream/GetDataStreamsResponseTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.elasticsearch.action.admin.indices.datastream.GetDataStreamsAction.Response;
2222
import org.elasticsearch.cluster.metadata.DataStream;
23+
import org.elasticsearch.cluster.metadata.DataStreamTests;
2324
import org.elasticsearch.common.io.stream.Writeable;
2425
import org.elasticsearch.common.xcontent.XContentParser;
2526
import org.elasticsearch.common.xcontent.XContentParser.Token;
@@ -29,8 +30,6 @@
2930
import java.util.ArrayList;
3031
import java.util.List;
3132

32-
import static org.elasticsearch.cluster.metadata.DataStreamTests.randomIndexInstances;
33-
3433
public class GetDataStreamsResponseTests extends AbstractSerializingTestCase<Response> {
3534

3635
@Override
@@ -54,7 +53,7 @@ protected Response createTestInstance() {
5453
int numDataStreams = randomIntBetween(0, 8);
5554
List<DataStream> dataStreams = new ArrayList<>();
5655
for (int i = 0; i < numDataStreams; i++) {
57-
dataStreams.add(new DataStream(randomAlphaOfLength(4), randomAlphaOfLength(4), randomIndexInstances()));
56+
dataStreams.add(DataStreamTests.randomInstance());
5857
}
5958
return new Response(dataStreams);
6059
}

‎server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java

+20
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,24 @@ public void testRollover() {
7878
assertTrue(rolledDs.getIndices().containsAll(ds.getIndices()));
7979
assertTrue(rolledDs.getIndices().contains(newWriteIndex));
8080
}
81+
82+
public void testRemoveBackingIndex() {
83+
int numBackingIndices = randomIntBetween(2, 32);
84+
int indexToRemove = randomIntBetween(1, numBackingIndices - 1);
85+
String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
86+
87+
List<Index> indices = new ArrayList<>(numBackingIndices);
88+
for (int k = 1; k <= numBackingIndices; k++) {
89+
indices.add(new Index(DataStream.getBackingIndexName(dataStreamName, k), UUIDs.randomBase64UUID(random())));
90+
}
91+
DataStream original = new DataStream(dataStreamName, "@timestamp", indices);
92+
DataStream updated = original.removeBackingIndex(indices.get(indexToRemove - 1));
93+
assertThat(updated.getName(), equalTo(original.getName()));
94+
assertThat(updated.getGeneration(), equalTo(original.getGeneration()));
95+
assertThat(updated.getTimeStampField(), equalTo(original.getTimeStampField()));
96+
assertThat(updated.getIndices().size(), equalTo(numBackingIndices - 1));
97+
for (int k = 0; k < (numBackingIndices - 1); k++) {
98+
assertThat(updated.getIndices().get(k), equalTo(original.getIndices().get(k < (indexToRemove - 1) ? k : k + 1)));
99+
}
100+
}
81101
}

‎server/src/test/java/org/elasticsearch/cluster/metadata/MetadataDeleteIndexServiceTests.java

+51-2
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919

2020
package org.elasticsearch.cluster.metadata;
2121

22+
import org.elasticsearch.action.admin.indices.datastream.DeleteDataStreamRequestTests;
2223
import org.elasticsearch.cluster.ClusterName;
2324
import org.elasticsearch.cluster.ClusterState;
2425
import org.elasticsearch.cluster.SnapshotsInProgress;
2526
import org.elasticsearch.cluster.block.ClusterBlocks;
2627
import org.elasticsearch.cluster.routing.RoutingTable;
2728
import org.elasticsearch.cluster.routing.allocation.AllocationService;
2829
import org.elasticsearch.common.collect.ImmutableOpenMap;
30+
import org.elasticsearch.common.collect.Tuple;
2931
import org.elasticsearch.common.settings.Settings;
3032
import org.elasticsearch.index.Index;
3133
import org.elasticsearch.index.IndexNotFoundException;
@@ -36,18 +38,36 @@
3638
import org.elasticsearch.snapshots.SnapshotInfoTests;
3739
import org.elasticsearch.test.ESTestCase;
3840
import org.elasticsearch.test.VersionUtils;
41+
import org.hamcrest.core.IsNull;
42+
import org.junit.Before;
43+
44+
import java.util.List;
45+
import java.util.Locale;
46+
import java.util.Set;
3947

4048
import static java.util.Collections.singleton;
4149
import static java.util.Collections.singletonList;
50+
import static org.hamcrest.CoreMatchers.containsString;
51+
import static org.hamcrest.CoreMatchers.equalTo;
4252
import static org.mockito.Matchers.any;
4353
import static org.mockito.Mockito.mock;
4454
import static org.mockito.Mockito.verify;
4555
import static org.mockito.Mockito.when;
4656

4757

4858
public class MetadataDeleteIndexServiceTests extends ESTestCase {
49-
private final AllocationService allocationService = mock(AllocationService.class);
50-
private final MetadataDeleteIndexService service = new MetadataDeleteIndexService(Settings.EMPTY, null, allocationService);
59+
private AllocationService allocationService;
60+
private MetadataDeleteIndexService service;
61+
62+
@Override
63+
@Before
64+
public void setUp() throws Exception {
65+
super.setUp();
66+
allocationService = mock(AllocationService.class);
67+
when(allocationService.reroute(any(ClusterState.class), any(String.class)))
68+
.thenAnswer(mockInvocation -> mockInvocation.getArguments()[0]);
69+
service = new MetadataDeleteIndexService(Settings.EMPTY, null, allocationService);
70+
}
5171

5272
public void testDeleteMissing() {
5373
Index index = new Index("missing", "doesn't matter");
@@ -92,6 +112,35 @@ public void testDeleteUnassigned() {
92112
verify(allocationService).reroute(any(ClusterState.class), any(String.class));
93113
}
94114

115+
public void testDeleteBackingIndexForDataStream() {
116+
int numBackingIndices = randomIntBetween(2, 5);
117+
String dataStreamName = randomAlphaOfLength(6).toLowerCase(Locale.ROOT);
118+
ClusterState before = DeleteDataStreamRequestTests.getClusterStateWithDataStreams(
119+
List.of(new Tuple<>(dataStreamName, numBackingIndices)), List.of());
120+
121+
int numIndexToDelete = randomIntBetween(1, numBackingIndices - 1);
122+
123+
Index indexToDelete = before.metadata().index(DataStream.getBackingIndexName(dataStreamName, numIndexToDelete)).getIndex();
124+
ClusterState after = service.deleteIndices(before, Set.of(indexToDelete));
125+
126+
assertThat(after.metadata().getIndices().get(indexToDelete.getName()), IsNull.nullValue());
127+
assertThat(after.metadata().getIndices().size(), equalTo(numBackingIndices - 1));
128+
assertThat(after.metadata().getIndices().get(DataStream.getBackingIndexName(dataStreamName, numIndexToDelete)), IsNull.nullValue());
129+
}
130+
131+
public void testDeleteCurrentWriteIndexForDataStream() {
132+
int numBackingIndices = randomIntBetween(1, 5);
133+
String dataStreamName = randomAlphaOfLength(6).toLowerCase(Locale.ROOT);
134+
ClusterState before = DeleteDataStreamRequestTests.getClusterStateWithDataStreams(
135+
List.of(new Tuple<>(dataStreamName, numBackingIndices)), List.of());
136+
137+
Index indexToDelete = before.metadata().index(DataStream.getBackingIndexName(dataStreamName, numBackingIndices)).getIndex();
138+
Exception e = expectThrows(IllegalArgumentException.class, () -> service.deleteIndices(before, Set.of(indexToDelete)));
139+
140+
assertThat(e.getMessage(), containsString("index [" + indexToDelete.getName() + "] is the write index for data stream [" +
141+
dataStreamName + "] and cannot be deleted"));
142+
}
143+
95144
private ClusterState clusterState(String index) {
96145
IndexMetadata indexMetadata = IndexMetadata.builder(index)
97146
.settings(Settings.builder().put("index.version.created", VersionUtils.randomVersion(random())))

0 commit comments

Comments
 (0)