[Remote Store] Add remote store restore API implementation #3642

sachinpkale · 2022-06-21T18:53:15Z

Signed-off-by: Sachin Kale kalsac@amazon.com

Description

This change adds implementation for the remote store restore API.
Currently, only segment restore is supported. Once remote translog support is added, we need to modify implementation of this API to restore data from remote translog as well.
API endpoint is added as part of this PR: [Remote Store] Add rest endpoint for remote store restore #3576
This will be followed by integ tests PR which will cover end-to-end flow of segment upload and restore.

Issues Resolved

[Remote Store] API support to restore data from remote store #3145

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.

sachinpkale · 2022-06-21T18:54:35Z

.../java/org/opensearch/action/admin/cluster/remotestore/restore/RestoreRemoteStoreRequest.java

@@ -0,0 +1,211 @@
+/*


This file is added so that corresponding code in RestoreService is compiled without any errors. This file is same as defined here: #3576

sachinpkale · 2022-06-21T18:55:38Z

.../org/opensearch/action/admin/cluster/remotestore/restore/RestoreRemoteStoreRequestTests.java

@@ -0,0 +1,103 @@
+/*


The RestoreRemoteStoreRequest file is added so that corresponding code in RestoreService is compiled without any errors. This file is same as defined here: #3576

opensearch-ci-bot · 2022-06-21T19:31:13Z

✅ Gradle Check success f51871fa10b061f84cf18518d97ec7fb8b2eb134
Log 6189

Reports 6189

opensearch-ci-bot · 2022-06-26T10:24:09Z

✅ Gradle Check success 405689c53207cd7777a41b93d45923ba4c63df84
Log 6344

Reports 6344

Bukhtawar · 2022-06-27T16:45:32Z

server/src/main/java/org/opensearch/index/shard/StoreRecovery.java

+    void recoverFromRemoteStore(final IndexShard indexShard, ActionListener<Boolean> listener) {
+        if (canRecover(indexShard)) {
+            RecoverySource.Type recoveryType = indexShard.recoveryState().getRecoverySource().getType();
+            assert recoveryType == RecoverySource.Type.REMOTE_STORE : "expected remote store recovery type but was: " + recoveryType;
+            ActionListener.completeWith(recoveryListener(indexShard, listener), () -> {
+                logger.debug("starting recovery from remote store ...");
+                recoverFromRemoteStore(indexShard);
+                return true;
+            });
+        } else {
+            listener.onResponse(false);
+        }
+    }
+


Can we extend StoreRecovery to RemoteStoreRecovery and override recoverFromStore

StoreRecovery currently contains: recoverFromStore, recoverFromRepository and recoverFromLocalShards where the source of recovery is different for each of these methods. That is why I added one more method where recovery source is remote store.

Bukhtawar · 2022-06-27T16:59:09Z

server/src/main/java/org/opensearch/snapshots/RestoreService.java

+     * @param request  restore request
+     * @param listener restore listener
+     */
+    public void restoreFromRemoteStore(RestoreRemoteStoreRequest request, final ActionListener<RestoreCompletionResponse> listener) {


The method exists in the snapshot package, we should extend it and abstract it out

Yes,restoreSnapshot method is in the same class. I earlier thought of extending it but RestoreSnapshotRequest is different than RestoreRemoteStoreRequest and even if we create one method, it will anyway have conditional check to redirect the flow to snapshot restore vs remote store restore.

Bukhtawar · 2022-06-27T17:00:24Z

server/src/main/java/org/opensearch/snapshots/RestoreService.java

+            public ClusterState execute(ClusterState currentState) {
+                // Updating cluster state
+                ClusterState.Builder builder = ClusterState.builder(currentState);
+                Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata());
+                ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
+                RoutingTable.Builder rtBuilder = RoutingTable.builder(currentState.routingTable());
+
+                List<String> indicesToBeRestored = new ArrayList<>();


How do we ensure concurrent snapshot recovery isn't happening? We need to sync on the shard state

I missed adding a check in IndexRountingTable.initializeAsRemoteStoreRestore() method. Will add.

Thanks for pointing it out. Made change to handle this. #3642 (comment)

Wait! Forgot to commit index re-opening logic. Adding now.

Added code to open the closed index as part of restore flow.

Signed-off-by: Sachin Kale <kalsac@amazon.com>

github-actions · 2022-06-30T07:04:07Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/96/
CommitID: 405689c53207cd7777a41b93d45923ba4c63df84

server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java

github-actions · 2022-06-30T07:26:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/97/
CommitID: 374471d61ea1bd04141d644405b531648e7fee1c

github-actions · 2022-06-30T11:23:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/99/
CommitID: 527a653441afaa0de2fa5be70de477a4865b55bc

sachinpkale · 2022-06-30T12:43:31Z

Test failing: DiskThresholdDeciderIT.testHighWatermarkNotExceeded. This is a flaky test as mentioned here: https://build.ci.opensearch.org/job/gradle-check/99/console

Signed-off-by: Sachin Kale <kalsac@amazon.com>

github-actions · 2022-06-30T13:09:21Z

Gradle Check (Jenkins) Run Completed with:

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

sachinpkale · 2022-06-30T13:16:50Z

server/src/main/java/org/opensearch/snapshots/RestoreService.java

+                        logger.warn("Remote store restore is not supported for non-existent index. Skipping: {}", index);
+                        continue;
+                    }
+                    if (currentIndexMetadata.getState() != IndexMetadata.State.CLOSE) {


We allow restore only for closed indices (similar to what snapshot does). Once index is set for recovery, index state is changed to open. This check enables us to avoid running two restores (remote/remote or remote/snapshot) at the same time.

github-actions · 2022-07-01T08:31:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/114/
CommitID: b0fb31ee7e5c9beedbab6cfde4afdf49de42ede7

github-actions · 2022-07-01T13:41:54Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/117/
CommitID: e1bfcba5e37c4e8d984d7ed21e6775f79987c352

Signed-off-by: Sachin Kale <kalsac@amazon.com>

github-actions · 2022-07-01T14:28:35Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/119/
CommitID: 5f7baf9

Bukhtawar · 2022-07-04T18:42:27Z

.../java/org/opensearch/action/admin/cluster/remotestore/restore/RestoreRemoteStoreRequest.java

+    private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen();
+    private boolean waitForCompletion;
+
+    public RestoreRemoteStoreRequest() {}


Shouldn't indices be in constructor args(mandatory)

This is handled by overriding ActionRequet's validate() method.

I meant just from a stand alone class perspective

Got it. It is a bit tricky as it depends on how RestRestoreRemoteStoreAction initializes the RestoreRemoteStoreRequest.

https://github.com/opensearch-project/OpenSearch/pull/3576/files#diff-03a57182daff0959e24e2fccf7fe28e7dedcd711ef76758e24df834dc0e884b7R41

As indices is a part of post body, fetching it from RestRequest requires parsing the body content. This code of parsing is added as part of source method of the same class.

I just followed the conventions used by other Request classes but open to suggestions.

Bukhtawar · 2022-07-04T18:43:17Z

.../java/org/opensearch/action/admin/cluster/remotestore/restore/RestoreRemoteStoreRequest.java

+        super.writeTo(out);
+        out.writeStringArray(indices);
+        indicesOptions.writeIndicesOptions(out);
+        out.writeBoolean(waitForCompletion);


writeOptionalBoolean?

Bukhtawar · 2022-07-04T18:44:27Z

.../java/org/opensearch/action/admin/cluster/remotestore/restore/RestoreRemoteStoreRequest.java

+            if (name.equals("indices")) {
+                if (entry.getValue() instanceof String) {
+                    indices(Strings.splitStringByCommaToArray((String) entry.getValue()));
+                } else if (entry.getValue() instanceof ArrayList) {


Can it be ArrayList?

As indices is part of post body, it is better to get it as map from the XContentParser. This will help in easily adding new fields to the post body.

Signed-off-by: Sachin Kale <kalsac@amazon.com>

github-actions · 2022-07-05T04:44:18Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/132/
CommitID: fad667a94bfeedb0cd47b93d4c4a5c7a89cbb84c

github-actions · 2022-07-05T05:00:33Z

Gradle Check (Jenkins) Run Completed with:

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

Bukhtawar

Thanks @sachinpkale
Is it possible to restore just one shard rather than the complete index from remote store?

sachinpkale · 2022-07-05T10:57:49Z

Thanks @sachinpkale Is it possible to restore just one shard rather than the complete index from remote store?

@Bukhtawar With the current flow, it makes an entry to the IndexRoutingTable for the given index. Need to explore if we can change IndexShardRoutingTable instead and enable restore for a particular shard.

sachinpkale · 2022-07-05T11:00:33Z

Thanks @sachinpkale Is it possible to restore just one shard rather than the complete index from remote store?

@Bukhtawar With the current flow, it makes an entry to the IndexRoutingTable for the given index. Need to explore if we can change IndexShardRoutingTable instead and enable restore for a particular shard.

Created a tracking issue: #3768

…h-project#3642) * Add remote restore API implementation Signed-off-by: Sachin Kale <kalsac@amazon.com>

…4380) * [Remote Store] Upload segments to remote store post refresh (#3460) * Add RemoteDirectory interface to copy segment files to/from remote store Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com> * Add index level setting for remote store Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com> * Add RemoteDirectoryFactory and use RemoteDirectory instance in RefreshListener Co-authored-by: Sachin Kale <kalsac@amazon.com> Signed-off-by: Sachin Kale <kalsac@amazon.com> * Upload segment to remote store post refresh Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com> Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Inject remote store in IndexShard instead of RemoteStoreRefreshListener (#3703) * Inject remote store in IndexShard instead of RemoteStoreRefreshListener Signed-off-by: Sachin Kale <kalsac@amazon.com> * Pass supplier of RepositoriesService to RemoteDirectoryFactory Signed-off-by: Sachin Kale <kalsac@amazon.com> * Create isRemoteStoreEnabled function for IndexShard Signed-off-by: Sachin Kale <kalsac@amazon.com> * Explicitly close remoteStore on indexShard close Signed-off-by: Sachin Kale <kalsac@amazon.com> * Change RemoteDirectory.close to a no-op Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Add remote store restore API implementation (#3642) * Add remote restore API implementation Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Add support to add nested settings for remote store (#4060) * Add support to add nested settings for remote store Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Add rest endpoint for remote store restore (#3576) * Add rest endpoint for remote store restore Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Add validator that forces segment replication type before enabling remote store (#4175) * Add validator that forces segment replication type before enabling remote store Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Change remote_store setting validation message to make it more clear (#4199) * Change remote_store setting validation message to make it more clear Signed-off-by: Sachin Kale <kalsac@amazon.com> * [Remote Store] Add RemoteSegmentStoreDirectory to interact with remote segment store (#4020) * Add RemoteSegmentStoreDirectory to interact with remote segment store Signed-off-by: Sachin Kale <kalsac@amazon.com> * Use RemoteSegmentStoreDirectory instead of RemoteDirectory (#4240) * Use RemoteSegmentStoreDirectory instead of RemoteDirectory Signed-off-by: Sachin Kale <kalsac@amazon.com> Signed-off-by: Sachin Kale <kalsac@amazon.com> Co-authored-by: Sachin Kale <kalsac@amazon.com>

sachinpkale commented Jun 21, 2022

View reviewed changes

sachinpkale marked this pull request as ready for review June 22, 2022 03:35

sachinpkale requested review from a team and reta as code owners June 22, 2022 03:35

sachinpkale mentioned this pull request Jun 25, 2022

[Remote Store] Add rest endpoint for remote store restore #3576

Merged

5 tasks

sachinpkale force-pushed the feature/restore-api-impl branch from 57b52b6 to 405689c Compare June 26, 2022 09:51

Bukhtawar reviewed Jun 27, 2022

View reviewed changes

sachinpkale force-pushed the feature/restore-api-impl branch from 405689c to a9efcc8 Compare June 30, 2022 07:02

Sachin Kale added 2 commits June 30, 2022 12:33

Add remote restore API implementation

6560944

Signed-off-by: Sachin Kale <kalsac@amazon.com>

Add empty translog creation for remote store restore

d5ac191

Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale force-pushed the feature/restore-api-impl branch from a9efcc8 to 374471d Compare June 30, 2022 07:04

sachinpkale commented Jun 30, 2022

View reviewed changes

server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java Outdated Show resolved Hide resolved

sachinpkale force-pushed the feature/restore-api-impl branch from 374471d to 527a653 Compare June 30, 2022 10:58

Check for closed index and changes related to rebase

e5809d0

Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale force-pushed the feature/restore-api-impl branch from 527a653 to e5809d0 Compare June 30, 2022 12:44

sachinpkale commented Jun 30, 2022

View reviewed changes

sachinpkale requested a review from Bukhtawar June 30, 2022 13:17

sachinpkale force-pushed the feature/restore-api-impl branch from b0fb31e to e1bfcba Compare July 1, 2022 13:17

Open the index to be restored as part of restore flow

5f7baf9

Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale force-pushed the feature/restore-api-impl branch from e1bfcba to 5f7baf9 Compare July 1, 2022 14:02

Bukhtawar reviewed Jul 4, 2022

View reviewed changes

Address PR comments and simplify RestoreRemoteStoreRequest

e0d88ee

Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale force-pushed the feature/restore-api-impl branch from fad667a to e0d88ee Compare July 5, 2022 04:34

sachinpkale requested a review from Bukhtawar July 5, 2022 06:32

Bukhtawar approved these changes Jul 5, 2022

View reviewed changes

sachinpkale mentioned this pull request Jul 5, 2022

[Remote Store] Restore API to support shard level restore #3768

Open

Bukhtawar merged commit 27b58ab into opensearch-project:main Jul 5, 2022

sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 1, 2022

[Remote Store] Add remote store restore API implementation (opensearc…

14b84f9

…h-project#3642) * Add remote restore API implementation Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale mentioned this pull request Sep 1, 2022

[Backport 2.x] [Remote Store] Backport remote segment store changes #4380

Merged

6 tasks

sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022

[Remote Store] Add remote store restore API implementation (opensearc…

019d30b

…h-project#3642) * Add remote restore API implementation Signed-off-by: Sachin Kale <kalsac@amazon.com>

sachinpkale added a commit to sachinpkale/OpenSearch that referenced this pull request Sep 2, 2022

[Remote Store] Add remote store restore API implementation (opensearc…

be4e15b

…h-project#3642) * Add remote restore API implementation Signed-off-by: Sachin Kale <kalsac@amazon.com>

[Remote Store] Add remote store restore API implementation #3642

[Remote Store] Add remote store restore API implementation #3642

Conversation

sachinpkale commented Jun 21, 2022

Description

Issues Resolved

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-ci-bot commented Jun 21, 2022

opensearch-ci-bot commented Jun 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachinpkale Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 30, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 30, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 30, 2022

Gradle Check (Jenkins) Run Completed with:

sachinpkale commented Jun 30, 2022

github-actions bot commented Jun 30, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

github-actions bot commented Jul 1, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 1, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 1, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 5, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 5, 2022

Gradle Check (Jenkins) Run Completed with:

Bukhtawar left a comment

Choose a reason for hiding this comment

sachinpkale commented Jul 5, 2022

sachinpkale commented Jul 5, 2022

sachinpkale Jun 30, 2022 •

edited

Loading