[BUG] NPE in ReplicaShardBatchAllocator during node drops

[SwethaGuptha]

Describe the bug

[2024-05-28T14:34:01,627][ERROR][o.o.c.c.Coordinator      ] [66a99e3966c42f12f229f8b22da6d074] unexpected failure during [node-left]
java.lang.NullPointerException: Cannot invoke "org.opensearch.cluster.routing.ShardRouting.currentNodeId()" because "primaryShard" is null
        at org.opensearch.gateway.ReplicaShardAllocator.cancelExistingRecoveryForBetterMatch(ReplicaShardAllocator.java:108)
        at org.opensearch.gateway.ReplicaShardBatchAllocator.processExistingRecoveries(ReplicaShardBatchAllocator.java:73)
        at org.opensearch.gateway.ShardsBatchGatewayAllocator.afterPrimariesBeforeReplicas(ShardsBatchGatewayAllocator.java:198)
        at org.opensearch.cluster.routing.allocation.AllocationService.allocateAllUnassignedShards(AllocationService.java:639)
        at org.opensearch.cluster.routing.allocation.AllocationService.allocateExistingUnassignedShards(AllocationService.java:609)
        at org.opensearch.cluster.routing.allocation.AllocationService.reroute(AllocationService.java:588)
        at org.opensearch.cluster.routing.allocation.AllocationService.reroute(AllocationService.java:529)
        at org.opensearch.cluster.routing.allocation.AllocationService.disassociateDeadNodes(AllocationService.java:339)
        at org.opensearch.cluster.coordination.NodeRemovalClusterStateTaskExecutor.getTaskClusterTasksResult(NodeRemovalClusterStateTaskExecutor.java:123)
        at org.opensearch.cluster.coordination.NodeRemovalClusterStateTaskExecutor.execute(NodeRemovalClusterStateTaskExecutor.java:113)
        at org.opensearch.cluster.service.MasterService.executeTasks(MasterService.java:882)
        at org.opensearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:434)
        at org.opensearch.cluster.service.MasterService.runTasks(MasterService.java:301)
        at org.opensearch.cluster.service.MasterService$Batcher.run(MasterService.java:212)
        at org.opensearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:209)
        at org.opensearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:247)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:863)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedOpenSearchThreadPoolExecutor.java:283)
        at org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedOpenSearchThreadPoolExecutor.java:246)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:840)

Related component

Cluster Manager

To Reproduce

Perform all node drops from the cluster.

Expected behavior

Handle the NPE.

Additional Details

Plugins
Please list all plugins currently enabled.

Screenshots
If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

• OS: [e.g. iOS]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[peternied]

[Triage - attendees 1 2 3 4 5 6 7]
@SwethaGuptha Thanks for creating this issue, could you create a pull request to address?

[Bukhtawar]

Re-opening as the fix adds a null check however doesn't address the underlying root cause

[dblock]

[Catch All Triage - 1, 2]

[shwetathareja]

Thanks @Bukhtawar for re-opening the issue. Yes,swallowing the NPE here was not the right fix.

Repro steps:
Node 1 has primary shard s1
Node 2 has replica s2 recovering from primary
Node 1 leaves the cluster

Root cause:
It will result in NPE and the reason is processExistingRecoveries method in ReplicaShardBatchAllocator is iterating over batches which can be stale instead of RoutingAllocation which we use for single shard flow.

OpenSearch/server/src/main/java/org/opensearch/gateway/ReplicaShardBatchAllocator.java

Line 55 in 456ca97

for (List<ShardRouting> shardBatch : shardBatches) {

Why the batch cache for replica ShardRouting which is used in for loop is stale is because allocateAllUnassignedShards method in AllocationService is 3 steps - 1. Allocate unassigned primaries 2. afterPrimariesBeforeReplica step to do find better match for existing recoveries 3. Allocate unassigned replicas. But the replica cache is updated is step 3, so we shouldn't be iterating over it.

OpenSearch/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java

Lines 624 to 630 in 456ca97

	private void allocateAllUnassignedShards(RoutingAllocation allocation) {
	ExistingShardsAllocator allocator = existingShardsAllocators.get(ShardsBatchGatewayAllocator.ALLOCATOR_NAME);
	Optional.ofNullable(allocator.allocateAllUnassignedShards(allocation, true)).ifPresent(Runnable::run);
	allocator.afterPrimariesBeforeReplicas(allocation);
	// Replicas Assignment
	Optional.ofNullable(allocator.allocateAllUnassignedShards(allocation, false)).ifPresent(Runnable::run);
	}