Segment Replication - Release incorrectly retained commits on primary shards #6660

mch2 · 2023-03-14T02:20:26Z

This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy.

Description

This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy.

Issues Resolved

closes #6578

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
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

…rimary shards This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy. Signed-off-by: Marc Handalian <[email protected]>

github-actions · 2023-03-14T02:37:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/12402/
CommitID: 41eec4a
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?

dreamer-89 · 2023-03-14T02:55:30Z

server/src/main/java/org/opensearch/indices/replication/OngoingSegmentReplications.java

@@ -254,8 +260,22 @@ private void cancelHandlers(Predicate<? super SegmentReplicationSourceHandler> p
            .filter(predicate)
            .map(SegmentReplicationSourceHandler::getAllocationId)
            .collect(Collectors.toList());
+        logger.trace(() -> new ParameterizedMessage("Cancelling replications for allocationIds {}", allocationIds));


nit: As cancellation is an unexpected event, I think we can log this with warn or debug level.

This method is a helper that is used in multiple spots - both for cancellations on node drop and when a local primary is shutting down, this is why I thought trace was more appropriate, though these are valuable logs, will change to warn.

dreamer-89 · 2023-03-14T02:57:10Z

server/src/main/java/org/opensearch/indices/replication/OngoingSegmentReplications.java

+        cancelHandlers(
+            (handler) -> handler.getCopyState().getShard().shardId().equals(shardId)
+                && inSyncAllocationIds.contains(handler.getAllocationId()) == false,
+            "Shard is no longer in-sync with the primary"
+        );


nit: Can we add a unit test validating cancellation ?

dreamer-89 · 2023-03-14T03:01:34Z

...er/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java

+            refresh(INDEX_NAME);
+        }
+        // Refresh, this should trigger round of segment replication
+        assertBusy(() -> { assertDocCounts(docCount, replicaNode); });


I think irrespective of whether replica has different segment files, this test will pass ?. Can we assert on a different allocationID on shard to ensure shard indeed failed on replica ?

added this.

dreamer-89 · 2023-03-14T03:07:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌

URL: https://build.ci.opensearch.org/job/gradle-check/12402/

CommitID: 41eec4a
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?

Not sure why this failed. Let's see current gradle check output.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:forbiddenApisMain'.
> de.thetaphi.forbiddenapis.ForbiddenApiException: Check for forbidden API calls failed, see log.

github-actions · 2023-03-14T03:08:22Z

Gradle Check (Jenkins) Run Completed with:

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

codecov-commenter · 2023-03-14T03:09:33Z

Codecov Report

Merging #6660 (f76dc57) into main (9e1f9ad) will increase coverage by 0.13%.
The diff coverage is 37.50%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6660      +/-   ##
============================================
+ Coverage     70.61%   70.74%   +0.13%     
- Complexity    59071    59177     +106     
============================================
  Files          4803     4803              
  Lines        283192   283208      +16     
  Branches      40837    40842       +5     
============================================
+ Hits         199981   200368     +387     
+ Misses        66804    66441     -363     
+ Partials      16407    16399       -8

Impacted Files	Coverage Δ
...s/replication/SegmentReplicationSourceService.java	`52.11% <0.00%> (-7.57%)`	⬇️
...ndices/replication/OngoingSegmentReplications.java	`92.40% <85.71%> (-0.66%)`	⬇️

... and 522 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Marc Handalian <[email protected]>

github-actions · 2023-03-14T04:10:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

URL: https://build.ci.opensearch.org/job/gradle-check/12406/
CommitID: f76dc57
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

… shards (#6660) * Segment Replication - Release incorrectly retained index commits on primary shards This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy. Signed-off-by: Marc Handalian <[email protected]> * PR feedback. Signed-off-by: Marc Handalian <[email protected]> * spotless fix. Signed-off-by: Marc Handalian <[email protected]> --------- Signed-off-by: Marc Handalian <[email protected]> (cherry picked from commit 73a2279) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… shards (#6660) (#6661) * Segment Replication - Release incorrectly retained index commits on primary shards This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy. * PR feedback. * spotless fix. --------- (cherry picked from commit 73a2279) Signed-off-by: Marc Handalian <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

… shards (opensearch-project#6660) * Segment Replication - Release incorrectly retained index commits on primary shards This change ensures that primary shards clean up any state when a replica is marked out of sync. This can happen when replicas fail due to store corruption or mismatching segments during file copy. Signed-off-by: Marc Handalian <[email protected]> * PR feedback. Signed-off-by: Marc Handalian <[email protected]> * spotless fix. Signed-off-by: Marc Handalian <[email protected]> --------- Signed-off-by: Marc Handalian <[email protected]> Signed-off-by: Mingshi Liu <[email protected]>

mch2 changed the title ~~Segment Replication - Release incorrectly retained index commits on p…~~ Segment Replication - Release incorrectly retained commits on primary shards Mar 14, 2023

mch2 added the skip-changelog label Mar 14, 2023

mch2 marked this pull request as ready for review March 14, 2023 02:21

mch2 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and xuezhou25 as code owners March 14, 2023 02:21

This comment was marked as outdated.

Sign in to view

mch2 force-pushed the insync branch from 2422ea9 to 41eec4a Compare March 14, 2023 02:28

mch2 force-pushed the insync branch from 41eec4a to e259bbe Compare March 14, 2023 02:35

dreamer-89 approved these changes Mar 14, 2023

View reviewed changes

mch2 added 2 commits March 13, 2023 20:35

PR feedback.

9b73b11

Signed-off-by: Marc Handalian <[email protected]>

spotless fix.

f76dc57

Signed-off-by: Marc Handalian <[email protected]>

This comment was marked as outdated.

Sign in to view

mch2 merged commit 73a2279 into opensearch-project:main Mar 14, 2023

mch2 added the backport 2.x Backport to 2.x branch label Mar 14, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 14, 2023

[Backport 2.x] Segment Replication - Release incorrectly retained commits on primary shards #6661

Merged

mch2 deleted the insync branch March 14, 2023 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Segment Replication - Release incorrectly retained commits on primary shards #6660

Segment Replication - Release incorrectly retained commits on primary shards #6660

mch2 commented Mar 14, 2023 •

edited

Loading

This comment was marked as outdated.

github-actions bot commented Mar 14, 2023

dreamer-89 Mar 14, 2023

mch2 Mar 14, 2023 •

edited

Loading

dreamer-89 Mar 14, 2023

mch2 Mar 14, 2023

dreamer-89 Mar 14, 2023 •

edited

Loading

mch2 Mar 14, 2023

dreamer-89 commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 14, 2023

codecov-commenter commented Mar 14, 2023 •

edited

Loading

This comment was marked as outdated.

github-actions bot commented Mar 14, 2023

Segment Replication - Release incorrectly retained commits on primary shards #6660

Segment Replication - Release incorrectly retained commits on primary shards #6660

Conversation

mch2 commented Mar 14, 2023 • edited Loading

Description

Issues Resolved

Check List

This comment was marked as outdated.

github-actions bot commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

dreamer-89 Mar 14, 2023

Choose a reason for hiding this comment

mch2 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

dreamer-89 Mar 14, 2023

Choose a reason for hiding this comment

mch2 Mar 14, 2023

Choose a reason for hiding this comment

dreamer-89 Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

mch2 Mar 14, 2023

Choose a reason for hiding this comment

dreamer-89 commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Mar 14, 2023 • edited Loading

Codecov Report

This comment was marked as outdated.

github-actions bot commented Mar 14, 2023

Gradle Check (Jenkins) Run Completed with:

mch2 commented Mar 14, 2023 •

edited

Loading

mch2 Mar 14, 2023 •

edited

Loading

dreamer-89 Mar 14, 2023 •

edited

Loading

codecov-commenter commented Mar 14, 2023 •

edited

Loading