[Segment Replication] Fix flaky test testRelocateWhileContinuouslyIndexingAndWaitingForRefresh #6619
Conversation
Signed-off-by: Rishikesh1159 <[email protected]>
Rishikesh1159
requested review from
reta,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
gbbafna,
setiah,
kartg,
kotwanikunal,
mch2,
nknize,
owaiskazi19,
adnapibar,
ryanbogan,
saratvemulapalli,
shwetathareja,
dreamer-89,
tlfeng,
VachaShah and
xuezhou25
as code owners
| final List<ShardRouting> replicationTargets = indexShard.getReplicationGroup().getReplicationTargets(); | ||
| final List<ShardRouting> replicationTargets; | ||
| try { | ||
| replicationTargets = indexShard.getReplicationGroup().getReplicationTargets(); |
There was a problem hiding this comment.
@Rishikesh1159 : Thanks for fixing this. Can you also please add more details around why this fix, reduces/fixes the flakyness here.
There was a problem hiding this comment.
Sure. During the process of relocation of primary, primary still publishes checkpoints on refresh even if relocation is in progress. So, during relocation when handoff is completed we switch the primarymode to false. So, this handoff process and publishing checkpoints to replicas happen parallely in our scenario.
Before publishing checkpoints to replicas we do call getReplicationGroup() on IndexShard. And this methods asserts on primarymode. So, very rarely we end up in a situation where primarymode of shard is switched between checkpointRefreshListeners afterRefresh check here and this getReplicationGroup() assert.
By throwing an exception in getReplicationGroup() when shard not in primary mode we are reducing flakiness. But again we immediately have assert after this primarymode check in getReplicationGroup(), If relocation handoff completes between these two lines and primarymode switches, we may again fail assert, so there is flakiness but will not happen often.
We are not concerned about primary sending checkpoints to replica here, we are only trying to reduce flakiness.
Gradle Check (Jenkins) Run Completed with:
|
|
@Rishikesh1159 With the revert of the wait_until change, and the decision to not support that behavior with segrep, I think we should revert #6366 making this change obsolete. With that revert - #6637 is the only change we will need to release any wait_until reqs from nrt replicas. |
Sure @mch2, yes, if we revert changes made to PublishCheckpointAction class in #6366 then this PR and issue becomes obsolete. I will revert changes made to PublishCheckpointAction class by #6366, in PR : #6637 |
Gradle Check (Jenkins) Run Completed with:
|
Description
This PR fixes the flaky Test failing in
SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh. This PR is the best effort to reduce flakiness.Issues Resolved
#6531
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.