Skip to content

Commit dd5a87a

Browse files
sachinpkalesachinpkale
authored
Bugfix in snapshot V2 restore flow (#16332)
Signed-off-by: Sachin Kale <sachinpkale@gmail.com>
1 parent d404359 commit dd5a87a

File tree

5 files changed

+109
-7
lines changed

5 files changed

+109
-7
lines changed

‎server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,8 @@ public void testIndexDeletionNoPinnedTimestamps() throws Exception {
421421
client().admin().indices().prepareDelete(INDEX_NAME).get();
422422

423423
assertBusy(() -> {
424-
assertEquals(0, Files.list(translogMetadataPath).collect(Collectors.toList()).size());
425-
assertEquals(0, Files.list(translogDataPath).collect(Collectors.toList()).size());
424+
assertEquals(1, Files.list(translogMetadataPath).collect(Collectors.toList()).size());
425+
assertEquals(4, Files.list(translogDataPath).collect(Collectors.toList()).size());
426426
});
427427
}
428428

@@ -490,7 +490,7 @@ public void testIndexDeletionWithPinnedTimestamps() throws Exception {
490490

491491
assertBusy(() -> {
492492
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
493-
assertEquals(1, metadataFiles.size());
493+
assertEquals(2, metadataFiles.size());
494494

495495
verifyTranslogDataFileCount(metadataFiles, translogDataPath);
496496
});

‎server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,26 @@
2727
import org.opensearch.common.blobstore.BlobPath;
2828
import org.opensearch.common.io.PathUtils;
2929
import org.opensearch.common.settings.Settings;
30+
import org.opensearch.common.unit.TimeValue;
3031
import org.opensearch.common.util.io.IOUtils;
3132
import org.opensearch.core.index.Index;
3233
import org.opensearch.core.rest.RestStatus;
3334
import org.opensearch.index.IndexService;
3435
import org.opensearch.index.IndexSettings;
36+
import org.opensearch.index.mapper.MapperService;
3537
import org.opensearch.index.remote.RemoteStoreEnums;
3638
import org.opensearch.index.shard.IndexShard;
3739
import org.opensearch.indices.IndicesService;
3840
import org.opensearch.indices.RemoteStoreSettings;
3941
import org.opensearch.indices.recovery.RecoveryState;
4042
import org.opensearch.indices.replication.common.ReplicationType;
43+
import org.opensearch.node.remotestore.RemoteStorePinnedTimestampService;
4144
import org.opensearch.repositories.blobstore.BlobStoreRepository;
4245
import org.opensearch.snapshots.AbstractSnapshotIntegTestCase;
4346
import org.opensearch.snapshots.SnapshotInfo;
4447
import org.opensearch.snapshots.SnapshotRestoreException;
4548
import org.opensearch.snapshots.SnapshotState;
49+
import org.opensearch.test.BackgroundIndexer;
4650
import org.opensearch.test.InternalTestCluster;
4751
import org.opensearch.test.OpenSearchIntegTestCase;
4852
import org.junit.After;
@@ -53,15 +57,18 @@
5357
import java.nio.file.Path;
5458
import java.util.ArrayList;
5559
import java.util.Arrays;
60+
import java.util.HashMap;
5661
import java.util.List;
5762
import java.util.Map;
5863
import java.util.Objects;
5964
import java.util.Optional;
6065
import java.util.concurrent.ExecutionException;
66+
import java.util.concurrent.TimeUnit;
6167
import java.util.stream.Collectors;
6268
import java.util.stream.Stream;
6369

6470
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
71+
import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
6572
import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS;
6673
import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG;
6774
import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
@@ -885,4 +892,88 @@ public void testRestoreOperationsUsingDifferentRepos() throws Exception {
885892
ensureGreen(indexName1);
886893
assertDocsPresentInIndex(client, indexName1, 3 * numDocsInIndex1);
887894
}
895+
896+
public void testContinuousIndexing() throws Exception {
897+
internalCluster().startClusterManagerOnlyNode();
898+
internalCluster().startDataOnlyNode();
899+
String index = "test-index";
900+
String snapshotRepo = "test-restore-snapshot-repo";
901+
String baseSnapshotName = "snapshot_";
902+
Path absolutePath1 = randomRepoPath().toAbsolutePath();
903+
logger.info("Snapshot Path [{}]", absolutePath1);
904+
905+
createRepository(snapshotRepo, "fs", getRepositorySettings(absolutePath1, true));
906+
907+
Client client = client();
908+
Settings indexSettings = Settings.builder()
909+
.put(super.indexSettings())
910+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
911+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
912+
.build();
913+
914+
createIndex(index, indexSettings);
915+
ensureGreen(index);
916+
917+
RemoteStorePinnedTimestampService remoteStorePinnedTimestampService = internalCluster().getInstance(
918+
RemoteStorePinnedTimestampService.class,
919+
primaryNodeName(index)
920+
);
921+
922+
remoteStorePinnedTimestampService.rescheduleAsyncUpdatePinnedTimestampTask(TimeValue.timeValueSeconds(randomIntBetween(1, 5)));
923+
RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.timeValueSeconds(randomIntBetween(1, 5)));
924+
925+
long totalDocs = 0;
926+
Map<String, Long> snapshots = new HashMap<>();
927+
int numDocs = randomIntBetween(200, 300);
928+
totalDocs += numDocs;
929+
try (BackgroundIndexer indexer = new BackgroundIndexer(index, MapperService.SINGLE_MAPPING_NAME, client(), numDocs)) {
930+
int numberOfSnapshots = 5;
931+
for (int i = 0; i < numberOfSnapshots; i++) {
932+
logger.info("--> waiting for {} docs to be indexed ...", numDocs);
933+
long finalTotalDocs1 = totalDocs;
934+
assertBusy(() -> assertEquals(finalTotalDocs1, indexer.totalIndexedDocs()), 120, TimeUnit.SECONDS);
935+
logger.info("--> {} total docs indexed", totalDocs);
936+
String snapshotName = baseSnapshotName + i;
937+
createSnapshot(snapshotRepo, snapshotName, new ArrayList<>());
938+
snapshots.put(snapshotName, totalDocs);
939+
if (i < numberOfSnapshots - 1) {
940+
numDocs = randomIntBetween(200, 300);
941+
indexer.continueIndexing(numDocs);
942+
totalDocs += numDocs;
943+
}
944+
}
945+
}
946+
947+
logger.info("Snapshots Status: " + snapshots);
948+
949+
for (String snapshot : snapshots.keySet()) {
950+
logger.info("Restoring snapshot: {}", snapshot);
951+
assertAcked(client().admin().indices().delete(new DeleteIndexRequest(index)).get());
952+
953+
RestoreSnapshotResponse restoreSnapshotResponse1 = client.admin()
954+
.cluster()
955+
.prepareRestoreSnapshot(snapshotRepo, snapshot)
956+
.setWaitForCompletion(true)
957+
.setIndices()
958+
.get();
959+
960+
assertEquals(RestStatus.OK, restoreSnapshotResponse1.status());
961+
962+
// Verify restored index's stats
963+
ensureGreen(TimeValue.timeValueSeconds(60), index);
964+
long finalTotalDocs = totalDocs;
965+
assertBusy(() -> {
966+
Long hits = client().prepareSearch(index)
967+
.setQuery(matchAllQuery())
968+
.setSize((int) finalTotalDocs)
969+
.storedFields()
970+
.execute()
971+
.actionGet()
972+
.getHits()
973+
.getTotalHits().value;
974+
975+
assertEquals(snapshots.get(snapshot), hits);
976+
});
977+
}
978+
}
888979
}

‎server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.opensearch.index.store.lockmanager.RemoteStoreMetadataLockManager;
4040
import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadata;
4141
import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadataHandler;
42+
import org.opensearch.indices.RemoteStoreSettings;
4243
import org.opensearch.indices.replication.checkpoint.ReplicationCheckpoint;
4344
import org.opensearch.node.remotestore.RemoteStorePinnedTimestampService;
4445
import org.opensearch.threadpool.ThreadPool;
@@ -891,6 +892,16 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException
891892
lastSuccessfulFetchOfPinnedTimestamps
892893
);
893894

895+
if (metadataFilesEligibleToDelete.isEmpty()) {
896+
logger.debug("No metadata files are eligible to be deleted based on lastNMetadataFilesToKeep and age");
897+
return;
898+
}
899+
900+
// If pinned timestamps are enabled, make sure to not delete last metadata file.
901+
if (RemoteStoreSettings.isPinnedTimestampsEnabled()) {
902+
metadataFilesEligibleToDelete.remove(sortedMetadataFileList.get(0));
903+
}
904+
894905
List<String> metadataFilesToBeDeleted = metadataFilesEligibleToDelete.stream()
895906
.filter(metadataFile -> allLockFiles.contains(metadataFile) == false)
896907
.collect(Collectors.toList());
@@ -905,7 +916,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException
905916
Set<String> activeSegmentRemoteFilenames = new HashSet<>();
906917

907918
final Set<String> metadataFilesToFilterActiveSegments = getMetadataFilesToFilterActiveSegments(
908-
lastNMetadataFilesToKeep,
919+
sortedMetadataFileList.indexOf(metadataFilesEligibleToDelete.get(0)),
909920
sortedMetadataFileList,
910921
allLockFiles
911922
);

‎server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public void onResponse(List<BlobMetadata> blobMetadata) {
189189
List<String> metadataFilesToBeDeleted = getMetadataFilesToBeDeleted(metadataFiles, indexDeleted);
190190

191191
// If index is not deleted, make sure to keep latest metadata file
192-
if (indexDeleted == false) {
192+
if (indexDeleted == false || RemoteStoreSettings.isPinnedTimestampsEnabled()) {
193193
metadataFilesToBeDeleted.remove(metadataFiles.get(0));
194194
}
195195

‎server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,9 @@ public void testIndexDeletionWithNoPinnedTimestampNoRecentMdFiles() throws Excep
286286
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
287287

288288
assertBusy(() -> {
289-
assertEquals(0, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
289+
assertEquals(1, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
290290
assertEquals(
291-
0,
291+
12,
292292
blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
293293
);
294294
});

0 commit comments

Comments
 (0)