Improve same-shard allocation explanations (#56010)

I see occasional confusion about the explanations emitted by the same-shard
allocation decider, particularly amongst new users setting up a single-node
cluster and trying to determine why their cluster has `yellow` health. For
example:

    the shard cannot be allocated to the same node on which a copy of the shard
    already exists

This is technically correct but it's quite a complicated sentence. Also, by
starting with "the shard cannot be allocated" it makes it sound like this is
the problem, whereas in fact this message is a good thing and users should
typically focus their attention elsewhere.

This commit simplifies the wording of these messages and makes them sound more
positive, for example:

    a copy of this shard is already allocated to this node
This commit is contained in:
David Turner 2020-04-30 16:58:06 +01:00
parent fbba65d8b3
commit 69f50fe79f
3 changed files with 9 additions and 12 deletions

View File

@ -239,7 +239,7 @@ allocation:
{ {
"decider" : "same_shard", "decider" : "same_shard",
"decision" : "NO", "decision" : "NO",
"explanation" : "the shard cannot be allocated to the same node on which a copy of the shard already exists [[my_index][0], node[3sULLVJrRneSg0EfBB-2Ew], [P], s[STARTED], a[id=eV9P8BN1QPqRc3B4PLx6cg]]" "explanation" : "a copy of this shard is already allocated to this node [[my_index][0], node[3sULLVJrRneSg0EfBB-2Ew], [P], s[STARTED], a[id=eV9P8BN1QPqRc3B4PLx6cg]]"
} }
] ]
} }

View File

@ -97,16 +97,15 @@ public class SameShardAllocationDecider extends AllocationDecider {
String hostType = checkNodeOnSameHostAddress ? "address" : "name"; String hostType = checkNodeOnSameHostAddress ? "address" : "name";
String host = checkNodeOnSameHostAddress ? node.node().getHostAddress() : node.node().getHostName(); String host = checkNodeOnSameHostAddress ? node.node().getHostAddress() : node.node().getHostName();
return allocation.decision(Decision.NO, NAME, return allocation.decision(Decision.NO, NAME,
"the shard cannot be allocated on host %s [%s], where it already exists on node [%s]; " + "a copy of this shard is already allocated to host %s [%s], on node [%s], and [%s] is [true] which " +
"set cluster setting [%s] to false to allow multiple nodes on the same host to hold the same " + "forbids more than one node on this host from holding a copy of this shard",
"shard copies",
hostType, host, node.nodeId(), CLUSTER_ROUTING_ALLOCATION_SAME_HOST_SETTING.getKey()); hostType, host, node.nodeId(), CLUSTER_ROUTING_ALLOCATION_SAME_HOST_SETTING.getKey());
} }
} }
} }
} }
} }
return allocation.decision(Decision.YES, NAME, "the shard does not exist on the same host"); return allocation.decision(Decision.YES, NAME, "none of the nodes on this host hold a copy of this shard");
} }
@Override @Override
@ -122,15 +121,15 @@ public class SameShardAllocationDecider extends AllocationDecider {
if (node.nodeId().equals(assignedShard.currentNodeId())) { if (node.nodeId().equals(assignedShard.currentNodeId())) {
if (assignedShard.isSameAllocation(shardRouting)) { if (assignedShard.isSameAllocation(shardRouting)) {
return allocation.decision(Decision.NO, NAME, return allocation.decision(Decision.NO, NAME,
"the shard cannot be allocated to the node on which it already exists [%s]", "this shard is already allocated to this node [%s]",
shardRouting.toString()); shardRouting.toString());
} else { } else {
return allocation.decision(Decision.NO, NAME, return allocation.decision(Decision.NO, NAME,
"the shard cannot be allocated to the same node on which a copy of the shard already exists [%s]", "a copy of this shard is already allocated to this node [%s]",
assignedShard.toString()); assignedShard.toString());
} }
} }
} }
return allocation.decision(Decision.YES, NAME, "the shard does not exist on the same node"); return allocation.decision(Decision.YES, NAME, "this node does not hold a copy of this shard");
} }
} }

View File

@ -232,8 +232,7 @@ public final class ClusterAllocationExplainIT extends ESIntegTestCase {
for (Decision d : result.getCanAllocateDecision().getDecisions()) { for (Decision d : result.getCanAllocateDecision().getDecisions()) {
if (d.label().equals("same_shard") && nodeHoldingPrimary) { if (d.label().equals("same_shard") && nodeHoldingPrimary) {
assertEquals(Decision.Type.NO, d.type()); assertEquals(Decision.Type.NO, d.type());
assertThat(d.getExplanation(), startsWith( assertThat(d.getExplanation(), startsWith("a copy of this shard is already allocated to this node ["));
"the shard cannot be allocated to the same node on which a copy of the shard already exists"));
} else { } else {
assertEquals(Decision.Type.YES, d.type()); assertEquals(Decision.Type.YES, d.type());
assertNotNull(d.getExplanation()); assertNotNull(d.getExplanation());
@ -351,8 +350,7 @@ public final class ClusterAllocationExplainIT extends ESIntegTestCase {
for (Decision d : result.getCanAllocateDecision().getDecisions()) { for (Decision d : result.getCanAllocateDecision().getDecisions()) {
if (d.label().equals("same_shard") && nodeHoldingPrimary) { if (d.label().equals("same_shard") && nodeHoldingPrimary) {
assertEquals(Decision.Type.NO, d.type()); assertEquals(Decision.Type.NO, d.type());
assertThat(d.getExplanation(), startsWith( assertThat(d.getExplanation(), startsWith("a copy of this shard is already allocated to this node ["));
"the shard cannot be allocated to the same node on which a copy of the shard already exists"));
} else if (d.label().equals("filter") && nodeHoldingPrimary == false) { } else if (d.label().equals("filter") && nodeHoldingPrimary == false) {
assertEquals(Decision.Type.NO, d.type()); assertEquals(Decision.Type.NO, d.type());
assertEquals("node does not match index setting [index.routing.allocation.include] " + assertEquals("node does not match index setting [index.routing.allocation.include] " +