[Segment Replication] Fixing flaky test failure happening for testShardAlreadyReplicating() by Rishikesh1159 · Pull Request #3943 · opensearch-project/OpenSearch

Rishikesh1159 · 2022-07-19T06:10:19Z

Signed-off-by: Rishikesh1159 rishireddy1159@gmail.com

Description

This PR fixes flaky test failure of testShardAlreadyReplicating()

Issues Resolved

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2022-07-19T06:35:32Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/774/
CommitID: 683880c

codecov-commenter · 2022-07-19T06:36:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.57%. Comparing base (07775ff) to head (076f0ab).
⚠️ Report is 4770 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3943      +/-   ##
============================================
- Coverage     70.57%   70.57%   -0.01%     
- Complexity    56679    56748      +69     
============================================
  Files          4563     4573      +10     
  Lines        272755   273255     +500     
  Branches      40040    40076      +36     
============================================
+ Hits         192505   192839     +334     
- Misses        64014    64178     +164     
- Partials      16236    16238       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…rviceTests Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2022-07-19T07:21:09Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/780/
CommitID: 571d49e

github-actions · 2022-07-19T13:48:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/792/
CommitID: f93d316

mch2

LGTM

mch2 · 2022-07-19T18:28:50Z

...r/src/test/java/org/opensearch/indices/replication/SegmentReplicationTargetServiceTests.java

-    public void testShardAlreadyReplicating() {
-        SegmentReplicationTargetService spy = spy(sut);
+    public void testShardAlreadyReplicating() throws InterruptedException {
+        SegmentReplicationTargetService serviceSpy = spy(sut);


nit - A comment here explaining why we need to spy on the service and what we are asserting would be useful.

kartg

Approving since the test is functioning properly, but please consider adding more comments to the testShardAlreadyReplicating test case to make it easier to understand. The test is manipulating a lot of objects to achieve a specific scenario and it's hard to quickly grok what each step is doing.

kartg · 2022-07-19T18:26:43Z

...r/src/test/java/org/opensearch/indices/replication/SegmentReplicationTargetServiceTests.java

+        cp = indexShard.getLatestReplicationCheckpoint();
+        newCheckpoint = new ReplicationCheckpoint(


Nitpick - more descriptive variable names, please. cp is hard to decipher at first glance and newCheckpoint is confusing when it's being initialized to be equal to cp

kartg · 2022-07-19T18:27:38Z

...r/src/test/java/org/opensearch/indices/replication/SegmentReplicationTargetServiceTests.java

-        SegmentReplicationTargetService spy = spy(sut);
+    public void testShardAlreadyReplicating() throws InterruptedException {
+        SegmentReplicationTargetService serviceSpy = spy(sut);
        // Create a separate target and start it so the shard is already replicating.


This comment seems out of place now since the replication is not started until multiple lines later. Consider removing it

…cating() test case Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

…to main Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

github-actions · 2022-07-20T16:03:09Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/817/
CommitID: 076f0ab

…rdAlreadyReplicating() (opensearch-project#3943) * Fixing flaky test failure happening for testShardAlreadyReplicating() Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

…#3943 #3963 From main branch (#4181) * Resolving import conflict in Node.java and mergining PR #3525. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Resolving conflicts and merging PR #3533. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Resolving conflicts and Merging PR #3540. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Applying spotlesscheck and fixing wildcard imports. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * [Segment Replication] Fixing flaky test failure happening for testShardAlreadyReplicating() (#3943) * Fixing flaky test failure happening for testShardAlreadyReplicating() Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Fix possible flaky test for testBeforeIndexShardClosed_CancelsOngoingReplications() (#3963) * Fixing flaky test failure happening for testShardAlreadyReplicating() Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Removing assert segrep() in getProcessedLocalCheckpoint() of Index.shard class. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Adding back assert statement and make index setting to segment replication in SegmentReplicationSourceHandlerTests and SegmentReplicationTargetServiceTests. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Revert "Adding back assert statement and make index setting to segment replication in SegmentReplicationSourceHandlerTests and SegmentReplicationTargetServiceTests." Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> This reverts commit 8c5753b. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Co-authored-by: Marc Handalian <handalm@amazon.com> Co-authored-by: Poojita Raj <poojiraj@amazon.com>

Fixing flaky test failure happening for testShardAlreadyReplicating()

683880c

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 requested review from a team and reta as code owners July 19, 2022 06:10

Fixing incorrect import statement in class SegmentReplicationTargetSe…

571d49e

…rviceTests Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Merge branch 'main' into main

f93d316

mch2 approved these changes Jul 19, 2022

View reviewed changes

mch2 reviewed Jul 19, 2022

View reviewed changes

kartg approved these changes Jul 19, 2022

View reviewed changes

Rishikesh1159 added 2 commits July 20, 2022 15:35

Changing variable names and adding comments for testShardAlreadyRepli…

96dc44f

…cating() test case Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Merge branch 'main' of https://github.com/Rishikesh1159/OpenSearch in…

076f0ab

…to main Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Rishikesh1159 merged commit 4466a1f into opensearch-project:main Jul 20, 2022

Rishikesh1159 mentioned this pull request Aug 10, 2022

[Backport 2.x] [Segment Replication] Backport PR's : #3525 #3533 #3540 #3943 #3963 From main branch #4181

Merged

Rishikesh1159 added a commit to Rishikesh1159/OpenSearch that referenced this pull request Aug 10, 2022

Cherry-picking opensearch-project#3943.

c33936b

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Replication] Fixing flaky test failure happening for testShardAlreadyReplicating()#3943

[Segment Replication] Fixing flaky test failure happening for testShardAlreadyReplicating()#3943
Rishikesh1159 merged 5 commits intoopensearch-project:mainfrom
Rishikesh1159:main

Rishikesh1159 commented Jul 19, 2022

Uh oh!

github-actions bot commented Jul 19, 2022

Uh oh!

codecov-commenter commented Jul 19, 2022 •

edited by codecov bot

Loading

Uh oh!

github-actions bot commented Jul 19, 2022

Uh oh!

github-actions bot commented Jul 19, 2022

Uh oh!

mch2 left a comment

Uh oh!

mch2 Jul 19, 2022

Uh oh!

kartg left a comment

Uh oh!

kartg Jul 19, 2022

Uh oh!

kartg Jul 19, 2022

Uh oh!

github-actions bot commented Jul 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		cp = indexShard.getLatestReplicationCheckpoint();
		newCheckpoint = new ReplicationCheckpoint(

Conversation

Rishikesh1159 commented Jul 19, 2022

Description

Issues Resolved

Check List

Uh oh!

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

codecov-commenter commented Jul 19, 2022 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

mch2 left a comment

Choose a reason for hiding this comment

Uh oh!

mch2 Jul 19, 2022

Choose a reason for hiding this comment

Uh oh!

kartg left a comment

Choose a reason for hiding this comment

Uh oh!

kartg Jul 19, 2022

Choose a reason for hiding this comment

Uh oh!

kartg Jul 19, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 20, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jul 19, 2022 •

edited by codecov bot

Loading