Skip to content

Commit 69f50fe

Browse files
committed
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
1 parent fbba65d commit 69f50fe

File tree

3 files changed

+9
-12
lines changed

3 files changed

+9
-12
lines changed

docs/reference/cluster/allocation-explain.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ allocation:
239239
{
240240
"decider" : "same_shard",
241241
"decision" : "NO",
242-
"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]]"
242+
"explanation" : "a copy of this shard is already allocated to this node [[my_index][0], node[3sULLVJrRneSg0EfBB-2Ew], [P], s[STARTED], a[id=eV9P8BN1QPqRc3B4PLx6cg]]"
243243
}
244244
]
245245
}

server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/SameShardAllocationDecider.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,15 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
9797
String hostType = checkNodeOnSameHostAddress ? "address" : "name";
9898
String host = checkNodeOnSameHostAddress ? node.node().getHostAddress() : node.node().getHostName();
9999
return allocation.decision(Decision.NO, NAME,
100-
"the shard cannot be allocated on host %s [%s], where it already exists on node [%s]; " +
101-
"set cluster setting [%s] to false to allow multiple nodes on the same host to hold the same " +
102-
"shard copies",
100+
"a copy of this shard is already allocated to host %s [%s], on node [%s], and [%s] is [true] which " +
101+
"forbids more than one node on this host from holding a copy of this shard",
103102
hostType, host, node.nodeId(), CLUSTER_ROUTING_ALLOCATION_SAME_HOST_SETTING.getKey());
104103
}
105104
}
106105
}
107106
}
108107
}
109-
return allocation.decision(Decision.YES, NAME, "the shard does not exist on the same host");
108+
return allocation.decision(Decision.YES, NAME, "none of the nodes on this host hold a copy of this shard");
110109
}
111110

112111
@Override
@@ -122,15 +121,15 @@ private Decision decideSameNode(ShardRouting shardRouting, RoutingNode node, Rou
122121
if (node.nodeId().equals(assignedShard.currentNodeId())) {
123122
if (assignedShard.isSameAllocation(shardRouting)) {
124123
return allocation.decision(Decision.NO, NAME,
125-
"the shard cannot be allocated to the node on which it already exists [%s]",
124+
"this shard is already allocated to this node [%s]",
126125
shardRouting.toString());
127126
} else {
128127
return allocation.decision(Decision.NO, NAME,
129-
"the shard cannot be allocated to the same node on which a copy of the shard already exists [%s]",
128+
"a copy of this shard is already allocated to this node [%s]",
130129
assignedShard.toString());
131130
}
132131
}
133132
}
134-
return allocation.decision(Decision.YES, NAME, "the shard does not exist on the same node");
133+
return allocation.decision(Decision.YES, NAME, "this node does not hold a copy of this shard");
135134
}
136135
}

server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,7 @@ public void testUnassignedReplicaDelayedAllocation() throws Exception {
232232
for (Decision d : result.getCanAllocateDecision().getDecisions()) {
233233
if (d.label().equals("same_shard") && nodeHoldingPrimary) {
234234
assertEquals(Decision.Type.NO, d.type());
235-
assertThat(d.getExplanation(), startsWith(
236-
"the shard cannot be allocated to the same node on which a copy of the shard already exists"));
235+
assertThat(d.getExplanation(), startsWith("a copy of this shard is already allocated to this node ["));
237236
} else {
238237
assertEquals(Decision.Type.YES, d.type());
239238
assertNotNull(d.getExplanation());
@@ -351,8 +350,7 @@ public void testUnassignedReplicaWithPriorCopy() throws Exception {
351350
for (Decision d : result.getCanAllocateDecision().getDecisions()) {
352351
if (d.label().equals("same_shard") && nodeHoldingPrimary) {
353352
assertEquals(Decision.Type.NO, d.type());
354-
assertThat(d.getExplanation(), startsWith(
355-
"the shard cannot be allocated to the same node on which a copy of the shard already exists"));
353+
assertThat(d.getExplanation(), startsWith("a copy of this shard is already allocated to this node ["));
356354
} else if (d.label().equals("filter") && nodeHoldingPrimary == false) {
357355
assertEquals(Decision.Type.NO, d.type());
358356
assertEquals("node does not match index setting [index.routing.allocation.include] " +

0 commit comments

Comments
 (0)