Primary system indices should be write indices for system aliases (#8…

…5977) (#86769) Whenever we create a system index whose descriptor has an alias, we add the alias. If the system index is the primary index, we make it the write index for the alias. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · May 12, 2022 · a146af1 · a146af1
1 parent 8d9ebfc
commit a146af1
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 7 deletions.
diff --git a/docs/changelog/85977.yaml b/docs/changelog/85977.yaml
@@ -0,0 +1,6 @@
+pr: 85977
+summary: "[DRAFT] Do not autocreate alias for non-primary system indices"
+area: Infra/Core
+type: bug
+issues:
+ - 85072
diff --git a/...usterTest/java/org/elasticsearch/action/admin/indices/create/AutoCreateSystemIndexIT.java b/...usterTest/java/org/elasticsearch/action/admin/indices/create/AutoCreateSystemIndexIT.java
@@ -8,12 +8,14 @@
 
 package org.elasticsearch.action.admin.indices.create;
 
+import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
 import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse;
 import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
 import org.elasticsearch.action.admin.indices.template.delete.DeleteComposableIndexTemplateAction;
 import org.elasticsearch.action.admin.indices.template.put.PutComposableIndexTemplateAction;
+import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.cluster.metadata.AliasMetadata;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
@@ -26,6 +28,7 @@
 import org.elasticsearch.plugins.SystemIndexPlugin;
 import org.elasticsearch.snapshots.SystemIndicesSnapshotIT;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.xcontent.XContentType;
 import org.junit.After;
 
 import java.util.Collection;
@@ -68,6 +71,12 @@ public void testAutoCreatePrimaryIndex() throws Exception {
 
         GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(PRIMARY_INDEX_NAME).get();
         assertThat(response.indices().length, is(1));
+        assertThat(response.aliases().size(), is(1));
+        assertThat(response.aliases().get(PRIMARY_INDEX_NAME).size(), is(1));
+        assertThat(
+            response.aliases().get(PRIMARY_INDEX_NAME).get(0),
+            equalTo(AliasMetadata.builder(INDEX_NAME).isHidden(true).writeIndex(true).build())
+        );
     }
 
     public void testAutoCreatePrimaryIndexFromAlias() throws Exception {
@@ -76,6 +85,12 @@ public void testAutoCreatePrimaryIndexFromAlias() throws Exception {
 
         GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(PRIMARY_INDEX_NAME).get();
         assertThat(response.indices().length, is(1));
+        assertThat(response.aliases().size(), is(1));
+        assertThat(response.aliases().get(PRIMARY_INDEX_NAME).size(), is(1));
+        assertThat(
+            response.aliases().get(PRIMARY_INDEX_NAME).get(0),
+            equalTo(AliasMetadata.builder(INDEX_NAME).isHidden(true).writeIndex(true).build())
+        );
     }
 
     public void testAutoCreateNonPrimaryIndex() throws Exception {
@@ -84,6 +99,43 @@ public void testAutoCreateNonPrimaryIndex() throws Exception {
 
         GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(INDEX_NAME + "-2").get();
         assertThat(response.indices().length, is(1));
+        assertThat(response.aliases().size(), is(1));
+        assertThat(response.aliases().get(INDEX_NAME + "-2").size(), is(1));
+        assertThat(response.aliases().get(INDEX_NAME + "-2").get(0), equalTo(AliasMetadata.builder(INDEX_NAME).isHidden(true).build()));
+    }
+
+    public void testWriteToAliasPrimaryAutoCreatedFirst() throws Exception {
+        {
+            CreateIndexRequest request = new CreateIndexRequest(PRIMARY_INDEX_NAME);
+            client().execute(AutoCreateAction.INSTANCE, request).get();
+        }
+
+        {
+            CreateIndexRequest request = new CreateIndexRequest(INDEX_NAME + "-2");
+            client().execute(AutoCreateAction.INSTANCE, request).get();
+        }
+
+        IndexResponse response = client().prepareIndex(INDEX_NAME).setSource("{\"foo\":\"bar\"}", XContentType.JSON).get();
+        assertThat(response.getResult(), equalTo(DocWriteResponse.Result.CREATED));
+    }
+
+    /**
+     * Like {@link #testWriteToAliasPrimaryAutoCreatedFirst()}, but with indices created in the opposite order
+     * @throws Exception
+     */
+    public void testWriteToAliasSecondaryAutoCreatedFirst() throws Exception {
+        {
+            CreateIndexRequest request = new CreateIndexRequest(INDEX_NAME + "-2");
+            client().execute(AutoCreateAction.INSTANCE, request).get();
+        }
+
+        {
+            CreateIndexRequest request = new CreateIndexRequest(PRIMARY_INDEX_NAME);
+            client().execute(AutoCreateAction.INSTANCE, request).get();
+        }
+
+        IndexResponse response = client().prepareIndex(INDEX_NAME).setSource("{\"foo\":\"bar\"}", XContentType.JSON).get();
+        assertThat(response.getResult(), equalTo(DocWriteResponse.Result.CREATED));
     }
 
     public void testSystemIndicesAutoCreatedAsHidden() throws Exception {

diff --git a/...ClusterTest/java/org/elasticsearch/action/admin/indices/create/CreateSystemIndicesIT.java b/...ClusterTest/java/org/elasticsearch/action/admin/indices/create/CreateSystemIndicesIT.java
@@ -54,6 +54,8 @@
 import static org.elasticsearch.test.XContentTestUtils.convertToXContent;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class CreateSystemIndicesIT extends ESIntegTestCase {
@@ -98,9 +100,29 @@ public void testSystemIndexIsAutoCreatedViaConcreteName() {
      * settings when it is first used, when it is referenced via its concrete
      * index name.
      */
-    public void testNonPrimarySystemIndexIsAutoCreatedViaConcreteName() {
+    public void testNonPrimarySystemIndexIsAutoCreatedViaConcreteName() throws Exception {
         final String nonPrimarySystemIndex = INDEX_NAME + "-2";
-        doCreateTest(() -> indexDoc(nonPrimarySystemIndex, "1", "foo", "bar"), nonPrimarySystemIndex);
+        internalCluster().startNodes(1);
+
+        // Trigger the creation of the system index
+        indexDoc(nonPrimarySystemIndex, "1", "foo", "bar");
+        ensureGreen(nonPrimarySystemIndex);
+
+        assertFalse(indexExists(PRIMARY_INDEX_NAME));
+        assertTrue(indexExists(INDEX_NAME + "-2"));
+
+        // Check that a non-primary system index is not assigned as the write index for the alias
+        final GetAliasesResponse getAliasesResponse = client().admin()
+            .indices()
+            .getAliases(new GetAliasesRequest().indicesOptions(IndicesOptions.strictExpandHidden()))
+            .actionGet();
+
+        assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
+        assertThat(getAliasesResponse.getAliases().get(nonPrimarySystemIndex).size(), equalTo(1));
+        assertThat(
+            getAliasesResponse.getAliases().get(nonPrimarySystemIndex).get(0),
+            equalTo(AliasMetadata.builder(INDEX_NAME).isHidden(true).build())
+        );
     }
 
     /**
@@ -245,6 +267,7 @@ private void assertAliases(String concreteIndex) {
         assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
         assertThat(getAliasesResponse.getAliases().get(concreteIndex).size(), equalTo(1));
         assertThat(getAliasesResponse.getAliases().get(concreteIndex).get(0).isHidden(), equalTo(true));
+        assertThat(getAliasesResponse.getAliases().get(concreteIndex).get(0).writeIndex(), equalTo(true));
     }
 
     private void assertHasAliases(Set<String> aliasNames) throws InterruptedException, java.util.concurrent.ExecutionException {
@@ -253,15 +276,20 @@ private void assertHasAliases(Set<String> aliasNames) throws InterruptedExceptio
             .getAliases(new GetAliasesRequest().indicesOptions(IndicesOptions.strictExpandHidden()))
             .get();
 
-        // Attempting to directly create a non-primary system index only creates the primary index
         assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
         assertThat(getAliasesResponse.getAliases().get(PRIMARY_INDEX_NAME).size(), equalTo(2));
         assertThat(
             getAliasesResponse.getAliases().get(PRIMARY_INDEX_NAME).stream().map(AliasMetadata::alias).collect(Collectors.toSet()),
             equalTo(aliasNames)
         );
-        assertThat(getAliasesResponse.getAliases().get(PRIMARY_INDEX_NAME).get(0).isHidden(), equalTo(true));
-        assertThat(getAliasesResponse.getAliases().get(PRIMARY_INDEX_NAME).get(1).isHidden(), equalTo(true));
+        for (AliasMetadata aliasMetadata : getAliasesResponse.getAliases().get(PRIMARY_INDEX_NAME)) {
+            assertThat(aliasMetadata.isHidden(), equalTo(true));
+            if (aliasMetadata.alias().equals(INDEX_NAME)) {
+                assertThat(aliasMetadata.writeIndex(), is(true));
+            } else {
+                assertThat(aliasMetadata.writeIndex(), is(nullValue()));
+            }
+        }
     }
 
     /**

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java
@@ -337,7 +337,11 @@ private CreateIndexClusterStateUpdateRequest buildSystemIndexUpdateRequest(Strin
                     updateRequest.settings(settings);
                 }
                 if (aliasName != null) {
-                    updateRequest.aliases(Set.of(new Alias(aliasName).isHidden(true)));
+                    Alias systemAlias = new Alias(aliasName).isHidden(true);
+                    if (concreteIndexName.equals(descriptor.getPrimaryIndex())) {
+                        systemAlias.writeIndex(true);
+                    }
+                    updateRequest.aliases(Set.of(systemAlias));
                 }
 
                 if (logger.isDebugEnabled()) {

diff --git a/...c/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java b/...c/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java
@@ -182,7 +182,7 @@ private static CreateIndexClusterStateUpdateRequest buildSystemIndexUpdateReques
         if (descriptor.getAliasName() == null) {
             aliases = Set.of();
         } else {
-            aliases = Set.of(new Alias(descriptor.getAliasName()).isHidden(true));
+            aliases = Set.of(new Alias(descriptor.getAliasName()).isHidden(true).writeIndex(true));
         }
 
         // Here, we override the user's requested index with the descriptor's primary index