Browse Source

Correct check for write index and increment generation on all DS backing index operations (#79916)

Dan Hermann 4 years ago
parent
commit
c8a0dababd

+ 24 - 4
server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java

@@ -269,7 +269,17 @@ public final class DataStream extends AbstractDiffable<DataStream> implements To
         List<Index> backingIndices = new ArrayList<>(indices);
         backingIndices.remove(index);
         assert backingIndices.size() == indices.size() - 1;
-        return new DataStream(name, timeStampField, backingIndices, generation, metadata, hidden, replicated, system, allowCustomRouting);
+        return new DataStream(
+            name,
+            timeStampField,
+            backingIndices,
+            generation + 1,
+            metadata,
+            hidden,
+            replicated,
+            system,
+            allowCustomRouting
+        );
     }
 
     /**
@@ -290,18 +300,28 @@ public final class DataStream extends AbstractDiffable<DataStream> implements To
                 String.format(Locale.ROOT, "index [%s] is not part of data stream [%s]", existingBackingIndex.getName(), name)
             );
         }
-        if (generation == (backingIndexPosition + 1)) {
+        if (indices.size() == (backingIndexPosition + 1)) {
             throw new IllegalArgumentException(
                 String.format(
                     Locale.ROOT,
-                    "cannot replace backing index [%s] of data stream [%s] because " + "it is the write index",
+                    "cannot replace backing index [%s] of data stream [%s] because it is the write index",
                     existingBackingIndex.getName(),
                     name
                 )
             );
         }
         backingIndices.set(backingIndexPosition, newBackingIndex);
-        return new DataStream(name, timeStampField, backingIndices, generation, metadata, hidden, replicated, system, allowCustomRouting);
+        return new DataStream(
+            name,
+            timeStampField,
+            backingIndices,
+            generation + 1,
+            metadata,
+            hidden,
+            replicated,
+            system,
+            allowCustomRouting
+        );
     }
 
     /**

+ 19 - 4
server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java

@@ -99,7 +99,7 @@ public class DataStreamTests extends AbstractSerializingTestCase<DataStream> {
         DataStream original = new DataStream(dataStreamName, createTimestampField("@timestamp"), indices);
         DataStream updated = original.removeBackingIndex(indices.get(indexToRemove - 1));
         assertThat(updated.getName(), equalTo(original.getName()));
-        assertThat(updated.getGeneration(), equalTo(original.getGeneration()));
+        assertThat(updated.getGeneration(), equalTo(original.getGeneration() + 1));
         assertThat(updated.getTimeStampField(), equalTo(original.getTimeStampField()));
         assertThat(updated.getIndices().size(), equalTo(numBackingIndices - 1));
         for (int k = 0; k < (numBackingIndices - 1); k++) {
@@ -355,7 +355,7 @@ public class DataStreamTests extends AbstractSerializingTestCase<DataStream> {
         Index newBackingIndex = new Index("replacement-index", UUIDs.randomBase64UUID(random()));
         DataStream updated = original.replaceBackingIndex(indices.get(indexToReplace), newBackingIndex);
         assertThat(updated.getName(), equalTo(original.getName()));
-        assertThat(updated.getGeneration(), equalTo(original.getGeneration()));
+        assertThat(updated.getGeneration(), equalTo(original.getGeneration() + 1));
         assertThat(updated.getTimeStampField(), equalTo(original.getTimeStampField()));
         assertThat(updated.getIndices().size(), equalTo(numBackingIndices));
         assertThat(updated.getIndices().get(indexToReplace), equalTo(newBackingIndex));
@@ -391,10 +391,25 @@ public class DataStreamTests extends AbstractSerializingTestCase<DataStream> {
         for (int i = 1; i <= numBackingIndices; i++) {
             indices.add(new Index(DataStream.getDefaultBackingIndexName(dataStreamName, i), UUIDs.randomBase64UUID(random())));
         }
-        DataStream original = new DataStream(dataStreamName, createTimestampField("@timestamp"), indices);
+        int generation = randomBoolean() ? numBackingIndices : numBackingIndices + randomIntBetween(1, 5);
+        DataStream original = new DataStream(dataStreamName, createTimestampField("@timestamp"), indices, generation, null);
 
         Index newBackingIndex = new Index("replacement-index", UUIDs.randomBase64UUID(random()));
-        expectThrows(IllegalArgumentException.class, () -> original.replaceBackingIndex(indices.get(writeIndexPosition), newBackingIndex));
+        IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> original.replaceBackingIndex(indices.get(writeIndexPosition), newBackingIndex)
+        );
+        assertThat(
+            e.getMessage(),
+            equalTo(
+                String.format(
+                    Locale.ROOT,
+                    "cannot replace backing index [%s] of data stream [%s] because it is the write index",
+                    indices.get(writeIndexPosition).getName(),
+                    dataStreamName
+                )
+            )
+        );
     }
 
     public void testSnapshot() {

+ 2 - 0
x-pack/plugin/build.gradle

@@ -113,6 +113,8 @@ tasks.named("yamlRestTestV7CompatTransform").configure{ task ->
   task.skipTest("indices.freeze/10_basic/Basic", "#70192 -- the freeze index API is removed from 8.0")
   task.skipTest("indices.freeze/10_basic/Test index options", "#70192 -- the freeze index API is removed from 8.0")
   task.skipTest("ml/categorization_agg/Test categorization aggregation with poor settings", "https://github.com/elastic/elasticsearch/pull/79586")
+  task.skipTest("data_stream/50_delete_backing_indices/Delete backing index on data stream", "pending backport of https://github.com/elastic/elasticsearch/pull/79916")
+  task.skipTest("data_stream/170_modify_data_stream/Modify a data stream", "pending backport of https://github.com/elastic/elasticsearch/pull/79916")
   task.skipTest("service_accounts/10_basic/Test service account tokens", "mute till we decide whether to make the changes in 7.16")
 
   task.replaceValueInMatch("_type", "_doc")

+ 1 - 1
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/170_modify_data_stream.yml

@@ -79,7 +79,7 @@
         name: "data-stream-for-modification"
   - match: { data_streams.0.name: data-stream-for-modification }
   - match: { data_streams.0.timestamp_field.name: '@timestamp' }
-  - match: { data_streams.0.generation: 3 }
+  - match: { data_streams.0.generation: 4 }
   - length: { data_streams.0.indices: 2 }
   - match: { data_streams.0.indices.0.index_name: $first_index }
   - match: { data_streams.0.indices.1.index_name: $write_index }

+ 1 - 1
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/data_stream/50_delete_backing_indices.yml

@@ -62,7 +62,7 @@ setup:
         name: "*"
   - match: { data_streams.0.name: simple-data-stream }
   - match: { data_streams.0.timestamp_field.name: '@timestamp' }
-  - match: { data_streams.0.generation: 2 }
+  - match: { data_streams.0.generation: 3 }
   - length: { data_streams.0.indices: 1 }
   - match: { data_streams.0.indices.0.index_name: '/\.ds-simple-data-stream-(\d{4}\.\d{2}\.\d{2}-)?000002/' }