[Segment Replication] Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery #9002
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ng translog replay from recovery. Signed-off-by: Rishikesh1159 <[email protected]>
Rishikesh1159
requested review from
reta,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
gbbafna,
setiah,
kartg,
kotwanikunal,
mch2,
nknize,
owaiskazi19,
ryanbogan,
saratvemulapalli,
shwetathareja,
dreamer-89,
tlfeng,
VachaShah,
dbwiddis,
sachinpkale and
sohami
as code owners
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
|
Poojita-Raj
reviewed
Aug 1, 2023
server/src/main/java/org/opensearch/index/shard/IndexShard.java
Outdated
Show resolved
Hide resolved
|
Thanks for catching this @Rishikesh1159. While we are still working on these tests randomly using segrep, can we add a segrep version of the IT that caught this that always runs? |
mch2
reviewed
Aug 1, 2023
server/src/main/java/org/opensearch/index/shard/IndexShard.java
Outdated
Show resolved
Hide resolved
…cation enabled replica shard. Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
|
Compatibility status: |
Signed-off-by: Rishikesh1159 <[email protected]>
|
Compatibility status: |
Gradle Check (Jenkins) Run Completed with:
|
|
Compatibility status: |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
@@ Coverage Diff @@
## main #9002 +/- ##
============================================
+ Coverage 71.03% 71.12% +0.08%
- Complexity 57233 57313 +80
============================================
Files 4765 4765
Lines 270334 270335 +1
Branches 39538 39539 +1
============================================
+ Hits 192040 192278 +238
+ Misses 62089 61842 -247
- Partials 16205 16215 +10
|
Poojita-Raj
approved these changes
Aug 3, 2023
mch2
approved these changes
Aug 3, 2023
There was a problem hiding this comment.
Thanks for finding this @Rishikesh1159 ! Only a nit so approved.
...er/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationIT.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Rishikesh1159 <[email protected]>
…hikesh1159/OpenSearch into fix-seqNo-Prcesses-twice-bug
|
Compatibility status: |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Aug 6, 2023
…led replica shard during translog replay from recovery (#9002) * Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery. Signed-off-by: Rishikesh1159 <[email protected]> * Adding unit test to verify document is not parsed on an segment replication enabled replica shard. Signed-off-by: Rishikesh1159 <[email protected]> * remove unnecessary unit tests and address comments. Signed-off-by: Rishikesh1159 <[email protected]> * address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> (cherry picked from commit 435408b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Rishikesh1159
pushed a commit
that referenced
this pull request
Aug 7, 2023
…led replica shard during translog replay from recovery (#9002) (#9140) * Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery. * Adding unit test to verify document is not parsed on an segment replication enabled replica shard. * remove unnecessary unit tests and address comments. * address comments on PR. --------- (cherry picked from commit 435408b) Signed-off-by: Rishikesh1159 <[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>
kaushalmahi12
pushed a commit
to kaushalmahi12/OpenSearch
that referenced
this pull request
Sep 12, 2023
…led replica shard during translog replay from recovery (opensearch-project#9002) * Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery. Signed-off-by: Rishikesh1159 <[email protected]> * Adding unit test to verify document is not parsed on an segment replication enabled replica shard. Signed-off-by: Rishikesh1159 <[email protected]> * remove unnecessary unit tests and address comments. Signed-off-by: Rishikesh1159 <[email protected]> * address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> Signed-off-by: Kaushal Kumar <[email protected]>
brusic
pushed a commit
to brusic/OpenSearch
that referenced
this pull request
Sep 25, 2023
…led replica shard during translog replay from recovery (opensearch-project#9002) * Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery. Signed-off-by: Rishikesh1159 <[email protected]> * Adding unit test to verify document is not parsed on an segment replication enabled replica shard. Signed-off-by: Rishikesh1159 <[email protected]> * remove unnecessary unit tests and address comments. Signed-off-by: Rishikesh1159 <[email protected]> * address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> Signed-off-by: Ivan Brusic <[email protected]>
shiv0408
pushed a commit
to Gaurav614/OpenSearch
that referenced
this pull request
Apr 25, 2024
…led replica shard during translog replay from recovery (opensearch-project#9002) * Remove Doc Parsing for segment replication enabled replica shard during translog replay from recovery. Signed-off-by: Rishikesh1159 <[email protected]> * Adding unit test to verify document is not parsed on an segment replication enabled replica shard. Signed-off-by: Rishikesh1159 <[email protected]> * remove unnecessary unit tests and address comments. Signed-off-by: Rishikesh1159 <[email protected]> * address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes removes Doc parsing step for segment replication enabled replica shard, which is called during translog replay in phase 2 of recovery.
There are two ways an operation can be written into replica shard's translog:
-> First Way: This is from applyIndexOperationOnReplica(), where we avoid doc parsing and we set routing to null.
-> Second way: This is from applyTranslogOperation(), here we don't avoid doc parsing and parse actual routing value from input.
When phase 2 of recovery is started for a replica shard, it opens it's engine to write new translog operations (First way mentioned above )and then later we replay translog ops from primary shard (Second way mentioned above) during recovery. Sometimes a translog operation might be already written into replica's shard but it is still replayed from primary. Usually translog writer will be able to recognize that same operation is happening and does a noop if it is already present.
But sometimes in case of segrep replica shard, the translog writer is not able to recognize both of them as same operation because the routing value in one operation is null and routing value in another will be an actual parsed routing value. So we hit the following error #8848 sometimes.
To avoid this situation, this PR removes doc parsing even in second way on a segrep enabled replica shard. This way we completely remove doc parsing step for a segrep enabled replica shard.
Related Issues
Resolves #8848
Check List
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.