From 2b422ce8f67abbf2ce3694fb51f6ff20226e77ee Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 18:39:10 +0100 Subject: [PATCH 01/10] Strengthen FilterRoutingTests Today the FilterRoutingTests take the belt-and-braces approach of excluding some node attribute values and including some others. This means that we don't really test that both inclusion and exclusion work correctly: as long as one of them works as expected then the test will pass. This change improves these tests by only using one approach at once, demonstrating that both do indeed work. --- .../decider/FilterAllocationDecider.java | 8 ++-- .../allocation/FilterRoutingTests.java | 46 +++++++++++-------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java index f3146f6f771e8..df623aa8a5e07 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java @@ -158,13 +158,13 @@ private Decision shouldFilter(IndexMetaData indexMd, RoutingNode node, RoutingAl private Decision shouldIndexFilter(IndexMetaData indexMd, RoutingNode node, RoutingAllocation allocation) { if (indexMd.requireFilters() != null) { - if (!indexMd.requireFilters().match(node.node())) { + if (indexMd.requireFilters().match(node.node()) == false) { return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]", IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_PREFIX, indexMd.requireFilters()); } } if (indexMd.includeFilters() != null) { - if (!indexMd.includeFilters().match(node.node())) { + if (indexMd.includeFilters().match(node.node()) == false) { return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]", IndexMetaData.INDEX_ROUTING_INCLUDE_GROUP_PREFIX, indexMd.includeFilters()); } @@ -180,13 +180,13 @@ private Decision shouldIndexFilter(IndexMetaData indexMd, RoutingNode node, Rout private Decision shouldClusterFilter(RoutingNode node, RoutingAllocation allocation) { if (clusterRequireFilters != null) { - if (!clusterRequireFilters.match(node.node())) { + if (clusterRequireFilters.match(node.node()) == false) { return allocation.decision(Decision.NO, NAME, "node does not match cluster setting [%s] filters [%s]", CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX, clusterRequireFilters); } } if (clusterIncludeFilters != null) { - if (!clusterIncludeFilters.match(node.node())) { + if (clusterIncludeFilters.match(node.node()) == false) { return allocation.decision(Decision.NO, NAME, "node does not cluster setting [%s] filters [%s]", CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX, clusterIncludeFilters); } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 79473759f8f72..0c4f09a916087 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.cluster.routing.allocation; -import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ESAllocationTestCase; @@ -30,8 +29,8 @@ import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; -import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.Settings.Builder; import org.hamcrest.Matchers; import java.util.List; @@ -42,13 +41,15 @@ import static org.hamcrest.Matchers.equalTo; public class FilterRoutingTests extends ESAllocationTestCase { - private final Logger logger = Loggers.getLogger(FilterRoutingTests.class); public void testClusterFilters() { - AllocationService strategy = createAllocationService(Settings.builder() - .put("cluster.routing.allocation.include.tag1", "value1,value2") - .put("cluster.routing.allocation.exclude.tag1", "value3,value4") - .build()); + Builder settingsBuilder = Settings.builder(); + if (randomBoolean()) { + settingsBuilder.put("cluster.routing.allocation.include.tag1", "value1,value2"); + } else { + settingsBuilder.put("cluster.routing.allocation.exclude.tag1", "value3,value4"); + } + AllocationService strategy = createAllocationService(settingsBuilder.build()); logger.info("Building initial routing table"); @@ -92,14 +93,13 @@ public void testIndexFilters() { logger.info("Building initial routing table"); - MetaData initialMetaData = MetaData.builder() - .put(IndexMetaData.builder("test").settings(settings(Version.CURRENT) - .put("index.number_of_shards", 2) - .put("index.number_of_replicas", 1) - .put("index.routing.allocation.include.tag1", "value1,value2") - .put("index.routing.allocation.exclude.tag1", "value3,value4") - .build())) - .build(); + final Settings.Builder settingsBuilder = settings(Version.CURRENT).put("index.number_of_shards", 2).put("index.number_of_replicas", 1); + if (randomBoolean()) { + settingsBuilder.put("index.routing.allocation.include.tag1", "value1,value2"); + } else { + settingsBuilder.put("index.routing.allocation.exclude.tag1", "value3,value4"); + } + final MetaData initialMetaData = MetaData.builder().put(IndexMetaData.builder("test").settings(settingsBuilder.build())).build(); RoutingTable initialRoutingTable = RoutingTable.builder() .addAsNew(initialMetaData.index("test")) @@ -132,13 +132,19 @@ public void testIndexFilters() { logger.info("--> switch between value2 and value4, shards should be relocating"); - IndexMetaData existingMetaData = clusterState.metaData().index("test"); - MetaData updatedMetaData = MetaData.builder() - .put(IndexMetaData.builder(existingMetaData).settings(Settings.builder().put(existingMetaData.getSettings()) + final IndexMetaData existingMetaData = clusterState.metaData().index("test"); + final Settings.Builder updatedSettingsBuilder = Settings.builder().put(existingMetaData.getSettings()); + if (randomBoolean()) { + updatedSettingsBuilder .put("index.routing.allocation.include.tag1", "value1,value4") + .putNull("index.routing.allocation.exclude.tag1"); + } else { + updatedSettingsBuilder .put("index.routing.allocation.exclude.tag1", "value2,value3") - .build())) - .build(); + .putNull("index.routing.allocation.include.tag1"); + } + final MetaData updatedMetaData + = MetaData.builder().put(IndexMetaData.builder(existingMetaData).settings(updatedSettingsBuilder.build())).build(); clusterState = ClusterState.builder(clusterState).metaData(updatedMetaData).build(); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(ShardRoutingState.STARTED).size(), equalTo(2)); From 5cca61ba2799381da53712946dfe614e67d3f274 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 21:53:20 +0100 Subject: [PATCH 02/10] Whitespace --- .../allocation/FilterRoutingTests.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 0c4f09a916087..053246244cbac 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -54,21 +54,21 @@ public void testClusterFilters() { logger.info("Building initial routing table"); MetaData metaData = MetaData.builder() - .put(IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(1)) - .build(); + .put(IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(1)) + .build(); RoutingTable initialRoutingTable = RoutingTable.builder() - .addAsNew(metaData.index("test")) - .build(); + .addAsNew(metaData.index("test")) + .build(); ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); logger.info("--> adding four nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() - .add(newNode("node1", singletonMap("tag1", "value1"))) - .add(newNode("node2", singletonMap("tag1", "value2"))) - .add(newNode("node3", singletonMap("tag1", "value3"))) - .add(newNode("node4", singletonMap("tag1", "value4"))) + .add(newNode("node1", singletonMap("tag1", "value1"))) + .add(newNode("node2", singletonMap("tag1", "value2"))) + .add(newNode("node3", singletonMap("tag1", "value3"))) + .add(newNode("node4", singletonMap("tag1", "value4"))) ).build(); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(2)); @@ -89,7 +89,7 @@ public void testClusterFilters() { public void testIndexFilters() { AllocationService strategy = createAllocationService(Settings.builder() - .build()); + .build()); logger.info("Building initial routing table"); @@ -102,17 +102,17 @@ public void testIndexFilters() { final MetaData initialMetaData = MetaData.builder().put(IndexMetaData.builder("test").settings(settingsBuilder.build())).build(); RoutingTable initialRoutingTable = RoutingTable.builder() - .addAsNew(initialMetaData.index("test")) - .build(); + .addAsNew(initialMetaData.index("test")) + .build(); ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(initialMetaData).routingTable(initialRoutingTable).build(); logger.info("--> adding two nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() - .add(newNode("node1", singletonMap("tag1", "value1"))) - .add(newNode("node2", singletonMap("tag1", "value2"))) - .add(newNode("node3", singletonMap("tag1", "value3"))) - .add(newNode("node4", singletonMap("tag1", "value4"))) + .add(newNode("node1", singletonMap("tag1", "value1"))) + .add(newNode("node2", singletonMap("tag1", "value2"))) + .add(newNode("node3", singletonMap("tag1", "value3"))) + .add(newNode("node4", singletonMap("tag1", "value4"))) ).build(); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(2)); @@ -166,14 +166,14 @@ public void testConcurrentRecoveriesAfterShardsCannotRemainOnNode() { logger.info("Building initial routing table"); MetaData metaData = MetaData.builder() - .put(IndexMetaData.builder("test1").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(0)) - .put(IndexMetaData.builder("test2").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(0)) - .build(); + .put(IndexMetaData.builder("test1").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(0)) + .put(IndexMetaData.builder("test2").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(0)) + .build(); RoutingTable initialRoutingTable = RoutingTable.builder() - .addAsNew(metaData.index("test1")) - .addAsNew(metaData.index("test2")) - .build(); + .addAsNew(metaData.index("test1")) + .addAsNew(metaData.index("test2")) + .build(); ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); @@ -193,9 +193,9 @@ public void testConcurrentRecoveriesAfterShardsCannotRemainOnNode() { logger.info("--> disable allocation for node1 and reroute"); strategy = createAllocationService(Settings.builder() - .put("cluster.routing.allocation.node_concurrent_recoveries", "1") - .put("cluster.routing.allocation.exclude.tag1", "value1") - .build()); + .put("cluster.routing.allocation.node_concurrent_recoveries", "1") + .put("cluster.routing.allocation.exclude.tag1", "value1") + .build()); logger.info("--> move shards from node1 to node2"); clusterState = strategy.reroute(clusterState, "reroute"); From a10333fcf7a21fbc789495c11c1311b62a7a1bac Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 21:53:35 +0100 Subject: [PATCH 03/10] Import --- .../cluster/routing/allocation/FilterRoutingTests.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 053246244cbac..7aa67c6c520ea 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -36,6 +36,7 @@ import java.util.List; import static java.util.Collections.singletonMap; +import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; import static org.hamcrest.Matchers.equalTo; @@ -61,7 +62,7 @@ public void testClusterFilters() { .addAsNew(metaData.index("test")) .build(); - ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); logger.info("--> adding four nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() @@ -105,7 +106,7 @@ public void testIndexFilters() { .addAsNew(initialMetaData.index("test")) .build(); - ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(initialMetaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(initialMetaData).routingTable(initialRoutingTable).build(); logger.info("--> adding two nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() @@ -175,7 +176,7 @@ public void testConcurrentRecoveriesAfterShardsCannotRemainOnNode() { .addAsNew(metaData.index("test2")) .build(); - ClusterState clusterState = ClusterState.builder(org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); logger.info("--> adding two nodes and performing rerouting"); DiscoveryNode node1 = newNode("node1", singletonMap("tag1", "value1")); From f9e56bed76d19273f0c58caa3e9d68ce4bc7bfc2 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 22:16:34 +0100 Subject: [PATCH 04/10] Repeat tests, rather than using randomBoolean() --- .../allocation/FilterRoutingTests.java | 89 ++++++++++++------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 7aa67c6c520ea..4dd65e123f9af 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -30,7 +30,6 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.Settings.Builder; import org.hamcrest.Matchers; import java.util.List; @@ -39,30 +38,39 @@ import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; +import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING; +import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING; import static org.hamcrest.Matchers.equalTo; public class FilterRoutingTests extends ESAllocationTestCase { - public void testClusterFilters() { - Builder settingsBuilder = Settings.builder(); - if (randomBoolean()) { - settingsBuilder.put("cluster.routing.allocation.include.tag1", "value1,value2"); - } else { - settingsBuilder.put("cluster.routing.allocation.exclude.tag1", "value3,value4"); - } - AllocationService strategy = createAllocationService(settingsBuilder.build()); + public void testClusterIncludeFilters() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2")); + } + + public void testClusterExcludeFilters() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4")); + } + + /** + * A test that creates a 2p1r index in a 4-node cluster and which expects the given allocation service's settings only to allocate + * the shards of this index to two of the nodes. + */ + private void testClusterFilters(Settings.Builder allocationServiceSettings) { + final AllocationService strategy = createAllocationService(allocationServiceSettings.build()); logger.info("Building initial routing table"); - MetaData metaData = MetaData.builder() + final MetaData metaData = MetaData.builder() .put(IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(2).numberOfReplicas(1)) .build(); - RoutingTable initialRoutingTable = RoutingTable.builder() + final RoutingTable initialRoutingTable = RoutingTable.builder() .addAsNew(metaData.index("test")) .build(); - ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metaData(metaData).routingTable(initialRoutingTable).build(); logger.info("--> adding four nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() @@ -81,26 +89,51 @@ public void testClusterFilters() { clusterState = strategy.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); logger.info("--> make sure shards are only allocated on tag1 with value1 and value2"); - List startedShards = clusterState.getRoutingNodes().shardsWithState(ShardRoutingState.STARTED); + final List startedShards = clusterState.getRoutingNodes().shardsWithState(ShardRoutingState.STARTED); assertThat(startedShards.size(), equalTo(4)); for (ShardRouting startedShard : startedShards) { assertThat(startedShard.currentNodeId(), Matchers.anyOf(equalTo("node1"), equalTo("node2"))); } } - public void testIndexFilters() { + public void testIndexIncludeFilters() { + testIndexFilters( + Settings.builder().put("index.routing.allocation.include.tag1", "value1,value2"), + Settings.builder().put("index.routing.allocation.include.tag1", "value1,value4")); + } + + public void testIndexExcludeFilters() { + testIndexFilters( + Settings.builder().put("index.routing.allocation.exclude.tag1", "value3,value4"), + Settings.builder().put("index.routing.allocation.exclude.tag1", "value2,value3")); + } + + public void testIndexIncludeThenExcludeFilters() { + testIndexFilters( + Settings.builder().put("index.routing.allocation.include.tag1", "value1,value2"), + Settings.builder().put("index.routing.allocation.exclude.tag1", "value2,value3") + .putNull("index.routing.allocation.include.tag1")); + } + + public void testIndexExcludeThenIncludeFilters() { + testIndexFilters( + Settings.builder().put("index.routing.allocation.exclude.tag1", "value3,value4"), + Settings.builder().put("index.routing.allocation.include.tag1", "value1,value4") + .putNull("index.routing.allocation.exclude.tag1")); + } + + /** + * A test that creates a 2p1r index in a 4-node cluster and expects the given index allocation settings only to allocate the shards + * to two of the nodes; on updating the index allocation settings the shards should be relocated. + */ + private void testIndexFilters(Settings.Builder initialIndexSettings, Settings.Builder updatedIndexSettings) { AllocationService strategy = createAllocationService(Settings.builder() .build()); logger.info("Building initial routing table"); - final Settings.Builder settingsBuilder = settings(Version.CURRENT).put("index.number_of_shards", 2).put("index.number_of_replicas", 1); - if (randomBoolean()) { - settingsBuilder.put("index.routing.allocation.include.tag1", "value1,value2"); - } else { - settingsBuilder.put("index.routing.allocation.exclude.tag1", "value3,value4"); - } - final MetaData initialMetaData = MetaData.builder().put(IndexMetaData.builder("test").settings(settingsBuilder.build())).build(); + final MetaData initialMetaData = MetaData.builder().put(IndexMetaData.builder("test").settings(settings(Version.CURRENT) + .put("index.number_of_shards", 2).put("index.number_of_replicas", 1).put(initialIndexSettings.build()))).build(); RoutingTable initialRoutingTable = RoutingTable.builder() .addAsNew(initialMetaData.index("test")) @@ -134,18 +167,10 @@ public void testIndexFilters() { logger.info("--> switch between value2 and value4, shards should be relocating"); final IndexMetaData existingMetaData = clusterState.metaData().index("test"); - final Settings.Builder updatedSettingsBuilder = Settings.builder().put(existingMetaData.getSettings()); - if (randomBoolean()) { - updatedSettingsBuilder - .put("index.routing.allocation.include.tag1", "value1,value4") - .putNull("index.routing.allocation.exclude.tag1"); - } else { - updatedSettingsBuilder - .put("index.routing.allocation.exclude.tag1", "value2,value3") - .putNull("index.routing.allocation.include.tag1"); - } final MetaData updatedMetaData - = MetaData.builder().put(IndexMetaData.builder(existingMetaData).settings(updatedSettingsBuilder.build())).build(); + = MetaData.builder().put(IndexMetaData.builder(existingMetaData).settings(Settings.builder() + .put(existingMetaData.getSettings()).put(updatedIndexSettings.build()).build())).build(); + clusterState = ClusterState.builder(clusterState).metaData(updatedMetaData).build(); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(ShardRoutingState.STARTED).size(), equalTo(2)); From 046757e55f81f7cb47e7b7866defee654622800c Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 22:21:36 +0100 Subject: [PATCH 05/10] Use proper settings rather than strings --- .../allocation/FilterRoutingTests.java | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 4dd65e123f9af..04daa9b1afe7b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -36,10 +36,14 @@ import static java.util.Collections.singletonMap; import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; +import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_EXCLUDE_GROUP_SETTING; +import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_INCLUDE_GROUP_SETTING; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING; import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING; +import static org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider + .CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_RECOVERIES_SETTING; import static org.hamcrest.Matchers.equalTo; public class FilterRoutingTests extends ESAllocationTestCase { @@ -98,28 +102,28 @@ private void testClusterFilters(Settings.Builder allocationServiceSettings) { public void testIndexIncludeFilters() { testIndexFilters( - Settings.builder().put("index.routing.allocation.include.tag1", "value1,value2"), - Settings.builder().put("index.routing.allocation.include.tag1", "value1,value4")); + Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), + Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value4")); } public void testIndexExcludeFilters() { testIndexFilters( - Settings.builder().put("index.routing.allocation.exclude.tag1", "value3,value4"), - Settings.builder().put("index.routing.allocation.exclude.tag1", "value2,value3")); + Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), + Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value2,value3")); } public void testIndexIncludeThenExcludeFilters() { testIndexFilters( - Settings.builder().put("index.routing.allocation.include.tag1", "value1,value2"), - Settings.builder().put("index.routing.allocation.exclude.tag1", "value2,value3") - .putNull("index.routing.allocation.include.tag1")); + Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), + Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value2,value3") + .putNull(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1")); } public void testIndexExcludeThenIncludeFilters() { testIndexFilters( - Settings.builder().put("index.routing.allocation.exclude.tag1", "value3,value4"), - Settings.builder().put("index.routing.allocation.include.tag1", "value1,value4") - .putNull("index.routing.allocation.exclude.tag1")); + Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), + Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value4") + .putNull(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1")); } /** @@ -135,11 +139,12 @@ private void testIndexFilters(Settings.Builder initialIndexSettings, Settings.Bu final MetaData initialMetaData = MetaData.builder().put(IndexMetaData.builder("test").settings(settings(Version.CURRENT) .put("index.number_of_shards", 2).put("index.number_of_replicas", 1).put(initialIndexSettings.build()))).build(); - RoutingTable initialRoutingTable = RoutingTable.builder() + final RoutingTable initialRoutingTable = RoutingTable.builder() .addAsNew(initialMetaData.index("test")) .build(); - ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(initialMetaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metaData(initialMetaData).routingTable(initialRoutingTable).build(); logger.info("--> adding two nodes and performing rerouting"); clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() @@ -201,7 +206,8 @@ public void testConcurrentRecoveriesAfterShardsCannotRemainOnNode() { .addAsNew(metaData.index("test2")) .build(); - ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).metaData(metaData).routingTable(initialRoutingTable).build(); + ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metaData(metaData).routingTable(initialRoutingTable).build(); logger.info("--> adding two nodes and performing rerouting"); DiscoveryNode node1 = newNode("node1", singletonMap("tag1", "value1")); @@ -219,8 +225,8 @@ public void testConcurrentRecoveriesAfterShardsCannotRemainOnNode() { logger.info("--> disable allocation for node1 and reroute"); strategy = createAllocationService(Settings.builder() - .put("cluster.routing.allocation.node_concurrent_recoveries", "1") - .put("cluster.routing.allocation.exclude.tag1", "value1") + .put(CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_RECOVERIES_SETTING.getKey(), "1") + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value1") .build()); logger.info("--> move shards from node1 to node2"); From 3e1c19e9e6386784293a56126bae440cb372de45 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sat, 25 Aug 2018 22:24:45 +0100 Subject: [PATCH 06/10] LineLength is now ok --- buildSrc/src/main/resources/checkstyle_suppressions.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 420ed3b10b4c9..516cb80a1748c 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -467,7 +467,6 @@ - From a6a5839c596e9406bd476cea3205d70740a1e798 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sun, 26 Aug 2018 13:13:56 +0100 Subject: [PATCH 07/10] Add more tests and a test of the require filters too (at cluster level) --- .../allocation/FilterRoutingTests.java | 106 +++++++++++++++--- 1 file changed, 89 insertions(+), 17 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 04daa9b1afe7b..f0a47280dc52c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -32,8 +32,11 @@ import org.elasticsearch.common.settings.Settings; import org.hamcrest.Matchers; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_EXCLUDE_GROUP_SETTING; @@ -42,25 +45,100 @@ import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING; import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_INCLUDE_GROUP_SETTING; -import static org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider - .CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_RECOVERIES_SETTING; +import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_REQUIRE_GROUP_SETTING; +import static org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_RECOVERIES_SETTING; import static org.hamcrest.Matchers.equalTo; public class FilterRoutingTests extends ESAllocationTestCase { - public void testClusterIncludeFilters() { - testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2")); + public void testClusterIncludeFiltersSingleAttribute() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4")))); } - public void testClusterExcludeFilters() { - testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4")); + public void testClusterIncludeFiltersMultipleAttributes() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1") + .put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag2", "value2"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag2", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag2", "value4")))); + } + + public void testClusterIncludeFiltersOptionalAttribute() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap())) + .add(newNode("node4", attrMap()))); + } + + public void testClusterExcludeFiltersSingleAttribute() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4")))); + } + + public void testClusterExcludeFiltersMultipleAttributes() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3") + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag2", "value4"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag2", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag2", "value4")))); + } + + public void testClusterExcludeFiltersOptionalAttribute() { + testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap())) + .add(newNode("node2", attrMap())) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4")))); + } + + public void testClusterRequireFilters() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag1", "req1") + .put(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag2", "req2"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "req1", "tag2", "req2"))) + .add(newNode("node2", attrMap("tag1", "req1", "tag2", "req2"))) + .add(newNode("node3", attrMap("tag1", "req1"))) + .add(newNode("node4", attrMap("tag1", "other", "tag2", "req2")))); + } + + private static Map attrMap(String... keysValues) { + if (keysValues.length == 0) { + return emptyMap(); + } + if (keysValues.length == 2) { + return singletonMap(keysValues[0], keysValues[1]); + } + Map result = new HashMap<>(); + for (int i = 0; i < keysValues.length; i += 2) { + result.put(keysValues[i], keysValues[i + 1]); + } + return result; } /** - * A test that creates a 2p1r index in a 4-node cluster and which expects the given allocation service's settings only to allocate - * the shards of this index to two of the nodes. + * A test that creates a 2p1r index and which expects the given allocation service's settings only to allocate the shards of this index + * to `node1` and `node2`. */ - private void testClusterFilters(Settings.Builder allocationServiceSettings) { + private void testClusterFilters(Settings.Builder allocationServiceSettings, DiscoveryNodes.Builder nodes) { final AllocationService strategy = createAllocationService(allocationServiceSettings.build()); logger.info("Building initial routing table"); @@ -74,15 +152,9 @@ private void testClusterFilters(Settings.Builder allocationServiceSettings) { .build(); ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) - .metaData(metaData).routingTable(initialRoutingTable).build(); + .metaData(metaData).routingTable(initialRoutingTable).nodes(nodes).build(); - logger.info("--> adding four nodes and performing rerouting"); - clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() - .add(newNode("node1", singletonMap("tag1", "value1"))) - .add(newNode("node2", singletonMap("tag1", "value2"))) - .add(newNode("node3", singletonMap("tag1", "value3"))) - .add(newNode("node4", singletonMap("tag1", "value4"))) - ).build(); + logger.info("--> rerouting"); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(2)); From da728f003632eb0a38763210fe54769cca956a2a Mon Sep 17 00:00:00 2001 From: David Turner Date: Sun, 26 Aug 2018 13:31:11 +0100 Subject: [PATCH 08/10] Add tests for 'require' filters on index allocation --- .../allocation/FilterRoutingTests.java | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index f0a47280dc52c..5135b36cfe983 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -26,6 +26,8 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.node.DiscoveryNodes.Builder; +import org.elasticsearch.cluster.routing.RoutingNodes; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; @@ -41,6 +43,7 @@ import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_EXCLUDE_GROUP_SETTING; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_INCLUDE_GROUP_SETTING; +import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING; @@ -175,34 +178,73 @@ private void testClusterFilters(Settings.Builder allocationServiceSettings, Disc public void testIndexIncludeFilters() { testIndexFilters( Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), - Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value4")); + Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value4"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4"))) + .add(newNode("node5", attrMap())) + ); } public void testIndexExcludeFilters() { testIndexFilters( Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), - Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value2,value3")); + Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value2,value3"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap())) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4")))); } public void testIndexIncludeThenExcludeFilters() { testIndexFilters( Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value2"), Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value2,value3") - .putNull(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1")); + .putNull(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap("tag1", "value2"))) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap()))); } public void testIndexExcludeThenIncludeFilters() { testIndexFilters( Settings.builder().put(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), Settings.builder().put(INDEX_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "value1,value4") - .putNull(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1")); + .putNull(INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1"))) + .add(newNode("node2", attrMap())) + .add(newNode("node3", attrMap("tag1", "value3"))) + .add(newNode("node4", attrMap("tag1", "value4")))); + } + + public void testIndexRequireFilters() { + testIndexFilters( + Settings.builder() + .put(INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag1", "value1") + .put(INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag2", "value2"), + Settings.builder() + .putNull(INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag2") + .put(INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag3", "value3"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "value1", "tag2", "value2", "tag3", "value3"))) + .add(newNode("node2", attrMap("tag1", "value1", "tag2", "value2", "tag3", "other"))) + .add(newNode("node3", attrMap("tag1", "other", "tag2", "value2", "tag3", "other"))) + .add(newNode("node4", attrMap("tag1", "value1", "tag2", "other", "tag3", "value3"))) + .add(newNode("node5", attrMap("tag2", "value2", "tag3", "value3"))) + .add(newNode("node6", attrMap()))); } /** - * A test that creates a 2p1r index in a 4-node cluster and expects the given index allocation settings only to allocate the shards - * to two of the nodes; on updating the index allocation settings the shards should be relocated. + * A test that creates a 2p1r index and expects the given index allocation settings only to allocate the shards to `node1` and `node2`; + * on updating the index allocation settings the shards should be relocated to nodes `node1` and `node4`. */ - private void testIndexFilters(Settings.Builder initialIndexSettings, Settings.Builder updatedIndexSettings) { + private void testIndexFilters(Settings.Builder initialIndexSettings, Settings.Builder updatedIndexSettings, Builder nodesBuilder) { AllocationService strategy = createAllocationService(Settings.builder() .build()); @@ -216,15 +258,9 @@ private void testIndexFilters(Settings.Builder initialIndexSettings, Settings.Bu .build(); ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) - .metaData(initialMetaData).routingTable(initialRoutingTable).build(); + .metaData(initialMetaData).routingTable(initialRoutingTable).nodes(nodesBuilder).build(); - logger.info("--> adding two nodes and performing rerouting"); - clusterState = ClusterState.builder(clusterState).nodes(DiscoveryNodes.builder() - .add(newNode("node1", singletonMap("tag1", "value1"))) - .add(newNode("node2", singletonMap("tag1", "value2"))) - .add(newNode("node3", singletonMap("tag1", "value3"))) - .add(newNode("node4", singletonMap("tag1", "value4"))) - ).build(); + logger.info("--> rerouting"); clusterState = strategy.reroute(clusterState, "reroute"); assertThat(clusterState.getRoutingNodes().shardsWithState(INITIALIZING).size(), equalTo(2)); From dd7831b5f80de26b4a43fc8583df024c6f789764 Mon Sep 17 00:00:00 2001 From: David Turner Date: Sun, 26 Aug 2018 13:33:37 +0100 Subject: [PATCH 09/10] Import --- .../cluster/routing/allocation/FilterRoutingTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index 5135b36cfe983..debd6ce751a96 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.node.DiscoveryNodes.Builder; -import org.elasticsearch.cluster.routing.RoutingNodes; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; From 88c7af07e00fbbd07c332df3e4b0563262cf88ed Mon Sep 17 00:00:00 2001 From: David Turner Date: Sun, 26 Aug 2018 13:41:41 +0100 Subject: [PATCH 10/10] Add tests for wildcards and combinations of include/exclude filters --- .../allocation/FilterRoutingTests.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java index debd6ce751a96..86e8887688ff2 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/FilterRoutingTests.java @@ -82,6 +82,17 @@ public void testClusterIncludeFiltersOptionalAttribute() { .add(newNode("node4", attrMap()))); } + public void testClusterIncludeFiltersWildcards() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "*incl*") + .put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag2", "*incl*"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "do_include_this"))) + .add(newNode("node2", attrMap("tag2", "also_include_this"))) + .add(newNode("node3", attrMap("tag1", "exclude_this"))) + .add(newNode("node4", attrMap("tag2", "also_exclude_this")))); + } + public void testClusterExcludeFiltersSingleAttribute() { testClusterFilters(Settings.builder().put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "value3,value4"), DiscoveryNodes.builder() @@ -111,6 +122,28 @@ public void testClusterExcludeFiltersOptionalAttribute() { .add(newNode("node4", attrMap("tag1", "value4")))); } + public void testClusterExcludeFiltersWildcards() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag1", "*excl*") + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag2", "*excl*"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "do_include_this"))) + .add(newNode("node2", attrMap("tag2", "also_include_this"))) + .add(newNode("node3", attrMap("tag1", "exclude_this"))) + .add(newNode("node4", attrMap("tag2", "also_exclude_this")))); + } + + public void testClusterIncludeAndExcludeFilters() { + testClusterFilters(Settings.builder() + .put(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getKey() + "tag1", "*incl*") + .put(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getKey() + "tag2", "*excl*"), + DiscoveryNodes.builder() + .add(newNode("node1", attrMap("tag1", "do_include_this"))) + .add(newNode("node2", attrMap("tag1", "also_include_this", "tag2", "ok_by_tag2"))) + .add(newNode("node3", attrMap("tag1", "included_by_tag1", "tag2", "excluded_by_tag2"))) + .add(newNode("node4", attrMap("tag1", "excluded_by_tag1", "tag2", "included_by_tag2")))); + } + public void testClusterRequireFilters() { testClusterFilters(Settings.builder() .put(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "tag1", "req1")