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] Add update doc integ test for Segment replication #4234

Conversation

dreamer-89
Copy link
Member

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

Description

This PR adds update document integration tests in order to verify the fix (#4185) works for update operations.

Verified without fix above, the added update IT fails with below error

[2022-08-16T20:03:03,300][ERROR][o.o.i.r.SegmentReplicationTargetService] [node_t1] replication failure
org.opensearch.OpenSearchException: Segment Replication failed
	at org.opensearch.indices.replication.SegmentReplicationTargetService$3.onFailure(SegmentReplicationTargetService.java:235) [main/:?]
...
Caused by: java.lang.IllegalStateException: Shard [test-idx-1][0] has local copies of segments that differ from the primary
	at org.opensearch.indices.replication.SegmentReplicationTarget.getFiles(SegmentReplicationTarget.java:167) ~[main/:?]
	... 24 more

Issues Resolved

#3787

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.

@dreamer-89 dreamer-89 requested review from a team and reta as code owners August 16, 2022 19:09
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Suraj Singh <surajrider@gmail.com>
@dreamer-89 dreamer-89 force-pushed the add_segment_replication_updates_tests branch from 1429d76 to 516f4d1 Compare August 16, 2022 22:53
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #4234 (516f4d1) into main (5f2e66b) will decrease coverage by 0.06%.
The diff coverage is 92.10%.

@@             Coverage Diff              @@
##               main    #4234      +/-   ##
============================================
- Coverage     70.78%   70.72%   -0.07%     
+ Complexity    57218    57169      -49     
============================================
  Files          4605     4606       +1     
  Lines        274695   274706      +11     
  Branches      40228    40228              
============================================
- Hits         194441   194281     -160     
- Misses        63955    64132     +177     
+ Partials      16299    16293       -6     
Impacted Files Coverage Δ
...main/java/org/opensearch/common/lucene/Lucene.java 67.30% <ø> (ø)
...c/main/java/org/opensearch/index/IndexService.java 74.09% <0.00%> (-0.40%) ⬇️
...index/codec/PerFieldMappingPostingFormatCodec.java 64.28% <ø> (ø)
...g/opensearch/index/engine/EngineConfigFactory.java 88.63% <ø> (ø)
...earch/index/translog/WriteOnlyTranslogManager.java 60.00% <ø> (ø)
...s/replication/SegmentReplicationSourceHandler.java 87.71% <ø> (-0.81%) ⬇️
...org/opensearch/index/shard/IndexShardTestCase.java 94.04% <ø> (ø)
...n/java/org/opensearch/index/engine/NoOpEngine.java 65.27% <80.00%> (+0.48%) ⬆️
...va/org/opensearch/index/engine/ReadOnlyEngine.java 72.59% <80.00%> (+0.20%) ⬆️
server/src/main/java/org/opensearch/Version.java 79.35% <100.00%> (-0.37%) ⬇️
... and 486 more

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

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

Nice... I do have a quick question before explicit approval.

import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchHits;

@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
public class SegmentReplicationIT extends OpenSearchIntegTestCase {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we just make this extend OpenSearchSingleNodeTestCase?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @nknize for the review. There are certain tests in this class which needs multiple nodes (specially failover related), so I think we can not use OpenSearchSingleNodeTestCase class. Please correct me if I am missing something.

@dreamer-89 dreamer-89 merged commit 06bef8e into opensearch-project:main Aug 23, 2022
@dreamer-89 dreamer-89 deleted the add_segment_replication_updates_tests branch August 23, 2022 22:50
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