Test case for validating AllocationDeciders not being honoured in bat… #13748
Conversation
SwethaGuptha
requested review from
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
dbwiddis,
dreamer-89,
gbbafna,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami,
tlfeng and
VachaShah
as code owners
|
❌ Gradle check result for c0aa1f6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java
Show resolved
Hide resolved
server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/opensearch/gateway/PrimaryShardBatchAllocatorTests.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for 828b854: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 828b854: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
| } catch (Exception e) { | ||
| logger.error("Failed to execute decision for shard {} while initializing {}", shard, e); | ||
| throw e; | ||
| ShardRouting shardRouting = iterator.next(); |
There was a problem hiding this comment.
Can we just iterator over shardRoutings instead of allocation.routingNodes().unassigned().iterator()
There was a problem hiding this comment.
Initialization of shards is handled by the interface UnassignedAllocationHandler and is implemented only by UnassignedShards.UnassignedIterator, hence we need to use this iterator instead of looping through shardRoutings.
server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/gateway/PrimaryShardBatchAllocator.java
Outdated
Show resolved
Hide resolved
| public AllocateUnassignedDecision makeAllocationDecision(ShardRouting unassignedShard, RoutingAllocation allocation, Logger logger) { | ||
| return makeAllocationDecision(Collections.singletonList(unassignedShard), allocation, logger).get(unassignedShard); | ||
| } | ||
|
|
||
| /** | ||
| * Build allocation decisions for all the shards present in the batch identified by batchId. | ||
| * | ||
| * @param shards set of shards given for allocation | ||
| * @param allocation current allocation of all the shards | ||
| * @param logger logger used for logging | ||
| * @return shard to allocation decision map | ||
| */ | ||
| @Override | ||
| public HashMap<ShardRouting, AllocateUnassignedDecision> makeAllocationDecision( |
There was a problem hiding this comment.
This method is being invoked for explainUnassignedShardAllocation. We will have to retain the method.
| @@ -60,51 +62,42 @@ protected FetchResult<TransportNodesListGatewayStartedShards.NodeGatewayStartedS | |||
| } | |||
|
|
|||
| @Override | |||
| public AllocateUnassignedDecision makeAllocationDecision(ShardRouting unassignedShard, RoutingAllocation allocation, Logger logger) { | |||
There was a problem hiding this comment.
This needs to be overriden here - otherwise it will fallback to single fetch which is not supported
protected FetchResult<TransportNodesListGatewayStartedShards.NodeGatewayStartedShards> fetchData(
ShardRouting shard,
RoutingAllocation allocation
) {
logger.error("fetchData for single shard called via batch allocator, shard id {}", shard.shardId());
throw new IllegalStateException("PrimaryShardBatchAllocator should only be used for a batch of shards");
}
There was a problem hiding this comment.
Added the method to batch shard allocators.
SwethaGuptha
force-pushed
the
async-bug-branch#13702
branch
from
19b6530
to
0b92e0d
Compare
…arch-project#13702) Signed-off-by: Swetha Guptha <[email protected]>
SwethaGuptha
force-pushed
the
async-bug-branch#13702
branch
from
5f1715a
to
ce7373d
Compare
|
❌ Gradle check result for 19b6530: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 0b92e0d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 5f1715a: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for ce7373d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Description
Added a test case to assert the allocation deciders not being honored in batch mode due to execution of deciders of all shards in a batch together.
Related Issues
Resolves #13702
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.