From 4cc0676b1bdd0eadabdaad7958188ac2e2e27a1b Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Tue, 9 Jul 2024 12:55:09 +0530 Subject: [PATCH] Replace Set with List Signed-off-by: Rishab Nahata --- .../gateway/PrimaryShardBatchAllocator.java | 2 +- .../gateway/ReplicaShardBatchAllocator.java | 6 +++--- .../gateway/ShardsBatchGatewayAllocator.java | 6 +++--- .../opensearch/gateway/GatewayAllocatorTests.java | 4 ++-- .../gateway/PrimaryShardBatchAllocatorTests.java | 2 +- .../gateway/ReplicaShardBatchAllocatorTests.java | 15 ++++++++------- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java b/server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java index a531c6b14cec6..7291817e08bc9 100644 --- a/server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java +++ b/server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java @@ -77,7 +77,7 @@ public AllocateUnassignedDecision makeAllocationDecision(ShardRouting unassigned * @param shardRoutings the shards to allocate * @param allocation the allocation state container object */ - public void allocateUnassignedBatch(Set shardRoutings, RoutingAllocation allocation) { + public void allocateUnassignedBatch(List shardRoutings, RoutingAllocation allocation) { HashMap ineligibleShardAllocationDecisions = new HashMap<>(); List eligibleShards = new ArrayList<>(); List inEligibleShards = new ArrayList<>(); diff --git a/server/src/main/java/org/opensearch/gateway/ReplicaShardBatchAllocator.java b/server/src/main/java/org/opensearch/gateway/ReplicaShardBatchAllocator.java index ef0bd7ddb9848..b64ee6d2c8a0b 100644 --- a/server/src/main/java/org/opensearch/gateway/ReplicaShardBatchAllocator.java +++ b/server/src/main/java/org/opensearch/gateway/ReplicaShardBatchAllocator.java @@ -44,10 +44,10 @@ public abstract class ReplicaShardBatchAllocator extends ReplicaShardAllocator { * @param allocation the overall routing allocation * @param shardBatches a list of shard batches to check for existing recoveries */ - public void processExistingRecoveries(RoutingAllocation allocation, List> shardBatches) { + public void processExistingRecoveries(RoutingAllocation allocation, List> shardBatches) { List shardCancellationActions = new ArrayList<>(); // iterate through the batches, each batch needs to be processed together as fetch call should be made for shards from same batch - for (Set shardBatch : shardBatches) { + for (List shardBatch : shardBatches) { List eligibleShards = new ArrayList<>(); List ineligibleShards = new ArrayList<>(); // iterate over shards to check for match for each of those @@ -112,7 +112,7 @@ public AllocateUnassignedDecision makeAllocationDecision(ShardRouting unassigned * @param shardRoutings the shards to allocate * @param allocation the allocation state container object */ - public void allocateUnassignedBatch(Set shardRoutings, RoutingAllocation allocation) { + public void allocateUnassignedBatch(List shardRoutings, RoutingAllocation allocation) { List eligibleShards = new ArrayList<>(); List ineligibleShards = new ArrayList<>(); Map ineligibleShardAllocationDecisions = new HashMap<>(); diff --git a/server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java b/server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java index afdc67f5ad1c0..d533dc366e24a 100644 --- a/server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java +++ b/server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java @@ -179,7 +179,7 @@ public void beforeAllocation(final RoutingAllocation allocation) { @Override public void afterPrimariesBeforeReplicas(RoutingAllocation allocation) { assert replicaShardBatchAllocator != null; - List> storedShardBatches = batchIdToStoreShardBatch.values() + List> storedShardBatches = batchIdToStoreShardBatch.values() .stream() .map(ShardsBatch::getBatchedShardRoutings) .collect(Collectors.toList()); @@ -684,8 +684,8 @@ private void clearShardFromCache(ShardId shardId) { asyncBatch.clearShard(shardId); } - public Set getBatchedShardRoutings() { - return batchInfo.values().stream().map(ShardEntry::getShardRouting).collect(Collectors.toSet()); + public List getBatchedShardRoutings() { + return batchInfo.values().stream().map(ShardEntry::getShardRouting).collect(Collectors.toList()); } public Set getBatchedShards() { diff --git a/server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java b/server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java index 9e42e152bbe38..aa31c710c1fbd 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java @@ -129,7 +129,7 @@ public void testCorrectnessOfBatch() { .values() .stream() .map(ShardsBatchGatewayAllocator.ShardsBatch::getBatchedShardRoutings) - .flatMap(Set::stream) + .flatMap(List::stream) .collect(Collectors.toSet()); primariesInAllBatches.forEach(shardRouting -> assertTrue(shardRouting.unassigned() && shardRouting.primary() == true)); @@ -137,7 +137,7 @@ public void testCorrectnessOfBatch() { .values() .stream() .map(ShardsBatchGatewayAllocator.ShardsBatch::getBatchedShardRoutings) - .flatMap(Set::stream) + .flatMap(List::stream) .collect(Collectors.toSet()); replicasInAllBatches.forEach(shardRouting -> assertTrue(shardRouting.unassigned() && shardRouting.primary() == false)); diff --git a/server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java b/server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java index 5aaadbf529a09..e90850de3fe33 100644 --- a/server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java +++ b/server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java @@ -83,7 +83,7 @@ private void allocateAllUnassigned(final RoutingAllocation allocation) { private void allocateAllUnassignedBatch(final RoutingAllocation allocation) { final RoutingNodes.UnassignedShards.UnassignedIterator iterator = allocation.routingNodes().unassigned().iterator(); - Set shardsToBatch = new HashSet<>(); + List shardsToBatch = new ArrayList<>(); while (iterator.hasNext()) { shardsToBatch.add(iterator.next()); } diff --git a/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java b/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java index 926093e8e4411..570db82c70d00 100644 --- a/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java +++ b/server/src/test/java/org/opensearch/gateway/ReplicaShardBatchAllocatorTests.java @@ -53,6 +53,7 @@ import org.opensearch.snapshots.SnapshotShardSizeInfo; import org.junit.Before; +import java.sql.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -97,7 +98,7 @@ public static void setUpShards(int numberOfShards) { private void allocateAllUnassignedBatch(final RoutingAllocation allocation) { final RoutingNodes.UnassignedShards.UnassignedIterator iterator = allocation.routingNodes().unassigned().iterator(); - Set shardToBatch = new HashSet<>(); + List shardToBatch = new ArrayList<>(); while (iterator.hasNext()) { shardToBatch.add(iterator.next()); } @@ -275,7 +276,7 @@ public void testCancelRecoveryIfFoundCopyWithNoopRetentionLease() { new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION) ); Collection replicaShards = allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED); - Set shardRoutingBatch = new HashSet<>(replicaShards); + List shardRoutingBatch = new ArrayList<>(replicaShards); List> shardBatchList = Collections.singletonList( new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING)) ); @@ -319,7 +320,7 @@ public void testNotCancellingRecoveryIfCurrentRecoveryHasRetentionLease() { ); testBatchAllocator.processExistingRecoveries( allocation, - Collections.singletonList(new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) + Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) ); assertThat(allocation.routingNodesChanged(), equalTo(false)); assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).size(), equalTo(0)); @@ -348,7 +349,7 @@ public void testNotCancelIfPrimaryDoesNotHaveValidRetentionLease() { ); testBatchAllocator.processExistingRecoveries( allocation, - Collections.singletonList(new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) + Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) ); assertThat(allocation.routingNodesChanged(), equalTo(false)); assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).size(), equalTo(0)); @@ -586,7 +587,7 @@ public void testNotCancellingRecoveryIfSyncedOnExistingRecovery() { ); testBatchAllocator.processExistingRecoveries( allocation, - Collections.singletonList(new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) + Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) ); assertThat(allocation.routingNodesChanged(), equalTo(false)); assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).size(), equalTo(0)); @@ -598,7 +599,7 @@ public void testNotCancellingRecovery() { .addData(node2, "MATCH", null, new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION)); testBatchAllocator.processExistingRecoveries( allocation, - Collections.singletonList(new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) + Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) ); assertThat(allocation.routingNodesChanged(), equalTo(false)); assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED).size(), equalTo(0)); @@ -638,7 +639,7 @@ public void testDoNotCancelForBrokenNode() { .addData(node3, randomSyncId(), null, new StoreFileMetadata("file1", 10, "MATCH_CHECKSUM", MIN_SUPPORTED_LUCENE_VERSION)); testBatchAllocator.processExistingRecoveries( allocation, - Collections.singletonList(new HashSet<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) + Collections.singletonList(new ArrayList<>(allocation.routingNodes().shardsWithState(ShardRoutingState.INITIALIZING))) ); assertThat(allocation.routingNodesChanged(), equalTo(false)); assertThat(allocation.routingNodes().shardsWithState(ShardRoutingState.UNASSIGNED), empty());