[Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. #4182

mch2 · 2022-08-10T03:13:00Z

Signed-off-by: Marc Handalian handalm@amazon.com

Description

This change fixes segrep to work with multiple shards per node by keying ongoing replications on
allocation ID. This also updates cancel methods to ensure state is properly cleared on shard cancel.

Issues Resolved

#4129.

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.

…coveryNode. This change fixes segrep to work with multiple shards per node by keying ongoing replications on allocation ID. This also updates cancel methods to ensure state is properly cleared on shard cancel. Signed-off-by: Marc Handalian <handalm@amazon.com>

github-actions · 2022-08-10T03:37:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1644/
CommitID: c95112f

Signed-off-by: Marc Handalian <handalm@amazon.com>

dreamer-89 · 2022-08-10T04:07:00Z

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

+    private void cancelHandlers(Predicate<? super SegmentReplicationSourceHandler> predicate, String reason) {
+        final List<String> allocationIds = allocationIdToHandlers.values()
+            .stream()
+            .filter(predicate)
+            .map(SegmentReplicationSourceHandler::getAllocationId)
+            .collect(Collectors.toList());
+        for (String allocationId : allocationIds) {
+            final SegmentReplicationSourceHandler handler = allocationIdToHandlers.remove(allocationId);
+            if (handler != null) {
+                handler.cancel(reason);
+                removeCopyState(handler.getCopyState());
+            }
+        }
+    }


👍

This is better than previous iteration :)

dreamer-89

LGTM!

github-actions · 2022-08-10T04:12:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1645/
CommitID: 3092e172ae1728f84e036d647c3b71538e33304b

dreamer-89 · 2022-08-10T04:15:47Z

@mch2 1645 is failing with spotlessCheck.

github-actions · 2022-08-10T04:26:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1646/
CommitID: 6548f9b

ankitkala · 2022-08-10T11:15:32Z

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

@@ -38,7 +41,7 @@ class OngoingSegmentReplications {
    private final RecoverySettings recoverySettings;
    private final IndicesService indicesService;
    private final Map<ReplicationCheckpoint, CopyState> copyStateMap;
-    private final Map<DiscoveryNode, SegmentReplicationSourceHandler> nodesToHandlers;
+    private final Map<String, SegmentReplicationSourceHandler> allocationIdToHandlers;


~~Should we also remove the handler from allocationIdToHandlers after the segments have been copied()?~~

LGTM

…tion ID instead of DiscoveryNode. (opensearch-project#4182) * Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. This change fixes segrep to work with multiple shards per node by keying ongoing replications on allocation ID. This also updates cancel methods to ensure state is properly cleared on shard cancel. Signed-off-by: Marc Handalian <handalm@amazon.com> * Clean up cancel methods. Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Marc Handalian <handalm@amazon.com>

…aining segment replication changes (#4243) * [Segment Replication] Checkpoint Replay on Replica Shard (#3658) * Adding Latest Recevied checkpoint, replay checkpoint logic along with tests Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * [Segment Replication] Wire up segment replication with peer recovery and add ITs. (#3743) * Add null check when computing max segment version. With segment replication enabled it is possible Lucene does not set the SegmentInfos min segment version, leaving the default value as null. Signed-off-by: Marc Handalian <handalm@amazon.com> * Update peer recovery to set the translogUUID of replicas to the UUID generated on the primary. This change updates the UUID when the translog is created to the value stored in the passed segment userdata. This is to ensure during failover scenarios that the replica can be promoted and not have a uuid mismatch with the value stored in user data. Signed-off-by: Marc Handalian <handalm@amazon.com> * Wire up Segment Replication under the feature flag. This PR wires up segment replication and adds some initial integration tests. Signed-off-by: Marc Handalian <handalm@amazon.com> * Add test to ensure replicas use primary translog uuid with segrep. Signed-off-by: Marc Handalian <handalm@amazon.com> * Update SegmentReplicationIT to assert previous commit points are valid and SegmentInfos can be built. Fix nitpicks in PR feedback. Signed-off-by: Marc Handalian <handalm@amazon.com> * Fix test with Assert.fail to include a message. Signed-off-by: Marc Handalian <handalm@amazon.com> * Disable shard idle with segment replication. (#4118) This change disables shard idle when segment replication is enabled. Primary shards will only push out new segments on refresh, we do not want to block this based on search behavior. Replicas will only refresh on an externally provided SegmentInfos, so we do not want search requests to hang waiting for a refresh. Signed-off-by: Marc Handalian <handalm@amazon.com> * Fix isAheadOf logic for ReplicationCheckpoint comparison (#4112) Signed-off-by: Suraj Singh <surajrider@gmail.com> * Fix waitUntil refresh policy for segrep enabled indices. (#4137) Signed-off-by: Marc Handalian <handalm@amazon.com> * Add IndexShard#getLatestReplicationCheckpoint behind segrep enable feature flag (#4163) * Add IndexShard#getLatestReplicationCheckpoint behind segrep enable feature flag Signed-off-by: Suraj Singh <surajrider@gmail.com> * Address review comment. Move tests to SegmentReplicationIndexShardTests Signed-off-by: Suraj Singh <surajrider@gmail.com> * Add segrep enbaled index settings in TargetServiceTests, SourceHandlerTests Signed-off-by: Suraj Singh <surajrider@gmail.com> * [Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. (#4182) * Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. This change fixes segrep to work with multiple shards per node by keying ongoing replications on allocation ID. This also updates cancel methods to ensure state is properly cleared on shard cancel. Signed-off-by: Marc Handalian <handalm@amazon.com> * Clean up cancel methods. Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Marc Handalian <handalm@amazon.com> * [Bug] [Segment Replication] Update store metadata recovery diff logic to ignore missing files causing exception (#4185) * Update Store for segment replication dif Signed-off-by: Poojita Raj <poojiraj@amazon.com> Signed-off-by: Poojita Raj <poojiraj@amazon.com> * Update recoveryDiff logic to ingore missing files causing exception on replica during copy Signed-off-by: Suraj Singh <surajrider@gmail.com> * Address review comments Signed-off-by: Suraj Singh <surajrider@gmail.com> Signed-off-by: Poojita Raj <poojiraj@amazon.com> Signed-off-by: Suraj Singh <surajrider@gmail.com> Co-authored-by: Poojita Raj <poojiraj@amazon.com> * [Segment Replication] Adding PrimaryMode check before publishing checkpoint and processing a received checkpoint. (#4157) * Adding PrimaryMode check before publishing checkpoint. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Applying spotless check Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Moving segrep specific tests to SegmentReplicationIndexShardTests. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Adding logic and tests for rejecting checkpoints if shard is in PrimaryMode. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Applying ./gradlew :server:spotlessApply. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Applying ./gradlew :server:spotlessApply Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Changing log level to warn in shouldProcessCheckpoint() of IndexShard.java class. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * Removing unnecessary lazy logging in shouldProcessCheckpoint(). Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> * [Segment Replication] Wait for documents to replicate to replica shards (#4236) * [Segment Replication] Add thread sleep to account for replica lag in delete operations test Signed-off-by: Suraj Singh <surajrider@gmail.com> * Address review comments, assertBusy on doc count rather than sleep Signed-off-by: Suraj Singh <surajrider@gmail.com> Signed-off-by: Suraj Singh <surajrider@gmail.com> * Segment Replication - Remove unnecessary call to markAllocationIdAsInSync. (#4224) This PR Removes an unnecessary call to markAllocationIdAsInSync on the primary shard when replication events complete. Recovery will manage this initial call. Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Marc Handalian <handalm@amazon.com> * Segment Replication - Add additional unit tests for update & delete ops. (#4237) * Segment Replication - Add additional unit tests for update & delete operations. Signed-off-by: Marc Handalian <handalm@amazon.com> * Fix spotless. Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com> Signed-off-by: Marc Handalian <handalm@amazon.com> Signed-off-by: Suraj Singh <surajrider@gmail.com> Signed-off-by: Poojita Raj <poojiraj@amazon.com> Co-authored-by: Marc Handalian <handalm@amazon.com> Co-authored-by: Suraj Singh <surajrider@gmail.com> Co-authored-by: Poojita Raj <poojiraj@amazon.com>

mch2 requested review from a team and reta as code owners August 10, 2022 03:13

mch2 mentioned this pull request Aug 10, 2022

[Segment Replication BUG] Replica shard fails during segment replication during indexing / bulk indexing calls #4129

Closed

Clean up cancel methods.

6548f9b

Signed-off-by: Marc Handalian <handalm@amazon.com>

mch2 force-pushed the 4129 branch from 3092e17 to 6548f9b Compare August 10, 2022 03:56

dreamer-89 reviewed Aug 10, 2022

View reviewed changes

dreamer-89 approved these changes Aug 10, 2022

View reviewed changes

ankitkala reviewed Aug 10, 2022

View reviewed changes

ankitkala approved these changes Aug 10, 2022

View reviewed changes

mch2 merged commit 7dad063 into opensearch-project:main Aug 10, 2022

mch2 deleted the 4129 branch August 10, 2022 17:07

Rishikesh1159 mentioned this pull request Aug 17, 2022

[Backport 2.x] [Segment Replication] Backport all PR's containing remaining segment replication changes #4243

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. #4182

[Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. #4182

mch2 commented Aug 10, 2022

github-actions bot commented Aug 10, 2022

dreamer-89 Aug 10, 2022

dreamer-89 left a comment

github-actions bot commented Aug 10, 2022

dreamer-89 commented Aug 10, 2022

github-actions bot commented Aug 10, 2022

ankitkala Aug 10, 2022 •

edited

Loading

[Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. #4182

[Segment Replication] Fix OngoingSegmentReplications to key by allocation ID instead of DiscoveryNode. #4182

Conversation

mch2 commented Aug 10, 2022

Description

Issues Resolved

Check List

github-actions bot commented Aug 10, 2022

Gradle Check (Jenkins) Run Completed with:

dreamer-89 Aug 10, 2022

Choose a reason for hiding this comment

dreamer-89 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 10, 2022

Gradle Check (Jenkins) Run Completed with:

dreamer-89 commented Aug 10, 2022

github-actions bot commented Aug 10, 2022

Gradle Check (Jenkins) Run Completed with:

ankitkala Aug 10, 2022 • edited Loading

Choose a reason for hiding this comment

ankitkala Aug 10, 2022 •

edited

Loading