Lab 4.4: Fix It — An AllocationDecider Edge Case

This is a Fix-It lab. You will work in org.opensearch.cluster.routing.allocation, understand how the AllocationService and the AllocationDeciders chain decide where shards go, then fix a realistic bug in a single decider — complete with a diff, a unit test that constructs a RoutingAllocation and asserts the resulting Decision, and a _cluster/allocation/explain reproduction on a live cluster.

Allocation is where "unassigned shards" issues live, and they are some of the most common real OpenSearch bugs. The skill is precise: a single decider returning the wrong Decision — or the right decision with a useless explanation — silently breaks recovery or balancing.

Background

When shards need placement (new index, a node left, a snapshot restored, a failed shard), the AllocationService runs a reroute. It builds a RoutingAllocation (the working context) and asks the AllocationDeciders chain whether each candidate placement is allowed. Each decider returns a Decision:

`Decision.Type`	Meaning
`YES`	This decider permits the placement
`NO`	This decider forbids it (with an explanation)
`THROTTLE`	Allowed eventually, but not right now (e.g. too many concurrent recoveries)

The chain's combined verdict is the most restrictive: any NO forbids; otherwise any THROTTLE throttles; otherwise YES. The explanation strings are what _cluster/allocation/explain shows the operator — so a wrong or unhelpful explanation is itself a bug.

find server/src/main/java -path "*routing/allocation*" -name "AllocationService.java"
find server/src/main/java -path "*routing/allocation/decider*" -name "*.java" | sort

Class	Role
`AllocationService`	`reroute(...)`, `applyStartedShards(...)`, `applyFailedShards(...)` — runs allocation
`RoutingAllocation`	Per-round context: `RoutingNodes`, `DiscoveryNodes`, `Metadata`, the deciders, `Decision.Debug` mode
`RoutingNodes`	Mutable shard→node assignment being computed
`AllocationDeciders`	The ordered chain; combines per-decider `Decision`s
`Decision`	`YES`/`NO`/`THROTTLE` + explanation
`BalancedShardsAllocator`	Proposes moves within what deciders allow

Deep-dive companion: shard-allocation.md. The cluster state that allocation reads/writes is from Lab 4.2.

Why This Lab Matters for Contributors

"My shard is UNASSIGNED and I don't know why" is one of the highest-volume issue categories in the tracker, and _cluster/allocation/explain is the first tool a maintainer reaches for. Its output is built directly from decider Decisions. Fixing a decider — getting the verdict and the explanation right, with a unit test that pins it — is a high-value, very mergeable kind of PR, and a frequent good first issue shape. This lab is the bridge from reading the allocation engine to changing it.

Prerequisites

OpenSearch builds; you can run ./gradlew :server:test.
You read Lab 4.2 (allocation runs as part of state computation).
A 2-node cluster for the live reproduction (Lab 3.2 setup), or a single node for the unit test.

Step 1 (15 min) — Read one decider end to end

We'll center on MaxRetryAllocationDecider, which stops OpenSearch from endlessly retrying a shard that keeps failing to allocate (after index.allocation.max_retries, default 5). It is small, real, and an excellent specimen of the "off-by-one + bad explanation" bug class.

find server/src/main/java -name "MaxRetryAllocationDecider.java"
sed -n '1,120p' server/src/main/java/org/opensearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java
grep -n "canAllocate\|getNumFailedAllocations\|maxRetries\|SETTING_ALLOCATION_MAX_RETRY\|Decision\|allocation.decision" \
  server/src/main/java/org/opensearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java

What to understand:

canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) is the hook. It reads the shard's UnassignedInfo, which tracks getNumFailedAllocations().
It compares failures against index.allocation.max_retries (SETTING_ALLOCATION_MAX_RETRY).
If failures have reached the limit, it returns Decision.NO with an explanation that includes the failure count, the limit, and a hint to retry via POST /_cluster/reroute?retry_failed=true.
Otherwise it returns Decision.YES (or allocation.decision(Decision.YES, NAME, ...)).

Read the real explanation string — it's the operator-facing contract you must not break.

Step 2 (10 min) — The bug

Suppose a contributor "simplified" the comparison and introduced an off-by-one that lets a shard be retried one time too many, and also degraded the explanation so it no longer tells the operator how to recover. Here is the planted regression as a diff (your starting point):

--- a/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java
+++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java
@@ public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
         final UnassignedInfo unassignedInfo = shardRouting.unassignedInfo();
         final int maxRetries = SETTING_ALLOCATION_MAX_RETRY.get(indexMetadata.getSettings());
         if (unassignedInfo != null && unassignedInfo.getNumFailedAllocations() > 0) {
             final int numFailedAllocations = unassignedInfo.getNumFailedAllocations();
-            if (numFailedAllocations >= maxRetries) {
-                return allocation.decision(Decision.NO, NAME,
-                    "shard has exceeded the maximum number of retries [%d] on failed allocation attempts - "
-                        + "manually call [%s] to retry, [%s]",
-                    maxRetries, RETRY_FAILED_API, unassignedInfo.toString());
+            if (numFailedAllocations > maxRetries) {
+                return allocation.decision(Decision.NO, NAME,
+                    "shard cannot be allocated");
             } else {
                 return allocation.decision(Decision.YES, NAME,
                     "shard has failed allocating [%d] times but [%d] retries are allowed",
                     numFailedAllocations, maxRetries);
             }
         }
         return allocation.decision(Decision.YES, NAME, "shard has no previous failures");

Two defects, both realistic:

Off-by-one: > maxRetries instead of >= maxRetries. With max_retries = 5, the shard is allowed a 6th attempt — one too many. The contract is "stop at the limit," i.e. >=.
Useless explanation: "shard cannot be allocated" drops the failure count, the limit, and the crucial recovery hint (?retry_failed=true). An operator running allocation/explain now learns nothing actionable.

Note: This is a teaching regression. The real MaxRetryAllocationDecider already uses >= and a rich explanation. You are practicing the fix-it motion on a class whose correct behavior you can verify against the actual source.

Step 3 (10 min) — The fix

@@ public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
             final int numFailedAllocations = unassignedInfo.getNumFailedAllocations();
-            if (numFailedAllocations > maxRetries) {
-                return allocation.decision(Decision.NO, NAME,
-                    "shard cannot be allocated");
+            if (numFailedAllocations >= maxRetries) {
+                return allocation.decision(Decision.NO, NAME,
+                    "shard has exceeded the maximum number of retries [%d] on failed allocation attempts - "
+                        + "manually call [%s] to retry, [%s]",
+                    maxRetries, RETRY_FAILED_API, unassignedInfo.toString());
             } else {

The fix restores the boundary (>=) and the actionable explanation (count, limit, retry API, and the full UnassignedInfo so the operator sees why it failed).

Pitfall — explanation strings are an API. _cluster/allocation/explain output and decider messages are consumed by operators, dashboards, and support tooling. Changing them casually breaks people's runbooks. When you fix the verdict, preserve (or improve, deliberately) the message — and note it in CHANGELOG.md.

Step 4 (20 min) — The unit test that pins the boundary

This is the heart of the lab: a test that constructs a RoutingAllocation with a shard that has exactly maxRetries failures and asserts the Decision is NO. Find the existing test to mirror its setup:

find server/src/test/java -name "MaxRetryAllocationDeciderTests.java"
grep -n "createOnFailedAllocation\|allocation\|canAllocate\|max_retries\|Decision\|RoutingAllocation\|UnassignedInfo" \
  server/src/test/java/org/opensearch/cluster/routing/allocation/decider/MaxRetryAllocationDeciderTests.java | head -40

The test shape (adapt names to your branch; helpers like createInitialClusterState / MockAllocationService already exist in the allocation test package):

package org.opensearch.cluster.routing.allocation.decider;

import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.routing.ShardRouting;
import org.opensearch.cluster.routing.allocation.RoutingAllocation;
import org.opensearch.cluster.routing.allocation.decider.Decision;
import org.opensearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider;

public class MaxRetryBoundaryTests extends OpenSearchAllocationTestCase {

    public void testDeniedExactlyAtMaxRetries() {
        int maxRetries = 5;

        // Build a cluster state with one index whose primary has failed exactly maxRetries times.
        ClusterState clusterState = createClusterStateWithFailedAllocations(
            "idx", maxRetries, maxRetries /* index.allocation.max_retries */);

        RoutingAllocation allocation = newRoutingAllocation(clusterState);
        allocation.debugDecision(true); // capture explanations for assertion

        ShardRouting unassignedPrimary = allocation.routingNodes()
            .unassigned().iterator().next();

        MaxRetryAllocationDecider decider = new MaxRetryAllocationDecider();
        Decision decision = decider.canAllocate(unassignedPrimary, allocation);

        // The boundary: at exactly maxRetries, the shard must be DENIED (>=, not >).
        assertEquals(Decision.Type.NO, decision.type());
        // The explanation must remain actionable.
        assertThat(decision.getExplanation(),
            containsString("exceeded the maximum number of retries"));
        assertThat(decision.getExplanation(),
            containsString("retry"));
    }

    public void testAllowedBelowMaxRetries() {
        int maxRetries = 5;
        ClusterState clusterState = createClusterStateWithFailedAllocations(
            "idx", maxRetries - 1, maxRetries);
        RoutingAllocation allocation = newRoutingAllocation(clusterState);

        ShardRouting unassignedPrimary = allocation.routingNodes()
            .unassigned().iterator().next();

        Decision decision = new MaxRetryAllocationDecider()
            .canAllocate(unassignedPrimary, allocation);

        assertEquals(Decision.Type.YES, decision.type());
    }
}

The two assertions are the whole point:

testDeniedExactlyAtMaxRetries fails on the buggy > (it would return YES at the boundary) and passes on the fixed >=. It also pins the explanation, so the "useless message" regression can't slip back in.
testAllowedBelowMaxRetries guards the other side: you didn't over-correct into denying valid retries.

Note: The real test class (MaxRetryAllocationDeciderTests) uses helpers from OpenSearchAllocationTestCase to build cluster states and routing allocations and to simulate failed allocations (applyFailedShards). Read it before writing your own — it shows the supported way to construct a RoutingAllocation with a shard that has N failures, which is fiddly to do by hand.

Run it:

./gradlew :server:test --tests "*MaxRetryBoundaryTests"
# or the real class while you study it:
./gradlew :server:test --tests "*MaxRetryAllocationDeciderTests"

A failing run against the buggy code, then a passing run after the fix, is your proof.

Step 5 (15 min) — Reproduce on a live cluster with allocation/explain

Unit tests pin the boundary; _cluster/allocation/explain shows the operator-visible behavior.

# On a 2-node cluster. Create an index that can't allocate a replica by forcing it onto a
# non-existent attribute, so the shard fails and retries accumulate.
curl -s -XPUT 'localhost:9200/retry-demo' -H 'Content-Type: application/json' -d '{
  "settings": {
    "number_of_shards": 1,
    "number_of_replicas": 1,
    "index.routing.allocation.require._name": "does-not-exist",
    "index.allocation.max_retries": 5
  }
}'

# Ask why the shard is unassigned:
curl -s -XGET 'localhost:9200/_cluster/allocation/explain?pretty' -H 'Content-Type: application/json' -d '{
  "index": "retry-demo", "shard": 0, "primary": true
}'

In the response, look at node_allocation_decisions[].deciders[]. You will find entries from each decider — including filter (the require._name you set) and, once retries accumulate, max_retry. The decision field is the Decision.Type and explanation is the string you just fixed:

{
  "decider": "max_retry",
  "decision": "NO",
  "explanation": "shard has exceeded the maximum number of retries [5] on failed allocation attempts - manually call [/_cluster/reroute?retry_failed=true] to retry, [unassigned_info ...]"
}

With the buggy code, the explanation would read "shard cannot be allocated" and the boundary would be off by one — the exact operator-facing degradation your fix prevents. Recover the demo:

curl -s -XPOST 'localhost:9200/_cluster/reroute?retry_failed=true'
curl -s -XDELETE 'localhost:9200/retry-demo'

Pitfall — allocation/explain needs an unassigned (or movable) shard to explain. If everything is green, the API returns an error. Force an unassignable shard (as above) before calling it.

Implementation Requirements

The decider uses the correct boundary (>= maxRetries) so a shard is denied at the limit, not one past it.
The Decision.NO explanation includes the failure count, the limit, and the retry API hint.
A unit test asserts Decision.Type.NO at exactly maxRetries and YES below it, and pins the explanation content.
You reproduced the operator-facing behavior with _cluster/allocation/explain and saw the max_retry decider entry.
A CHANGELOG.md entry under ## [Unreleased] (Fixed) if you were submitting this for real.

Common Pitfalls

Pitfall	Why it bites	Avoid by
Treating a `Decision` as boolean	You ignore `THROTTLE`, which is not a `NO`	Always switch on `Decision.Type` (`YES`/`NO`/`THROTTLE`)
Changing the verdict but not the test	Regression slips back in later	Pin the boundary value, not just "denies eventually"
Degrading the explanation string	Breaks operator runbooks and dashboards	Treat `allocation/explain` text as API; preserve actionable detail
Off-by-one on `>=` vs `>`	Allows one retry too many/few	Write the boundary test first
Constructing `RoutingAllocation` by hand	Brittle, wrong shard states	Reuse `OpenSearchAllocationTestCase` helpers
Calling `allocation/explain` on a green cluster	API errors	Create an intentionally-unassignable shard first
Forgetting `allocation.debugDecision(true)`	`getExplanation()` is empty in the test	Enable debug decisions to capture the message

Expected Output

Test:

> Task :server:test
MaxRetryBoundaryTests > testDeniedExactlyAtMaxRetries PASSED
MaxRetryBoundaryTests > testAllowedBelowMaxRetries PASSED

Live reproduction shows a max_retry decider entry with decision: NO and the full, actionable explanation string.

Stretch Goals

A different decider, same motion. Apply the read → spot-bug → test → explain loop to SameShardAllocationDecider (it must return NO when the same shard copy is already on the node) or DiskThresholdDecider (watermarks). Each has its own boundary worth a test.
Combine deciders. Read AllocationDeciders.canAllocate(...) and prove with a test that one NO overrides any number of YESes, and that THROTTLE is returned only when there's no NO.
Trace applyFailedShards. In AllocationService, follow how a failed shard's UnassignedInfo.numFailedAllocations gets incremented — that's the value MaxRetryAllocationDecider reads. This closes the loop between Lab 4.2's state updates and this decider.

Coding Exercises

You fixed one decider and pinned its boundary. Now broaden the skill: test other deciders' edge cases, add a real YES/NO/THROTTLE branch with a test, and verify chain-combination semantics. Locate every class with rg; reuse OpenSearchAllocationTestCase helpers rather than hand-building states.

(warm-up) Pin SameShardAllocationDecider's NO. It must forbid placing a shard copy on a node that already holds another copy of the same shard. Read it (rg -n "canAllocate|sameHost|Decision.NO" server/src/main/java/org/opensearch/cluster/routing/allocation/decider/SameShardAllocationDecider.java) and write an OpenSearchAllocationTestCase that builds a 1-node state with a primary already assigned, then asserts canAllocate(replica, allocation).type() == NO. Mirror its existing test: rg -l "class SameShardAllocationDeciderTests" server/src/test/java.
(core) Test the THROTTLE branch of a real decider. ThrottlingAllocationDecider returns THROTTLE when too many recoveries are in flight. Read the relevant settings (rg -n "CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT|THROTTLE|canAllocate" server/src/main/java/org/opensearch/cluster/routing/allocation/decider/ThrottlingAllocationDecider.java) and write a test that drives the in-flight count to the limit and asserts the decision type is THROTTLE (not NO). This forces you to handle the third Decision.Type the boolean trap in Common Pitfalls warns about.
(core) Prove chain combination semantics. Implement Stretch Goal 2 as a graded test: construct an AllocationDeciders from a list of stub deciders (find the constructor/interface: rg -n "class AllocationDeciders|canAllocate" server/src/main/java/org/opensearch/cluster/routing/allocation/decider/AllocationDeciders.java), and assert (a) one NO plus many YES yields NO; (b) one THROTTLE plus YES (no NO) yields THROTTLE; (c) all YES yields YES. Use simple anonymous AllocationDecider subclasses returning fixed decisions. This pins the "most restrictive wins" rule from the Background table.
(core) Write a custom AllocationDecider variant with all three branches. Create MaxShardsPerNodeDecider extends AllocationDecider (rg -n "abstract class AllocationDecider" server/src/main/java/org/opensearch/cluster/routing/allocation/decider/AllocationDecider.java) whose canAllocate returns: NO (with an actionable explanation) when the target node already holds >= hardLimit shards; THROTTLE when it holds >= softLimit (advise waiting); else YES. Count shards via allocation.routingNodes().node(nodeId).size() (rg -n "RoutingNode|size\(\)|numberOfShards" server/src/main/java/org/opensearch/cluster/routing/RoutingNode.java). Write a test asserting each branch at the boundary values. You now own a decider end to end.
(core) Boundary test for DiskThresholdDecider watermarks (read-then-assert). Read how the low/high watermarks map to YES/NO (rg -n "watermark|freeBytesThreshold|canAllocate|Decision" server/src/main/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDecider.java) and write a test that, with a ClusterInfo reporting disk just below vs just above the high watermark, asserts the decision flips from YES to NO at the threshold. Mirror DiskThresholdDeciderTests for the ClusterInfo fixture.
(advanced challenge) Close the loop from failed shard → decider input, with a test. Implement Stretch Goal 3 as code: write an OpenSearchAllocationTestCase that uses AllocationService.applyFailedShards(...) repeatedly to drive a shard's UnassignedInfo.getNumFailedAllocations() up to maxRetries, then asserts the same shard now gets a NO from MaxRetryAllocationDecider through a full reroute (not by calling canAllocate directly). Find the API: rg -n "applyFailedShards|reroute" server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java. This proves the value MaxRetryAllocationDecider reads is the one applyFailedShards increments — the end-to-end story behind the bug you fixed.

Issues to Practice On

"Unassigned shards" and allocation-explain issues are among the highest-volume, most mergeable categories — and decider fixes are a classic good first issue shape. Target opensearch-project/OpenSearch.

# Allocation / cluster-manager / first-timer issues (labels move; confirm on the tracker):
gh issue list --repo opensearch-project/OpenSearch --label "Cluster Manager" --state open --search "allocation"
gh issue list --repo opensearch-project/OpenSearch --label "bug" --search "unassigned shard allocation" --state open
gh issue list --repo opensearch-project/OpenSearch --label "good first issue" --state open
gh label list --repo opensearch-project/OpenSearch | rg -i "alloc|shard|routing|cluster"

Representative patterns:

"Shard stays UNASSIGNED and allocation/explain doesn't say why." Reproduce by forcing an unassignable shard (Step 5), find the decider whose explanation is unhelpful via rg -ln "extends AllocationDecider" server/src/main/java, improve the message and pin it with a test asserting getExplanation() content — exactly the second defect you fixed.
"Off-by-one / wrong boundary in a decider." Reproduce, write the boundary test first (Pitfall table), then fix the comparison. This is the shape of the whole lab.

Planted-bug drill. Beyond this lab's MaxRetryAllocationDecider regression, plant a second one: in SameShardAllocationDecider, flip its NO to YES for the already-present case. Run ./gradlew :server:test --tests "*SameShardAllocationDeciderTests*" and your exercise-1 test, watch them go red, and reason about the corruption (two copies of a shard on one node = no redundancy). Revert, then make sure exercise 1 asserts the exact NO that would have caught it.

Etiquette: claim an issue before working it, reproduce first, every PR needs a test + CHANGELOG.md entry (under Fixed/Added) + DCO Signed-off-by (git commit -s). Preserve operator-facing explanation strings. See community interaction and the Level 2 PR lab.

Validation / Self-check

Name the three Decision.Type values and explain how AllocationDeciders combines a chain of them into one verdict.
In MaxRetryAllocationDecider, which field on UnassignedInfo does the verdict depend on, and which index setting is the limit?
Why is >= (not >) the correct boundary, and what is the operator-visible symptom of the > bug?
Why is the explanation string part of the contract — who consumes it, and through which API?
Describe the unit test you would write to pin this fix so the regression can't return. Which exact failure-count value does it use, and what two things does it assert?
On a live cluster, how do you force an unassignable shard so that _cluster/allocation/explain shows the max_retry decider? Why won't it work on a green cluster?
If this were a real PR, what goes in CHANGELOG.md, and under which heading?

When your boundary test fails on the buggy code, passes on the fix, and your allocation/explain reproduction shows the actionable max_retry explanation, you've completed Lab 4.4 — and Level 4. Continue to Level 5: Testing and Debugging.

Open-Source Engineer & Contributor