Introduce TranslogFactory for Local/Remote Translog support #4172

Bukhtawar · 2022-08-09T08:25:39Z

Signed-off-by: Bukhtawar Khan [email protected]

Description

The PR tries to introduce a Translog factory for supporting remote file-system backed txlog. The remote file-system backed translog will still use operations committed on local on-disk txlog and will upload the same to the remote blob store container.

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.

Signed-off-by: Bukhtawar Khan <[email protected]>

github-actions · 2022-08-09T08:35:09Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1613/
CommitID: 4c0bfc4

github-actions · 2022-08-09T08:58:58Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1615/
CommitID: 1e022c6

Signed-off-by: Bukhtawar Khan <[email protected]>

github-actions · 2022-08-09T09:39:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1616/
CommitID: 8a56076

codecov-commenter · 2022-08-09T09:40:56Z

Codecov Report

Merging #4172 (8a56076) into main (a469a3c) will increase coverage by 0.03%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main    #4172      +/-   ##
============================================
+ Coverage     70.59%   70.63%   +0.03%     
+ Complexity    57083    57081       -2     
============================================
  Files          4603     4604       +1     
  Lines        274551   274561      +10     
  Branches      40210    40211       +1     
============================================
+ Hits         193831   193933     +102     
+ Misses        64514    64394     -120     
- Partials      16206    16234      +28

Impacted Files	Coverage Δ
...c/main/java/org/opensearch/index/IndexService.java	`73.63% <0.00%> (-0.17%)`	⬇️
...g/opensearch/index/engine/EngineConfigFactory.java	`88.63% <ø> (ø)`
...earch/index/translog/WriteOnlyTranslogManager.java	`60.00% <ø> (ø)`
...org/opensearch/index/shard/IndexShardTestCase.java	`91.45% <ø> (-2.60%)`	⬇️
...n/java/org/opensearch/index/engine/NoOpEngine.java	`65.27% <80.00%> (+0.48%)`	⬆️
...va/org/opensearch/index/engine/ReadOnlyEngine.java	`74.07% <80.00%> (+1.68%)`	⬆️
...java/org/opensearch/index/engine/EngineConfig.java	`97.40% <100.00%> (+0.06%)`	⬆️
...va/org/opensearch/index/engine/InternalEngine.java	`75.39% <100.00%> (-0.70%)`	⬇️
.../opensearch/index/engine/NRTReplicationEngine.java	`76.19% <100.00%> (+0.99%)`	⬆️
...in/java/org/opensearch/index/shard/IndexShard.java	`69.10% <100.00%> (-0.24%)`	⬇️
... and 477 more

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

sachinpkale · 2022-08-12T09:18:38Z

Please add description, issue link and update the checklist.

sachinpkale · 2022-08-12T09:02:42Z

server/src/main/java/org/opensearch/index/IndexService.java

@@ -547,6 +548,8 @@ public synchronized IndexShard createShard(
                () -> globalCheckpointSyncer.accept(shardId),
                retentionLeaseSyncer,
                circuitBreakerService,
+                // TODO Replace with remote translog factory in the follow up PR
+                this.indexSettings.isRemoteTranslogStoreEnabled() ? null : new InternalTranslogFactory(),


Do we also need to check if the node is primary?

Ideally the engine should take care of using the write flavor of TranslogManager. Will ensure we add the right assertions once RemoteFactory changes are made. There is a PR in flight #4127 which will take care of primary and replica

sachinpkale · 2022-08-12T09:08:04Z

server/src/main/java/org/opensearch/index/engine/EngineConfig.java

@@ -253,7 +257,8 @@ public EngineConfig(
            retentionLeasesSupplier,
            primaryTermSupplier,
            tombstoneDocSupplier,
-            false
+            false,
+            new InternalTranslogFactory()


Don't we need similar check that is added for IndexService here as well?

this.indexSettings.isRemoteTranslogStoreEnabled() ? null : new InternalTranslogFactory()

This is the default constructor for EngineConfig

ashking94

LGTM

ashking94 · 2022-08-16T09:20:00Z

This looks good. There is one thing which will probably play a crucial role in defining the lower level semantics - We have TranslogManager, & TranslogFactory. Basis my understanding, TranslogFactory would provide us different abstraction for storage (local/remote) and TranslogManager helps us define integration of the Engine with the underlying Translog provided by TranslogFactory. This makes it look that TranslogFactory and TranslogManager implementation might be hardwired, is it fairly right understanding? I think this is a good extensibility point - but may be in future we might want to converge to single TranslogManagerCumFactory that helps us with both the needs?

mch2

LGTM

…ch-project#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]>

opensearch-trigger-bot · 2023-01-09T08:12:11Z

The backport to 2.x failed:

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

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
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4172-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 551f7c3481040a3b4b051a27eec973c90fb5def6
# Push it to GitHub
git push --set-upstream origin backport/backport-4172-to-2.x
# Go back to the original working tree
popd
# 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-4172-to-2.x.

…ch-project#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]>

…hanges (#5757) * Introduce TranslogFactory for Local/Remote Translog support (#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]> * [Remote Translog] Introduce remote translog with upload functionality (#5392) * Introduce remote translog with upload functionality Signed-off-by: Gaurav Bafna <[email protected]> Co-authored-by: Bukhtawar Khan <[email protected]> * Enable creation of indices using Remote Translog (#5638) * Enable creation of indices using Remote Translog behind a setting and feature flag Signed-off-by: Gaurav Bafna <[email protected]> * [Remote Translog] Add support for downloading files from remote translog (#5649) * Add support to download translog from remote store during recovery Signed-off-by: Sachin Kale <[email protected]> * Integrate remote translog download on failover (#5699) * Integrate remote translog download on failover Signed-off-by: Ashish Singh <[email protected]> Signed-off-by: Bukhtawar Khan <[email protected]> Signed-off-by: Gaurav Bafna <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Ashish Singh <[email protected]>

…hanges (opensearch-project#5757) * Introduce TranslogFactory for Local/Remote Translog support (opensearch-project#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]> * [Remote Translog] Introduce remote translog with upload functionality (opensearch-project#5392) * Introduce remote translog with upload functionality Signed-off-by: Gaurav Bafna <[email protected]> Co-authored-by: Bukhtawar Khan <[email protected]> * Enable creation of indices using Remote Translog (opensearch-project#5638) * Enable creation of indices using Remote Translog behind a setting and feature flag Signed-off-by: Gaurav Bafna <[email protected]> * [Remote Translog] Add support for downloading files from remote translog (opensearch-project#5649) * Add support to download translog from remote store during recovery Signed-off-by: Sachin Kale <[email protected]> * Integrate remote translog download on failover (opensearch-project#5699) * Integrate remote translog download on failover Signed-off-by: Ashish Singh <[email protected]> Signed-off-by: Bukhtawar Khan <[email protected]> Signed-off-by: Gaurav Bafna <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Ashish Singh <[email protected]>

* Introduce TranslogFactory for Local/Remote Translog support (#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]> * [Remote Translog] Introduce remote translog with upload functionality (#5392) * Introduce remote translog with upload functionality Signed-off-by: Gaurav Bafna <[email protected]> Co-authored-by: Bukhtawar Khan <[email protected]> * Enable creation of indices using Remote Translog (#5638) * Enable creation of indices using Remote Translog behind a setting and feature flag Signed-off-by: Gaurav Bafna <[email protected]> * [Remote Translog] Add support for downloading files from remote translog (#5649) * Add support to download translog from remote store during recovery Signed-off-by: Sachin Kale <[email protected]> * Integrate remote translog download on failover (#5699) * Integrate remote translog download on failover Signed-off-by: Ashish Singh <[email protected]> Signed-off-by: Bukhtawar Khan <[email protected]> Signed-off-by: Gaurav Bafna <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Ashish Singh <[email protected]> Signed-off-by: Bukhtawar Khan <[email protected]> Signed-off-by: Gaurav Bafna <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Ashish Singh <[email protected]> Co-authored-by: Gaurav Bafna <[email protected]>

…hanges (#5757) * Introduce TranslogFactory for Local/Remote Translog support (#4172) * Introduce TranslogFactory for Local/Remote Translog support Signed-off-by: Bukhtawar Khan <[email protected]> * [Remote Translog] Introduce remote translog with upload functionality (#5392) * Introduce remote translog with upload functionality Signed-off-by: Gaurav Bafna <[email protected]> Co-authored-by: Bukhtawar Khan <[email protected]> * Enable creation of indices using Remote Translog (#5638) * Enable creation of indices using Remote Translog behind a setting and feature flag Signed-off-by: Gaurav Bafna <[email protected]> * [Remote Translog] Add support for downloading files from remote translog (#5649) * Add support to download translog from remote store during recovery Signed-off-by: Sachin Kale <[email protected]> * Integrate remote translog download on failover (#5699) * Integrate remote translog download on failover Signed-off-by: Ashish Singh <[email protected]> Signed-off-by: Bukhtawar Khan <[email protected]> Signed-off-by: Gaurav Bafna <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Ashish Singh <[email protected]>

Introduce TranslogFactory for Local/Remote Translog support

4c0bfc4

Signed-off-by: Bukhtawar Khan <[email protected]>

Bukhtawar requested review from a team and reta as code owners August 9, 2022 08:25

Minor fix up

1e022c6

Signed-off-by: Bukhtawar Khan <[email protected]>

Added missing java docs

8a56076

Signed-off-by: Bukhtawar Khan <[email protected]>

Bukhtawar requested review from mch2, nknize and andrross August 10, 2022 07:53

sachinpkale reviewed Aug 12, 2022

View reviewed changes

sachinpkale approved these changes Aug 13, 2022

View reviewed changes

ashking94 approved these changes Aug 16, 2022

View reviewed changes

mch2 approved these changes Aug 16, 2022

View reviewed changes

Bukhtawar merged commit 551f7c3 into opensearch-project:main Aug 16, 2022

gbbafna added the backport 2.x Backport to 2.x branch label Jan 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce TranslogFactory for Local/Remote Translog support #4172

Introduce TranslogFactory for Local/Remote Translog support #4172

Bukhtawar commented Aug 9, 2022 •

edited

Loading

github-actions bot commented Aug 9, 2022

github-actions bot commented Aug 9, 2022

github-actions bot commented Aug 9, 2022

codecov-commenter commented Aug 9, 2022

sachinpkale commented Aug 12, 2022

sachinpkale Aug 12, 2022

Bukhtawar Aug 12, 2022 •

edited

Loading

sachinpkale Aug 12, 2022

Bukhtawar Aug 12, 2022

ashking94 left a comment

ashking94 commented Aug 16, 2022

mch2 left a comment

opensearch-trigger-bot bot commented Jan 9, 2023

Introduce TranslogFactory for Local/Remote Translog support #4172

Introduce TranslogFactory for Local/Remote Translog support #4172

Conversation

Bukhtawar commented Aug 9, 2022 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Aug 9, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 9, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 9, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Aug 9, 2022

Codecov Report

sachinpkale commented Aug 12, 2022

sachinpkale Aug 12, 2022

Choose a reason for hiding this comment

Bukhtawar Aug 12, 2022 • edited Loading

Choose a reason for hiding this comment

sachinpkale Aug 12, 2022

Choose a reason for hiding this comment

Bukhtawar Aug 12, 2022

Choose a reason for hiding this comment

ashking94 left a comment

Choose a reason for hiding this comment

ashking94 commented Aug 16, 2022

mch2 left a comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jan 9, 2023

Bukhtawar commented Aug 9, 2022 •

edited

Loading

Bukhtawar Aug 12, 2022 •

edited

Loading