From b260406ecf6f05228e6e57e5217cc338a27932c8 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Thu, 1 Nov 2018 14:38:00 -0700 Subject: [PATCH] ILM: use node _id attribute when allocating to one node (#35061) ILM's Shrink Action was using a nodes "_name" attribute to allocate to prepare for the shrink step. Since the name is configurable by a user and may use the same name for multiple nodes on one machine, _id is safer since it is guaranteed to be unique. closes #35043. --- .../SetSingleNodeAllocateStep.java | 12 +++---- .../xpack/core/indexlifecycle/ShrinkStep.java | 8 ++--- .../SetSingleNodeAllocateStepTests.java | 36 +++++++++---------- .../core/indexlifecycle/ShrinkStepTests.java | 2 +- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStep.java index 602ca6cbeb7..0caa2178417 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStep.java @@ -41,7 +41,7 @@ public class SetSingleNodeAllocateStep extends AsyncActionStep { public void performAction(IndexMetaData indexMetaData, ClusterState clusterState, Listener listener) { RoutingAllocation allocation = new RoutingAllocation(ALLOCATION_DECIDERS, clusterState.getRoutingNodes(), clusterState, null, System.nanoTime()); - List validNodeNames = new ArrayList<>(); + List validNodeIds = new ArrayList<>(); Optional anyShard = clusterState.getRoutingTable().allShards(indexMetaData.getIndex().getName()).stream().findAny(); if (anyShard.isPresent()) { // Iterate through the nodes finding ones that are acceptable for the current allocation rules of the shard @@ -50,15 +50,15 @@ public class SetSingleNodeAllocateStep extends AsyncActionStep { .type() == Decision.Type.YES; if (canRemainOnCurrentNode) { DiscoveryNode discoveryNode = node.node(); - validNodeNames.add(discoveryNode.getName()); + validNodeIds.add(discoveryNode.getId()); } } // Shuffle the list of nodes so the one we pick is random - Randomness.shuffle(validNodeNames); - Optional nodeName = validNodeNames.stream().findAny(); - if (nodeName.isPresent()) { + Randomness.shuffle(validNodeIds); + Optional nodeId = validNodeIds.stream().findAny(); + if (nodeId.isPresent()) { Settings settings = Settings.builder() - .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", nodeName.get()).build(); + .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", nodeId.get()).build(); UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexMetaData.getIndex().getName()) .settings(settings); getClient().admin().indices().updateSettings(updateSettingsRequest, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java index eb18fe98876..8c7adcb62ff 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStep.java @@ -49,7 +49,7 @@ public class ShrinkStep extends AsyncActionStep { .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, numberOfShards) .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, indexMetaData.getNumberOfReplicas()) .put(LifecycleSettings.LIFECYCLE_NAME, lifecycle) - .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", (String) null) // need to remove the single shard + .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", (String) null) // need to remove the single shard // allocation so replicas can be allocated .build(); @@ -71,7 +71,7 @@ public class ShrinkStep extends AsyncActionStep { public int hashCode() { return Objects.hash(super.hashCode(), numberOfShards, shrunkIndexPrefix); } - + @Override public boolean equals(Object obj) { if (obj == null) { @@ -81,8 +81,8 @@ public class ShrinkStep extends AsyncActionStep { return false; } ShrinkStep other = (ShrinkStep) obj; - return super.equals(obj) && - Objects.equals(numberOfShards, other.numberOfShards) && + return super.equals(obj) && + Objects.equals(numberOfShards, other.numberOfShards) && Objects.equals(shrunkIndexPrefix, other.shrunkIndexPrefix); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java index 7b8eed38b4c..b42ada6956f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java @@ -99,7 +99,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames = new HashSet<>(); + Set validNodeIds = new HashSet<>(); Settings validNodeSettings = Settings.EMPTY; DiscoveryNodes.Builder nodes = DiscoveryNodes.builder(); int numNodes = randomIntBetween(1, 20); @@ -107,13 +107,13 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames = new HashSet<>(); + Set validNodeIds = new HashSet<>(); Settings validNodeSettings = Settings.EMPTY; DiscoveryNodes.Builder nodes = DiscoveryNodes.builder(); int numNodes = randomIntBetween(1, 20); @@ -141,10 +141,10 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames = new HashSet<>(); + Set validNodeIds = new HashSet<>(); Settings validNodeSettings = Settings.builder().put(Node.NODE_ATTRIBUTES.getKey() + validAttr[0], validAttr[1]).build(); Settings invalidNodeSettings = Settings.builder().put(Node.NODE_ATTRIBUTES.getKey() + invalidAttr[0], invalidAttr[1]).build(); DiscoveryNodes.Builder nodes = DiscoveryNodes.builder(); @@ -166,9 +166,9 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames = new HashSet<>(); + Set validNodeIds = new HashSet<>(); Settings validNodeSettings = Settings.EMPTY; DiscoveryNodes.Builder nodes = DiscoveryNodes.builder(); int numNodes = randomIntBetween(1, 20); @@ -227,7 +227,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase indices = ImmutableOpenMap. builder().fPut(index.getName(), @@ -253,7 +253,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; assertSettingsRequestContainsValueFrom(request, - IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true, + IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", validNodeIds, true, indexMetaData.getIndex().getName()); listener.onFailure(exception); return null; @@ -296,7 +296,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames = new HashSet<>(); + Set validNodeIds = new HashSet<>(); Settings validNodeSettings = Settings.EMPTY; DiscoveryNodes.Builder nodes = DiscoveryNodes.builder(); int numNodes = randomIntBetween(1, 20); @@ -308,7 +308,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase indices = ImmutableOpenMap. builder().fPut(index.getName(), @@ -341,7 +341,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames, DiscoveryNodes.Builder nodes) throws IOException { + Set validNodeIds, DiscoveryNodes.Builder nodes) throws IOException { ImmutableOpenMap.Builder indices = ImmutableOpenMap. builder().fPut(index.getName(), indexMetaData); IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index) @@ -365,7 +365,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; assertSettingsRequestContainsValueFrom(request, - IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true, + IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", validNodeIds, true, indexMetaData.getIndex().getName()); listener.onResponse(new AcknowledgedResponse(true)); return null; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java index 820d1d15ded..472c22025e1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ShrinkStepTests.java @@ -117,7 +117,7 @@ public class ShrinkStepTests extends AbstractStepTestCase { .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, step.getNumberOfShards()) .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, sourceIndexMetaData.getNumberOfReplicas()) .put(LifecycleSettings.LIFECYCLE_NAME, lifecycleName) - .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", (String) null) + .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", (String) null) .build())); assertThat(request.getTargetIndexRequest().settings() .getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS, -1), equalTo(step.getNumberOfShards()));