From 87664f9e76d184f3780efec205eab300ba21f1d1 Mon Sep 17 00:00:00 2001 From: Shivansh Arora Date: Thu, 17 Oct 2024 14:51:06 +0530 Subject: [PATCH] Revert "[Bugfix] Fix NPE in ReplicaShardAllocator (#13993) (#14385)" This reverts commit cf0d6ccd Signed-off-by: Shivansh Arora --- .../gateway/ReplicaShardAllocator.java | 5 +- .../ReplicaShardBatchAllocatorTests.java | 54 ------------------- 2 files changed, 1 insertion(+), 58 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/ReplicaShardAllocator.java b/server/src/main/java/org/opensearch/gateway/ReplicaShardAllocator.java index c30ee8479ac97..9bb6a6f794ce3 100644 --- a/server/src/main/java/org/opensearch/gateway/ReplicaShardAllocator.java +++ b/server/src/main/java/org/opensearch/gateway/ReplicaShardAllocator.java @@ -100,10 +100,7 @@ protected Runnable cancelExistingRecoveryForBetterMatch( Metadata metadata = allocation.metadata(); RoutingNodes routingNodes = allocation.routingNodes(); ShardRouting primaryShard = allocation.routingNodes().activePrimary(shard.shardId()); - if (primaryShard == null) { - logger.trace("{}: no active primary shard found or allocated, letting actual allocation figure it out", shard); - return null; - } + assert primaryShard != null : "the replica shard can be allocated on at least one node, so there must be an active primary"; assert primaryShard.currentNodeId() != null; final DiscoveryNode primaryNode = allocation.nodes().get(primaryShard.currentNodeId()); diff --git a/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java b/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java index 988723e023a2a..3bf9b4c603ced 100644 --- a/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java +++ b/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java @@ -644,25 +644,6 @@ public void testDoNotCancelForBrokenNode() { assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED), empty()); } - public void testDoNotCancelForInactivePrimaryNode() { - RoutingAllocation allocation = oneInactivePrimaryOnNode1And1ReplicaRecovering(yesAllocationDeciders(), null); - testBatchAllocator.addData( - node1, - null, - "MATCH", - null, - new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION) - ).addData(node2, randomSyncId(), null, new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION)); - - testBatchAllocator.processExistingRecoveries( - allocation, - Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) - ); - - assertThat(allocation.routingNodesChanged(), equalTo(false)); - assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED), empty()); - } - public void testAllocateUnassignedBatchThrottlingAllocationDeciderIsHonoured() throws InterruptedException { ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); AllocationDeciders allocationDeciders = randomAllocationDeciders( @@ -936,41 +917,6 @@ private RoutingAllocation onePrimaryOnNode1And1ReplicaRecovering(AllocationDecid ); } - private RoutingAllocation oneInactivePrimaryOnNode1And1ReplicaRecovering(AllocationDeciders deciders, UnassignedInfo unassignedInfo) { - ShardRouting primaryShard = TestShardRouting.newShardRouting(shardId, node1.getId(), true, ShardRoutingState.INITIALIZING); - RoutingTable routingTable = RoutingTable.builder() - .add( - IndexRoutingTable.builder(shardId.getIndex()) - .addIndexShard( - new IndexShardRoutingTable.Builder(shardId).addShard(primaryShard) - .addShard( - TestShardRouting.newShardRouting( - shardId, - node2.getId(), - null, - false, - ShardRoutingState.INITIALIZING, - unassignedInfo - ) - ) - .build() - ) - ) - .build(); - ClusterState state = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) - .routingTable(routingTable) - .nodes(DiscoveryNodes.builder().add(node1).add(node2)) - .build(); - return new RoutingAllocation( - deciders, - new RoutingNodes(state, false), - state, - ClusterInfo.EMPTY, - SnapshotShardSizeInfo.EMPTY, - System.nanoTime() - ); - } - private RoutingAllocation onePrimaryOnNode1And1ReplicaRecovering(AllocationDeciders deciders) { return onePrimaryOnNode1And1ReplicaRecovering(deciders, new UnassignedInfo(UnassignedInfo.Reason.CLUSTER_RECOVERED, null)); }