From a24d71c9e720b4263aedd0b11c1838d1fb7de18b Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Thu, 22 Aug 2024 17:38:56 +0530 Subject: [PATCH 1/3] Populate RecoveryState details for shallow snapshot restore Signed-off-by: Lakshya Taragi --- .../remotestore/RemoteRestoreSnapshotIT.java | 20 +++++++++++++++++++ .../opensearch/index/shard/IndexShard.java | 19 +++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index dc0654c623137..706acc832b80f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -13,6 +13,7 @@ import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; +import org.opensearch.action.admin.indices.recovery.RecoveryResponse; import org.opensearch.action.delete.DeleteResponse; import org.opensearch.action.support.PlainActionFuture; import org.opensearch.client.Client; @@ -34,6 +35,7 @@ import org.opensearch.index.shard.IndexShard; import org.opensearch.indices.IndicesService; import org.opensearch.indices.RemoteStoreSettings; +import org.opensearch.indices.recovery.RecoveryState; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.repositories.RepositoriesService; import org.opensearch.repositories.Repository; @@ -589,6 +591,24 @@ public void testRestoreShallowSnapshotRepository() throws ExecutionException, In ensureGreen(restoredIndexName1); assertDocsPresentInIndex(client, restoredIndexName1, numDocsInIndex1); + // ensure recovery details are non-zero + RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(restoredIndexName1).execute().actionGet(); + for (Map.Entry> entry : recoveryResponse.shardRecoveryStates().entrySet()) { + for (RecoveryState recoveryState : entry.getValue()) { + // ensure populated file details + assertThat(recoveryState.getIndex().totalFileCount(), greaterThan(0)); + assertThat(recoveryState.getIndex().totalRecoverFiles(), greaterThan(0)); + assertThat(recoveryState.getIndex().recoveredFileCount(), greaterThan(0)); + assertThat(recoveryState.getIndex().recoveredFilesPercent(), greaterThan(0f)); + + // ensure populated bytes details + assertThat(recoveryState.getIndex().recoveredBytes(), greaterThan(0L)); + assertThat(recoveryState.getIndex().totalBytes(), greaterThan(0L)); + assertThat(recoveryState.getIndex().totalRecoverBytes(), greaterThan(0L)); + assertThat(recoveryState.getIndex().recoveredBytesPercent(), greaterThan(0f)); + } + } + // indexing some new docs and validating indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2); ensureGreen(restoredIndexName1); diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 82b68b32f3bf8..e65d1b0754dc5 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5117,10 +5117,23 @@ public void syncSegmentsFromGivenRemoteSegmentStore( } Map uploadedSegments = sourceRemoteDirectory .getSegmentsUploadedToRemoteStore(); - final Directory storeDirectory = store.directory(); - store.incRef(); - try { + final Directory storeDirectory; + if (recoveryState.getStage() == RecoveryState.Stage.INDEX) { + storeDirectory = new StoreRecovery.StatsDirectoryWrapper(store.directory(), recoveryState.getIndex()); + for (String file : uploadedSegments.keySet()) { + long checksum = Long.parseLong(uploadedSegments.get(file).getChecksum()); + if (overrideLocal || localDirectoryContains(storeDirectory, file, checksum) == false) { + recoveryState.getIndex().addFileDetail(file, uploadedSegments.get(file).getLength(), false); + } else { + recoveryState.getIndex().addFileDetail(file, uploadedSegments.get(file).getLength(), true); + } + } + } else { + storeDirectory = store.directory(); + } + store.incRef(); + String segmentsNFile = copySegmentFiles( storeDirectory, sourceRemoteDirectory, From 4d8e4cfcde805eb6425989a6fe850aa84634138a Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Thu, 22 Aug 2024 17:38:56 +0530 Subject: [PATCH 2/3] Move incRef and add more assertions Signed-off-by: Lakshya Taragi --- .../remotestore/RemoteRestoreSnapshotIT.java | 46 +++++++++++++------ .../opensearch/index/shard/IndexShard.java | 2 +- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 706acc832b80f..8be36df898ee0 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -20,6 +20,7 @@ import org.opensearch.client.Requests; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.routing.RecoverySource; import org.opensearch.common.Nullable; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.io.PathUtils; @@ -75,6 +76,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class RemoteRestoreSnapshotIT extends AbstractSnapshotIntegTestCase { @@ -593,21 +596,34 @@ public void testRestoreShallowSnapshotRepository() throws ExecutionException, In // ensure recovery details are non-zero RecoveryResponse recoveryResponse = client().admin().indices().prepareRecoveries(restoredIndexName1).execute().actionGet(); - for (Map.Entry> entry : recoveryResponse.shardRecoveryStates().entrySet()) { - for (RecoveryState recoveryState : entry.getValue()) { - // ensure populated file details - assertThat(recoveryState.getIndex().totalFileCount(), greaterThan(0)); - assertThat(recoveryState.getIndex().totalRecoverFiles(), greaterThan(0)); - assertThat(recoveryState.getIndex().recoveredFileCount(), greaterThan(0)); - assertThat(recoveryState.getIndex().recoveredFilesPercent(), greaterThan(0f)); - - // ensure populated bytes details - assertThat(recoveryState.getIndex().recoveredBytes(), greaterThan(0L)); - assertThat(recoveryState.getIndex().totalBytes(), greaterThan(0L)); - assertThat(recoveryState.getIndex().totalRecoverBytes(), greaterThan(0L)); - assertThat(recoveryState.getIndex().recoveredBytesPercent(), greaterThan(0f)); - } - } + assertEquals(1, recoveryResponse.getTotalShards()); + assertEquals(1, recoveryResponse.getSuccessfulShards()); + assertEquals(0, recoveryResponse.getFailedShards()); + assertEquals(1, recoveryResponse.shardRecoveryStates().size()); + assertTrue(recoveryResponse.shardRecoveryStates().containsKey(restoredIndexName1)); + assertEquals(1, recoveryResponse.shardRecoveryStates().get(restoredIndexName1).size()); + + RecoveryState recoveryState = recoveryResponse.shardRecoveryStates().get(restoredIndexName1).get(0); + assertEquals(RecoveryState.Stage.DONE, recoveryState.getStage()); + assertEquals(0, recoveryState.getShardId().getId()); + assertTrue(recoveryState.getPrimary()); + assertEquals(RecoverySource.Type.SNAPSHOT, recoveryState.getRecoverySource().getType()); + assertThat(recoveryState.getIndex().time(), greaterThanOrEqualTo(0L)); + + // ensure populated file details + assertTrue(recoveryState.getIndex().totalFileCount() > 0); + assertTrue(recoveryState.getIndex().totalRecoverFiles() > 0); + assertTrue(recoveryState.getIndex().recoveredFileCount() > 0); + assertThat(recoveryState.getIndex().recoveredFilesPercent(), greaterThanOrEqualTo(0.0f)); + assertThat(recoveryState.getIndex().recoveredFilesPercent(), lessThanOrEqualTo(100.0f)); + assertFalse(recoveryState.getIndex().fileDetails().isEmpty()); + + // ensure populated bytes details + assertTrue(recoveryState.getIndex().recoveredBytes() > 0L); + assertTrue(recoveryState.getIndex().totalBytes() > 0L); + assertTrue(recoveryState.getIndex().totalRecoverBytes() > 0L); + assertThat(recoveryState.getIndex().recoveredBytesPercent(), greaterThanOrEqualTo(0.0f)); + assertThat(recoveryState.getIndex().recoveredBytesPercent(), lessThanOrEqualTo(100.0f)); // indexing some new docs and validating indexDocuments(client, restoredIndexName1, numDocsInIndex1, numDocsInIndex1 + 2); diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index e65d1b0754dc5..4b18df7a7829d 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -5117,6 +5117,7 @@ public void syncSegmentsFromGivenRemoteSegmentStore( } Map uploadedSegments = sourceRemoteDirectory .getSegmentsUploadedToRemoteStore(); + store.incRef(); try { final Directory storeDirectory; if (recoveryState.getStage() == RecoveryState.Stage.INDEX) { @@ -5132,7 +5133,6 @@ public void syncSegmentsFromGivenRemoteSegmentStore( } else { storeDirectory = store.directory(); } - store.incRef(); String segmentsNFile = copySegmentFiles( storeDirectory, From 668dfec3ed87cdd838d961912e2a641ae3cb9b79 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Tue, 27 Aug 2024 11:27:57 +0530 Subject: [PATCH 3/3] Fix IT - testSyncSegmentsFromGivenRemoteSegmentStore Signed-off-by: Lakshya Taragi --- .../org/opensearch/remotestore/RemoteRestoreSnapshotIT.java | 1 - .../test/java/org/opensearch/index/shard/IndexShardTests.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java index 8be36df898ee0..42e44bd3f37c3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java @@ -76,7 +76,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) diff --git a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java index 3188de13bb00b..377e4e99e9964 100644 --- a/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/opensearch/index/shard/IndexShardTests.java @@ -2896,9 +2896,9 @@ public void testSyncSegmentsFromGivenRemoteSegmentStore() throws IOException { RecoverySource.ExistingStoreRecoverySource.INSTANCE ); routing = ShardRoutingHelper.newWithRestoreSource(routing, new RecoverySource.EmptyStoreRecoverySource()); - target = reinitShard(target, routing); - + DiscoveryNode localNode = new DiscoveryNode("foo", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); + target.markAsRecovering("from snapshot", new RecoveryState(routing, localNode, null)); target.syncSegmentsFromGivenRemoteSegmentStore(false, tempRemoteSegmentDirectory, primaryTerm, commitGeneration); RemoteSegmentStoreDirectory remoteStoreDirectory = ((RemoteSegmentStoreDirectory) ((FilterDirectory) ((FilterDirectory) target .remoteStore()