[Bug] [Segment Replication] Fix isAheadOf logic for ReplicationCheckpoint comparison #4112

dreamer-89 · 2022-08-03T19:04:52Z

Signed-off-by: Suraj Singh surajrider@gmail.com

Description

Coming from #3988 which relies on ReplicationCheckpoint ordering for correctly identifying the furthest ahead replica. The existing ReplicationCheckpoint#isAheadOf is not correct as it uses || b/w segmentInfosVersion (1st) & PrimaryTerm(2nd) terms; instead of relying on 2nd term (PrimaryTerm field) ONLY WHEN first field (segmentInfosVersion) is equal.

This PR also fixes, the comparison order i.e. PrimaryTerm first followed by segmentInfosVersion; this is to prioritize active primary over stale.

Issues Related

#3988

Gradle Check (Jenkins) Run Completed with:

RESULT: ABORTED ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1379/
CommitID: c992024

dreamer-89 · 2022-08-03T21:13:47Z

> Task :distribution:bwc:staged:buildBwcLinuxTar FAILED

> Task :distribution:bwc:minor:buildBwcLinuxTar
 [2.3.0] > Task :buildSrc:pluginUnderTestMetadata FAILED
 [2.3.0] 
 [2.3.0] 11 actionable tasks: 11 executed
 [2.3.0] FAILURE: Build failed with an exception.
 [2.3.0] 
 [2.3.0] * What went wrong:
 [2.3.0] Execution failed for task ':buildSrc:pluginUnderTestMetadata'.
 [2.3.0] > Timeout waiting to lock file hash cache (/home/ubuntu/.gradle/caches/7.5/fileHashes). It is currently in use by another Gradle instance.
 [2.3.0]   Owner PID: 28733
 [2.3.0]   Our PID: 32188
 [2.3.0]   Owner Operation: 
 [2.3.0]   Our operation: 
 [2.3.0]   Lock file: /home/ubuntu/.gradle/caches/7.5/fileHashes/fileHashes.lock
 [2.3.0] 
 [2.3.0] * Try:
 [2.3.0] > Run with --stacktrace option to get the stack trace.
 [2.3.0] > Run with --info or --debug option to get more log output.
 [2.3.0] 
 [2.3.0] * Get more help at https://help.gradle.org
 [2.3.0] 
 [2.3.0] BUILD FAILED in 2m 4s
Output for /var/jenkins/workspace/gradle-check/search/distribution/bwc/minor/build/bwc/checkout-2.x/gradlew:

> Task :distribution:bwc:minor:buildBwcLinuxTar FAILED

github-actions · 2022-08-03T21:39:20Z

Gradle Check (Jenkins) Run Completed with:

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

codecov-commenter · 2022-08-03T21:42:24Z

Codecov Report

Merging #4112 (c992024) into main (6f43dbc) will decrease coverage by 0.07%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##               main    #4112      +/-   ##
============================================
- Coverage     70.61%   70.53%   -0.08%     
- Complexity    56983    57004      +21     
============================================
  Files          4595     4595              
  Lines        274234   274236       +2     
  Branches      40181    40183       +2     
============================================
- Hits         193638   193427     -211     
- Misses        64342    64564     +222     
+ Partials      16254    16245       -9

Impacted Files	Coverage Δ
...pensearch/common/settings/IndexScopedSettings.java	`100.00% <ø> (ø)`
.../java/org/opensearch/snapshots/RestoreService.java	`55.37% <0.00%> (ø)`
.../replication/checkpoint/ReplicationCheckpoint.java	`53.48% <33.33%> (-0.18%)`	⬇️
...org/opensearch/cluster/metadata/IndexMetadata.java	`83.85% <100.00%> (ø)`
.../main/java/org/opensearch/index/IndexSettings.java	`86.35% <100.00%> (ø)`
...a/org/opensearch/client/cluster/SniffModeInfo.java	`0.00% <0.00%> (-58.83%)`	⬇️
...a/org/opensearch/tasks/TaskCancelledException.java	`50.00% <0.00%> (-50.00%)`	⬇️
...cluster/coordination/PendingClusterStateStats.java	`20.00% <0.00%> (-48.00%)`	⬇️
.../org/opensearch/client/indices/AnalyzeRequest.java	`31.00% <0.00%> (-45.00%)`	⬇️
...rch/client/transport/NoNodeAvailableException.java	`28.57% <0.00%> (-42.86%)`	⬇️
... and 470 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

…project#4112) Signed-off-by: Suraj Singh <surajrider@gmail.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>

opensearch-trigger-bot · 2022-09-02T20:22:14Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4112-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 7af8e37f81c0a124712a8cf86dab973df26f906f
# Push it to GitHub
git push --set-upstream origin backport/backport-4112-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4112-to-2.x.

Update isAheadOf logic to compare PrimaryTerms when segmentInfosVersi…

c992024

…ons are equal Signed-off-by: Suraj Singh <surajrider@gmail.com>

dreamer-89 requested review from a team and reta as code owners August 3, 2022 19:04

kartg approved these changes Aug 3, 2022

View reviewed changes

Rishikesh1159 approved these changes Aug 3, 2022

View reviewed changes

dreamer-89 merged commit 7af8e37 into opensearch-project:main Aug 3, 2022

dreamer-89 mentioned this pull request Aug 4, 2022

[Segment Replication] Update PrimaryShardAllocator to prefer replicas with higher replication checkpoint #4041

Merged

Rishikesh1159 pushed a commit to Rishikesh1159/OpenSearch that referenced this pull request Aug 17, 2022

Fix isAheadOf logic for ReplicationCheckpoint comparison (opensearch-…

a9a7369

…project#4112) Signed-off-by: Suraj Singh <surajrider@gmail.com>

Rishikesh1159 mentioned this pull request Aug 17, 2022

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

Merged

kartg mentioned this pull request Aug 30, 2022

Adding @dreamer-89 to Opensearch maintainers. #4342

Merged

2 tasks

dreamer-89 added the backport 2.x Backport to 2.x branch label Sep 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug] [Segment Replication] Fix isAheadOf logic for ReplicationCheckpoint comparison #4112

[Bug] [Segment Replication] Fix isAheadOf logic for ReplicationCheckpoint comparison #4112

dreamer-89 commented Aug 3, 2022 •

edited

Loading

github-actions bot commented Aug 3, 2022

dreamer-89 commented Aug 3, 2022

github-actions bot commented Aug 3, 2022

codecov-commenter commented Aug 3, 2022

opensearch-trigger-bot bot commented Sep 2, 2022

[Bug] [Segment Replication] Fix isAheadOf logic for ReplicationCheckpoint comparison #4112

[Bug] [Segment Replication] Fix isAheadOf logic for ReplicationCheckpoint comparison #4112

Conversation

dreamer-89 commented Aug 3, 2022 • edited Loading

Description

Issues Related

Related

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

dreamer-89 commented Aug 3, 2022

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Aug 3, 2022

Codecov Report

opensearch-trigger-bot bot commented Sep 2, 2022

dreamer-89 commented Aug 3, 2022 •

edited

Loading