Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Segment Replication] Fix possible flaky test for testBeforeIndexShardClosed_CancelsOngoingReplications() #3963

Merged
merged 9 commits into from
Jul 21, 2022

Conversation

Rishikesh1159
Copy link
Member

Description

This PR fixes possible flaky test failure for testBeforeIndexShardClosed_CancelsOngoingReplications() by adding null check

Issues Resolved

Related to #3872

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.

Rishikesh1159 and others added 7 commits July 19, 2022 06:05
Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
…rviceTests

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

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

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
@Rishikesh1159 Rishikesh1159 requested review from a team and reta as code owners July 20, 2022 20:53
Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@@ -172,6 +172,10 @@ public void run() {

private void start(final long replicationId) {
try (ReplicationRef<SegmentReplicationTarget> replicationRef = onGoingReplications.get(replicationId)) {
if (replicationRef == null) {
logger.trace("Cannot find any running replication with id [{}]", replicationId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a log line is necessary here. Just include a comment that this is for handling edge cases where the reference is removed before the ReplicationRunner is started by the threadpool.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

Codecov Report

Merging #3963 (b58e19e) into main (4466a1f) will decrease coverage by 0.17%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #3963      +/-   ##
============================================
- Coverage     70.68%   70.50%   -0.18%     
+ Complexity    56835    56721     -114     
============================================
  Files          4573     4573              
  Lines        273255   273258       +3     
  Branches      40076    40078       +2     
============================================
- Hits         193143   192655     -488     
- Misses        63871    64418     +547     
+ Partials      16241    16185      -56     
Impacted Files Coverage Δ
...s/replication/SegmentReplicationTargetService.java 71.92% <0.00%> (-4.00%) ⬇️
...adonly/AddIndexBlockClusterStateUpdateRequest.java 0.00% <0.00%> (-75.00%) ⬇️
...readonly/TransportVerifyShardIndexBlockAction.java 9.75% <0.00%> (-73.18%) ⬇️
...luster/routing/allocation/RoutingExplanations.java 41.37% <0.00%> (-58.63%) ⬇️
...n/admin/indices/readonly/AddIndexBlockRequest.java 17.85% <0.00%> (-53.58%) ⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java 55.00% <0.00%> (-45.00%) ⬇️
...regations/metrics/AbstractHyperLogLogPlusPlus.java 51.72% <0.00%> (-44.83%) ⬇️
...opensearch/persistent/PersistentTasksExecutor.java 22.22% <0.00%> (-44.45%) ⬇️
...adcast/BroadcastShardOperationFailedException.java 55.55% <0.00%> (-44.45%) ⬇️
...pensearch/indices/breaker/CircuitBreakerStats.java 27.77% <0.00%> (-41.67%) ⬇️
... and 462 more

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@Rishikesh1159 Rishikesh1159 merged commit 8b5a10c into opensearch-project:main Jul 21, 2022
@Rishikesh1159 Rishikesh1159 changed the title Fix possible flaky test for testBeforeIndexShardClosed_CancelsOngoingReplications() [Segment Replication] Fix possible flaky test for testBeforeIndexShardClosed_CancelsOngoingReplications() Jul 21, 2022
Rishikesh1159 added a commit to Rishikesh1159/OpenSearch that referenced this pull request Aug 10, 2022
…Replications() (opensearch-project#3963)

* Fixing flaky test failure happening for testShardAlreadyReplicating()

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Rishikesh1159 added a commit to Rishikesh1159/OpenSearch that referenced this pull request Aug 10, 2022
…Replications() (opensearch-project#3963)

* Fixing flaky test failure happening for testShardAlreadyReplicating()

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Rishikesh1159 added a commit that referenced this pull request Aug 11, 2022
…#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants