honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not update number of replicas on no indices (#34481) 

Today when submitting an update settings request to update the number of
replicas with a wildcard that does not match any indices and allow no
indices is set to true, the request ends up being interpreted as
updating the number of replicas for all indices. That is, consider the
following sequence:

PUT /test-index
{
  "settings": {
    "index.number_of_replicas": 0
  }
}

PUT /non-existent-*/_settings?expand_wildcards=open&allow_no_indices=true
{
  "settings": {
    "index.number_of_replicas": 1
  }
}

GET /test-index/_settings

The latter will show that the number of replicas on test-index is now
one. This is surprising, and should be considered a bug.

The underlying problem here is treating no indices in the underlying
methods used to update the routing table and the metadata as meaning all
indices. This commit takes away this assumption. Tests that relied on
this behavior have been changed to no longer rely on this.

A test for this situation is added in UpdateNumberOfReplicasIT.
This commit is contained in:
Jason Tedor 2018-10-15 19:49:58 -04:00 committed by GitHub
parent 23ece922c9
commit 55dee53046
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 48 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java
View File

@ -1034,10 +1034,14 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
return this;
}
public Builder updateNumberOfReplicas(int numberOfReplicas, String... indices) {
if (indices == null || indices.length == 0) {
indices = this.indices.keys().toArray(String.class);
}
/**
* Update the number of replicas for the specified indices.
*
* @param numberOfReplicas the number of replicas
* @param indices the indices to update the number of replicas for
* @return the builder
*/
public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] indices) {
for (String index : indices) {
IndexMetaData indexMetaData = this.indices.get(index);
if (indexMetaData == null) {

12
server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java
View File

@ -457,13 +457,17 @@ public class RoutingTable implements Iterable<IndexRoutingTable>, Diffable<Routi
return this;
}
public Builder updateNumberOfReplicas(int numberOfReplicas, String... indices) {
/**
* Update the number of replicas for the specified indices.
*
* @param numberOfReplicas the number of replicas
* @param indices the indices to update the number of replicas for
* @return the builder
*/
public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] indices) {
if (indicesRouting == null) {
throw new IllegalStateException("once build is called the builder cannot be reused");
}
if (indices == null || indices.length == 0) {
indices = indicesRouting.keys().toArray(String.class);
}
for (String index : indices) {
IndexRoutingTable indexRoutingTable = indicesRouting.get(index);
if (indexRoutingTable == null) {

2
server/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java
View File

@ -275,7 +275,7 @@ public class RoutingTableTests extends ESAllocationTestCase {
assertThat(e.getMessage(), containsString("cannot be reused"));
}
try {
b.updateNumberOfReplicas(1, "foo");
b.updateNumberOfReplicas(1, new String[]{"foo"});
fail("expected exception");
} catch (IllegalStateException e) {
assertThat(e.getMessage(), containsString("cannot be reused"));

4
server/src/test/java/org/elasticsearch/cluster/routing/allocation/InSyncAllocationIdTests.java
View File

@ -292,8 +292,8 @@ public class InSyncAllocationIdTests extends ESAllocationTestCase {
logger.info("decrease number of replicas to 0");
clusterState = ClusterState.builder(clusterState)
.routingTable(RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(0, "test").build())
.metaData(MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(0, "test")).build();
.routingTable(RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(0, new String[]{"test"}).build())
.metaData(MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(0, new String[]{"test"})).build();
logger.info("add back node 1");
clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder().add(

6
server/src/test/java/org/elasticsearch/cluster/routing/allocation/PreferPrimaryAllocationTests.java
View File

@ -68,8 +68,10 @@ public class PreferPrimaryAllocationTests extends ESAllocationTestCase {
}
logger.info("increasing the number of replicas to 1, and perform a reroute (to get the replicas allocation going)");
RoutingTable updatedRoutingTable = RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(1).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(1).build();
final String[] indices = {"test1", "test2"};
RoutingTable updatedRoutingTable =
RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(1, indices).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(1, indices).build();
clusterState = ClusterState.builder(clusterState).routingTable(updatedRoutingTable).metaData(metaData).build();
clusterState = strategy.reroute(clusterState, "reroute");

10
server/src/test/java/org/elasticsearch/cluster/routing/allocation/UpdateNumberOfReplicasTests.java
View File

@ -96,8 +96,10 @@ public class UpdateNumberOfReplicasTests extends ESAllocationTestCase {
logger.info("add another replica");
routingNodes = clusterState.getRoutingNodes();
RoutingTable updatedRoutingTable = RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(2).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(2).build();
final String[] indices = {"test"};
RoutingTable updatedRoutingTable =
RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(2, indices).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(2, indices).build();
clusterState = ClusterState.builder(clusterState).routingTable(updatedRoutingTable).metaData(metaData).build();
assertThat(clusterState.metaData().index("test").getNumberOfReplicas(), equalTo(2));
@ -143,8 +145,8 @@ public class UpdateNumberOfReplicasTests extends ESAllocationTestCase {
logger.info("now remove a replica");
routingNodes = clusterState.getRoutingNodes();
updatedRoutingTable = RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(1).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(1).build();
updatedRoutingTable = RoutingTable.builder(clusterState.routingTable()).updateNumberOfReplicas(1, indices).build();
metaData = MetaData.builder(clusterState.metaData()).updateNumberOfReplicas(1, indices).build();
clusterState = ClusterState.builder(clusterState).routingTable(updatedRoutingTable).metaData(metaData).build();
assertThat(clusterState.metaData().index("test").getNumberOfReplicas(), equalTo(1));

19
server/src/test/java/org/elasticsearch/indices/settings/UpdateNumberOfReplicasIT.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.indices.settings;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.Priority;
@ -28,6 +29,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESIntegTestCase;
import java.io.IOException;
import java.util.EnumSet;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
@ -274,4 +276,21 @@ public class UpdateNumberOfReplicasIT extends ESIntegTestCase {
assertEquals("Failed to parse value [" + value + "] for setting [index.number_of_replicas] must be >= 0", e.getMessage());
}
}
public void testUpdateNumberOfReplicasAllowNoIndices() {
createIndex("test-index", Settings.builder().put("index.number_of_replicas", 0).build());
final IndicesOptions options =
new IndicesOptions(EnumSet.of(IndicesOptions.Option.ALLOW_NO_INDICES), EnumSet.of(IndicesOptions.WildcardStates.OPEN));
assertAcked(client()
.admin()
.indices()
.prepareUpdateSettings("non-existent-*")
.setSettings(Settings.builder().put("index.number_of_replicas", 1))
.setIndicesOptions(options)
.get());
final int numberOfReplicas = Integer.parseInt(
client().admin().indices().prepareGetSettings("test-index").get().getSetting("test-index", "index.number_of_replicas"));
assertThat(numberOfReplicas, equalTo(0));
}
}