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:

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

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:

1. 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. >=.
2. 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

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

1. 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.
2. 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.
3. 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.

Validation / Self-check

1. Name the three Decision.Type values and explain how AllocationDeciders combines a chain of them into one verdict.
2. In MaxRetryAllocationDecider, which field on UnassignedInfo does the verdict depend on, and which index setting is the limit?
3. Why is >= (not >) the correct boundary, and what is the operator-visible symptom of the > bug?
4. Why is the explanation string part of the contract — who consumes it, and through which API?
5. 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?
6. 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?
7. 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.