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.

Sign up for GitHub

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

Jump to bottom

[Backport 2.x] Integrate Engine with decoupled Translog interfaces #3822

Merged
merged 19 commits into from
Aug 2, 2022
Merged

[Backport 2.x] Integrate Engine with decoupled Translog interfaces #3822

merged 19 commits into from
Aug 2, 2022

Conversation

satyajitg28
Copy link

Description

The PR aims to Integrate Engine with decoupled Translog interfaces. Original CR link. Ensuring Engine is backward compatible.

Issues Resolved

[List any issues this PR will resolve]

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.

@satyajitg28 satyajitg28 requested review from a team and reta as code owners July 8, 2022 16:42
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 marked this pull request as draft July 9, 2022 03:17
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from b3d63ae to bea5252 Compare July 14, 2022 10:02
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 changed the title Integrate Engine with decoupled Translog interfaces [2.x] [Backport 2.x] Integrate Engine with decoupled Translog interfaces Jul 14, 2022
@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 4c671bb to 1c3531b Compare July 14, 2022 13:31
@satyajitg28 satyajitg28 marked this pull request as ready for review July 14, 2022 13:32
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28
Copy link
Author

looks like a flaky test.
./gradlew ':server:test' --tests "org.opensearch.index.shard.RemoveCorruptedShardDataCommandTests.testCorruptedTranslog" passed on local.

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from a2eb953 to ed6ca73 Compare July 15, 2022 05:17
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from ed6ca73 to 833f3f7 Compare July 15, 2022 06:02
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2022 •
edited
Loading

Codecov Report

Merging #3822 (1c08624) into 2.x (bb353b6) will decrease coverage by 0.05%.
The diff coverage is 73.45%.

@@             Coverage Diff              @@
##                2.x    #3822      +/-   ##
============================================
- Coverage     70.54%   70.49%   -0.06%     
+ Complexity    56546    56545       -1     
============================================
  Files          4533     4533              
  Lines        272158   272232      +74     
  Branches      40003    40004       +1     
============================================
- Hits         191994   191907      -87     
- Misses        64020    64234     +214     
+ Partials      16144    16091      -53
Impacted Files Coverage Δ
...opensearch/index/translog/NoOpTranslogManager.java 71.42% <ø> (+71.42%) ⬆️
...index/translog/listener/TranslogEventListener.java 80.00% <ø> (+13.33%) ⬆️
...search/index/translog/InternalTranslogManager.java 54.62% <25.00%> (+14.45%) ⬆️
...n/java/org/opensearch/index/engine/NoOpEngine.java 60.19% <52.63%> (-2.89%) ⬇️
.../opensearch/index/engine/NRTReplicationEngine.java 56.49% <62.79%> (-6.60%) ⬇️
...va/org/opensearch/index/engine/ReadOnlyEngine.java 71.69% <80.00%> (+1.42%) ⬆️
...va/org/opensearch/index/engine/InternalEngine.java 74.72% <80.76%> (-0.13%) ⬇️
...va/org/opensearch/index/engine/EngineTestCase.java 87.48% <93.33%> (+1.92%) ⬆️
.../main/java/org/opensearch/index/engine/Engine.java 74.79% <100.00%> (+1.51%) ⬆️
...nslog/listener/CompositeTranslogEventListener.java 96.42% <100.00%> (-0.13%) ⬇️
... and 497 more

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

Bukhtawar
Bukhtawar reviewed Jul 15, 2022
View reviewed changes
test/framework/src/main/java/org/opensearch/index/engine/EngineTestCase.java Outdated
}
} finally {
IOUtils.close(replicaEngine, storeReplica, engine, store, () -> terminate(threadPool));
}
}

protected void assertEngineCleanedUp(Engine engine, Translog translog) throws Exception {
if (engine.isClosed.get() == false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this check redundant

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it was redundant. Removed it.

server/src/test/java/org/opensearch/index/shard/IndexShardTests.java Outdated
@@ -4036,8 +4036,7 @@ public void testCloseShardWhileResettingEngine() throws Exception {
CountDownLatch closeDoneLatch = new CountDownLatch(1);
IndexShard shard = newStartedShard(false, Settings.EMPTY, config -> new InternalEngine(config) {
@Override
public InternalEngine recoverFromTranslog(TranslogRecoveryRunner translogRecoveryRunner, long recoverUpToSeqNo)
throws IOException {
public Engine recoverFromTranslog(TranslogRecoveryRunner translogRecoveryRunner, long recoverUpToSeqNo) throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we revert this change. Any reason why couldn't use TranslogEventListener here?

Copy link
Author

Choose a reason for hiding this comment

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

Reverted the above change. Using TranslogEventListener, the test is getting stuck. Will debug it further.

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from a5715fa to 0729f79 Compare July 18, 2022 04:57
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 requested a review from reta July 18, 2022 05:27
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 0729f79 to 3711b3d Compare July 18, 2022 07:32
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28
Copy link
Author

Test failing: DiskThresholdDeciderIT.testHighWatermarkNotExceeded. Looks like a flaky test.

Satyajit Ganguly added 6 commits July 19, 2022 10:31
@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from caea217 to 55b9fd7 Compare July 19, 2022 05:01
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28
Copy link
Author

Again this test is failing: DiskThresholdDeciderIT.testHighWatermarkNotExceeded.

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 55b9fd7 to 902e780 Compare July 19, 2022 05:45
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 902e780 to 0f05090 Compare July 19, 2022 06:18
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 0f05090 to 5ad1379 Compare July 19, 2022 06:52
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 5ad1379 to 9c6fc2c Compare July 19, 2022 07:29
Synced IndexShard with 2.x
c00693f 
Signed-off-by: Satyajit Ganguly <[email protected]>
@satyajitg28 satyajitg28 force-pushed the decouple-txlog-2-2.x branch from 9c6fc2c to c00693f Compare July 19, 2022 07:34
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@satyajitg28 satyajitg28 requested a review from Bukhtawar July 20, 2022 11:04
@dblock
Copy link
Member

dblock commented Jul 21, 2022

@Bukhtawar care to take a look at this backport?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

@Bukhtawar Bukhtawar merged commit 57ce4a9 into opensearch-project:2.x Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reta reta reta approved these changes

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@satyajitg28 @codecov-commenter @dblock @reta @Bukhtawar