Skip to content

Commit

Permalink
AutoCreate should work for non-primary system indices (#77045)
Browse files Browse the repository at this point in the history
Previously, we handled the case of a write request to a system index
alias without a backing index by auto-creating the primary index. This
had the unfortunate side-effect of making it impossible to auto-create
non-primary system indices. This commit fixes the bug so that we can
handle both cases.

* Add internal cluster test for system index auto create
* Allow auto-creation of non-primary indices for a system index pattern
* Use primary index if autocreate is called with system index alias name
  • Loading branch information
williamrandolph committed Sep 2, 2021
1 parent f70123f commit e04e5eb
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.action.admin.indices.create;

import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.indices.TestSystemIndexPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;

import java.util.Collection;

import static org.elasticsearch.indices.TestSystemIndexDescriptor.INDEX_NAME;
import static org.elasticsearch.indices.TestSystemIndexDescriptor.PRIMARY_INDEX_NAME;
import static org.hamcrest.Matchers.is;

@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0)
public class AutoCreateSystemIndexIT extends ESIntegTestCase {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return CollectionUtils.appendToCopy(super.nodePlugins(), TestSystemIndexPlugin.class);
}

public void testAutoCreatePrimaryIndex() throws Exception {
CreateIndexRequest request = new CreateIndexRequest(PRIMARY_INDEX_NAME);
client().execute(AutoCreateAction.INSTANCE, request).get();

GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(PRIMARY_INDEX_NAME).get();
assertThat(response.indices().length, is(1));
}

public void testAutoCreatePrimaryIndexFromAlias() throws Exception {
CreateIndexRequest request = new CreateIndexRequest(INDEX_NAME);
client().execute(AutoCreateAction.INSTANCE, request).get();

GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(PRIMARY_INDEX_NAME).get();
assertThat(response.indices().length, is(1));
}

public void testAutoCreateNonPrimaryIndex() throws Exception {
CreateIndexRequest request = new CreateIndexRequest(INDEX_NAME + "-2");
client().execute(AutoCreateAction.INSTANCE, request).get();

GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices(INDEX_NAME + "-2").get();
assertThat(response.indices().length, is(1));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
* settings when it is first used, when it is referenced via its alias.
*/
public void testSystemIndexIsAutoCreatedViaAlias() {
doCreateTest(() -> indexDoc(INDEX_NAME, "1", "foo", "bar"));
doCreateTest(() -> indexDoc(INDEX_NAME, "1", "foo", "bar"), PRIMARY_INDEX_NAME);
}

/**
Expand All @@ -61,15 +61,25 @@ public void testSystemIndexIsAutoCreatedViaAlias() {
* index name.
*/
public void testSystemIndexIsAutoCreatedViaConcreteName() {
doCreateTest(() -> indexDoc(PRIMARY_INDEX_NAME, "1", "foo", "bar"));
doCreateTest(() -> indexDoc(PRIMARY_INDEX_NAME, "1", "foo", "bar"), PRIMARY_INDEX_NAME);
}

/**
* Check that a system index is auto-created with the expected mappings and
* settings when it is first used, when it is referenced via its concrete
* index name.
*/
public void testNonPrimarySystemIndexIsAutoCreatedViaConcreteName() {
final String nonPrimarySystemIndex = INDEX_NAME + "-2";
doCreateTest(() -> indexDoc(nonPrimarySystemIndex, "1", "foo", "bar"), nonPrimarySystemIndex);
}

/**
* Check that a system index is created with the expected mappings and
* settings when it is explicitly created, when it is referenced via its alias.
*/
public void testCreateSystemIndexViaAlias() {
doCreateTest(() -> assertAcked(prepareCreate(INDEX_NAME)));
doCreateTest(() -> assertAcked(prepareCreate(INDEX_NAME)), PRIMARY_INDEX_NAME);
}

/**
Expand All @@ -78,21 +88,21 @@ public void testCreateSystemIndexViaAlias() {
* concrete index name.
*/
public void testCreateSystemIndexViaConcreteName() {
doCreateTest(() -> assertAcked(prepareCreate(PRIMARY_INDEX_NAME)));
doCreateTest(() -> assertAcked(prepareCreate(PRIMARY_INDEX_NAME)), PRIMARY_INDEX_NAME);
}

private void doCreateTest(Runnable runnable) {
private void doCreateTest(Runnable runnable, String concreteIndex) {
internalCluster().startNodes(1);

// Trigger the creation of the system index
runnable.run();
ensureGreen(INDEX_NAME);

assertMappingsAndSettings(TestSystemIndexDescriptor.getOldMappings());
assertMappingsAndSettings(TestSystemIndexDescriptor.getOldMappings(), concreteIndex);

// Remove the index and alias...
assertAcked(client().admin().indices().prepareAliases().removeAlias(PRIMARY_INDEX_NAME, INDEX_NAME).get());
assertAcked(client().admin().indices().prepareDelete(PRIMARY_INDEX_NAME));
assertAcked(client().admin().indices().prepareAliases().removeAlias(concreteIndex, INDEX_NAME).get());
assertAcked(client().admin().indices().prepareDelete(concreteIndex));

// ...so that we can check that the they will still be auto-created again,
// but this time with updated settings
Expand All @@ -101,26 +111,26 @@ private void doCreateTest(Runnable runnable) {
runnable.run();
ensureGreen(INDEX_NAME);

assertMappingsAndSettings(TestSystemIndexDescriptor.getNewMappings());
assertMappingsAndSettings(TestSystemIndexDescriptor.getNewMappings(), concreteIndex);
}

/**
* Fetch the mappings and settings for {@link TestSystemIndexDescriptor#INDEX_NAME} and verify that they match the expected values.
* Note that in the case of the mappings, this is just a dumb string comparison, so order of keys matters.
*/
private void assertMappingsAndSettings(String expectedMappings) {
private void assertMappingsAndSettings(String expectedMappings, String concreteIndex) {
final GetMappingsResponse getMappingsResponse = client().admin()
.indices()
.getMappings(new GetMappingsRequest().indices(INDEX_NAME))
.actionGet();

final ImmutableOpenMap<String, MappingMetadata> mappings = getMappingsResponse.getMappings();
assertThat(
"Expected mappings to contain a key for [" + PRIMARY_INDEX_NAME + "], but found: " + mappings.toString(),
mappings.containsKey(PRIMARY_INDEX_NAME),
"Expected mappings to contain a key for [" + concreteIndex + "], but found: " + mappings.toString(),
mappings.containsKey(concreteIndex),
equalTo(true)
);
final Map<String, Object> sourceAsMap = mappings.get(PRIMARY_INDEX_NAME).getSourceAsMap();
final Map<String, Object> sourceAsMap = mappings.get(concreteIndex).getSourceAsMap();

try {
assertThat(convertToXContent(sourceAsMap, XContentType.JSON).utf8ToString(), equalTo(expectedMappings));
Expand All @@ -133,7 +143,7 @@ private void assertMappingsAndSettings(String expectedMappings) {
.getSettings(new GetSettingsRequest().indices(INDEX_NAME))
.actionGet();

final Settings actual = getSettingsResponse.getIndexToSettings().get(PRIMARY_INDEX_NAME);
final Settings actual = getSettingsResponse.getIndexToSettings().get(concreteIndex);

for (String settingName : TestSystemIndexDescriptor.SETTINGS.keySet()) {
assertThat(actual.get(settingName), equalTo(TestSystemIndexDescriptor.SETTINGS.get(settingName)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public ClusterState execute(ClusterState currentState) throws Exception {
throw new IllegalStateException(message);
}

updateRequest = buildSystemIndexUpdateRequest(descriptor);
updateRequest = buildSystemIndexUpdateRequest(indexName, descriptor);
} else {
updateRequest = buildUpdateRequest(indexName);
}
Expand All @@ -187,11 +187,14 @@ private CreateIndexClusterStateUpdateRequest buildUpdateRequest(String indexName
return updateRequest;
}

private CreateIndexClusterStateUpdateRequest buildSystemIndexUpdateRequest(SystemIndexDescriptor descriptor) {
private CreateIndexClusterStateUpdateRequest buildSystemIndexUpdateRequest(
String indexName, SystemIndexDescriptor descriptor) {
String mappings = descriptor.getMappings();
Settings settings = descriptor.getSettings();
String aliasName = descriptor.getAliasName();
String concreteIndexName = descriptor.getPrimaryIndex();

// if we are writing to the alias name, we should create the primary index here
String concreteIndexName = indexName.equals(aliasName) ? descriptor.getPrimaryIndex() : indexName;

CreateIndexClusterStateUpdateRequest updateRequest =
new CreateIndexClusterStateUpdateRequest(request.cause(), concreteIndexName, request.index())
Expand All @@ -210,7 +213,13 @@ private CreateIndexClusterStateUpdateRequest buildSystemIndexUpdateRequest(Syste
updateRequest.aliases(Set.of(new Alias(aliasName)));
}

logger.debug("Auto-creating system index {}", concreteIndexName);
if (logger.isDebugEnabled()) {
if (concreteIndexName.equals(indexName) == false) {
logger.debug("Auto-creating backing system index {} for alias {}", concreteIndexName, indexName);
} else {
logger.debug("Auto-creating system index {}", concreteIndexName);
}
}

return updateRequest;
}
Expand Down

0 comments on commit e04e5eb

Please sign in to comment.