Skip to content

Commit 87664f9

Browse files
committed
Revert "[Bugfix] Fix NPE in ReplicaShardAllocator (opensearch-project#13993) (opensearch-project#14385)"
This reverts commit cf0d6cc Signed-off-by: Shivansh Arora <hishiv@amazon.com>
1 parent e7ad37d commit 87664f9

File tree

2 files changed

+1
-58
lines changed

2 files changed

+1
-58
lines changed

server/src/main/java/org/opensearch/gateway/ReplicaShardAllocator.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,7 @@ protected Runnable cancelExistingRecoveryForBetterMatch(
100100
Metadata metadata = allocation.metadata();
101101
RoutingNodes routingNodes = allocation.routingNodes();
102102
ShardRouting primaryShard = allocation.routingNodes().activePrimary(shard.shardId());
103-
if (primaryShard == null) {
104-
logger.trace("{}: no active primary shard found or allocated, letting actual allocation figure it out", shard);
105-
return null;
106-
}
103+
assert primaryShard != null : "the replica shard can be allocated on at least one node, so there must be an active primary";
107104
assert primaryShard.currentNodeId() != null;
108105
final DiscoveryNode primaryNode = allocation.nodes().get(primaryShard.currentNodeId());
109106

server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java

-54
Original file line numberDiff line numberDiff line change
@@ -644,25 +644,6 @@ public void testDoNotCancelForBrokenNode() {
644644
assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED), empty());
645645
}
646646

647-
public void testDoNotCancelForInactivePrimaryNode() {
648-
RoutingAllocation allocation = oneInactivePrimaryOnNode1And1ReplicaRecovering(yesAllocationDeciders(), null);
649-
testBatchAllocator.addData(
650-
node1,
651-
null,
652-
"MATCH",
653-
null,
654-
new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION)
655-
).addData(node2, randomSyncId(), null, new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION));
656-
657-
testBatchAllocator.processExistingRecoveries(
658-
allocation,
659-
Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING)))
660-
);
661-
662-
assertThat(allocation.routingNodesChanged(), equalTo(false));
663-
assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED), empty());
664-
}
665-
666647
public void testAllocateUnassignedBatchThrottlingAllocationDeciderIsHonoured() throws InterruptedException {
667648
ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
668649
AllocationDeciders allocationDeciders = randomAllocationDeciders(
@@ -936,41 +917,6 @@ private RoutingAllocation onePrimaryOnNode1And1ReplicaRecovering(AllocationDecid
936917
);
937918
}
938919

939-
private RoutingAllocation oneInactivePrimaryOnNode1And1ReplicaRecovering(AllocationDeciders deciders, UnassignedInfo unassignedInfo) {
940-
ShardRouting primaryShard = TestShardRouting.newShardRouting(shardId, node1.getId(), true, ShardRoutingState.INITIALIZING);
941-
RoutingTable routingTable = RoutingTable.builder()
942-
.add(
943-
IndexRoutingTable.builder(shardId.getIndex())
944-
.addIndexShard(
945-
new IndexShardRoutingTable.Builder(shardId).addShard(primaryShard)
946-
.addShard(
947-
TestShardRouting.newShardRouting(
948-
shardId,
949-
node2.getId(),
950-
null,
951-
false,
952-
ShardRoutingState.INITIALIZING,
953-
unassignedInfo
954-
)
955-
)
956-
.build()
957-
)
958-
)
959-
.build();
960-
ClusterState state = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
961-
.routingTable(routingTable)
962-
.nodes(DiscoveryNodes.builder().add(node1).add(node2))
963-
.build();
964-
return new RoutingAllocation(
965-
deciders,
966-
new RoutingNodes(state, false),
967-
state,
968-
ClusterInfo.EMPTY,
969-
SnapshotShardSizeInfo.EMPTY,
970-
System.nanoTime()
971-
);
972-
}
973-
974920
private RoutingAllocation onePrimaryOnNode1And1ReplicaRecovering(AllocationDeciders deciders) {
975921
return onePrimaryOnNode1And1ReplicaRecovering(deciders, new UnassignedInfo(UnassignedInfo.Reason.CLUSTER_RECOVERED, null));
976922
}

0 commit comments

Comments
 (0)