swamirishi commented on code in PR #9268:
URL: https://github.com/apache/ozone/pull/9268#discussion_r2527268619
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -350,37 +361,34 @@ private void deleteDir(Path path) {
/**
* Convert from SnapshotInfo to DifferSnapshotInfo.
*/
- private DifferSnapshotInfo getDSIFromSI(SnapshotInfo snapshotInfo,
- OmSnapshot omSnapshot, final String volumeName, final String bucketName)
- throws IOException {
-
- final OMMetadataManager snapshotOMMM = omSnapshot.getMetadataManager();
- final String checkpointPath =
- snapshotOMMM.getStore().getDbLocation().getPath();
+ private static DifferSnapshotInfo getDSIFromSI(OMMetadataManager
activeOmMetadataManager,
+ SnapshotInfo snapshotInfo, OmSnapshotLocalData snapshotLocalData) throws
IOException {
final UUID snapshotId = snapshotInfo.getSnapshotId();
final long dbTxSequenceNumber = snapshotInfo.getDbTxSequenceNumber();
+ NavigableMap<Integer, List<SstFileInfo>> versionSstFiles =
snapshotLocalData.getVersionSstFileInfos()
+ .entrySet().stream().collect(toMap(Map.Entry::getKey, entry ->
entry.getValue().getSstFiles(),
+ (u, v) -> {
+ throw new IllegalStateException(String.format("Duplicate key %s",
u));
+ }, TreeMap::new));
+ if (versionSstFiles.isEmpty()) {
+ throw new IOException(String.format("No versions found corresponding to
%s", snapshotId));
+ }
return new DifferSnapshotInfo(
- checkpointPath,
- snapshotId,
- dbTxSequenceNumber,
- snapshotOMMM.getTableBucketPrefix(volumeName, bucketName),
- ((RDBStore)snapshotOMMM.getStore()).getDb().getManagedRocksDb());
+ version -> OmSnapshotManager.getSnapshotPath(activeOmMetadataManager,
snapshotId, version),
+ snapshotId, dbTxSequenceNumber, versionSstFiles);
}
@VisibleForTesting
- protected Set<String> getSSTFileListForSnapshot(OmSnapshot snapshot,
- Set<String> tablesToLookUp) {
- return RdbUtil.getSSTFilesForComparison(((RDBStore)snapshot
- .getMetadataManager().getStore()).getDb().getManagedRocksDb(),
- tablesToLookUp);
+ protected Set<SstFileInfo> getSSTFileListForSnapshot(OmSnapshot snapshot,
Set<String> tablesToLookUp) {
Review Comment:
done
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -863,53 +816,109 @@ public synchronized Optional<List<String>>
getSSTDiffListWithFullPath(DifferSnap
* must be non-null.
* @return A list of SST files without extension. e.g. ["000050", "000060"]
Review Comment:
done
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
Review Comment:
done
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -780,57 +803,31 @@ public void testGetSSTDiffListWithoutDB(String
description,
}
// Check same and different SST files result
- assertEquals(expectedSameSstFiles, actualSameSstFiles);
- assertEquals(expectedDiffSstFiles, actualDiffSstFiles);
- try (MockedStatic<RdbUtil> mockedHandler =
Mockito.mockStatic(RdbUtil.class, Mockito.CALLS_REAL_METHODS)) {
- RocksDB rocksDB = Mockito.mock(RocksDB.class);
- Mockito.when(rocksDB.getName()).thenReturn("dummy");
- Mockito.when(srcSnapshot.getRocksDB().get()).thenReturn(rocksDB);
- Mockito.when(destSnapshot.getRocksDB().get()).thenReturn(rocksDB);
- Mockito.when(srcSnapshot.getRocksDB().getLiveMetadataForSSTFiles())
- .thenAnswer(invocation ->
srcSnapshotSstFiles.stream().filter(metaDataMap::containsKey).map(file -> {
- LiveFileMetaData liveFileMetaData =
Mockito.mock(LiveFileMetaData.class);
- String[] metaData = metaDataMap.get(file);
- Mockito.when(liveFileMetaData.fileName()).thenReturn("/" + file +
SST_FILE_EXTENSION);
-
Mockito.when(liveFileMetaData.smallestKey()).thenReturn(metaData[0].getBytes(UTF_8));
-
Mockito.when(liveFileMetaData.largestKey()).thenReturn(metaData[1].getBytes(UTF_8));
-
Mockito.when(liveFileMetaData.columnFamilyName()).thenReturn(metaData[2].getBytes(UTF_8));
- return liveFileMetaData;
- }).collect(Collectors.toMap(liveFileMetaData ->
FilenameUtils.getBaseName(liveFileMetaData.fileName()),
- Function.identity())));
- Set<String> tablesToLookup;
- String dummyTable;
- if (srcSnapshot.getTablePrefixes() != null) {
- tablesToLookup = srcSnapshot.getTablePrefixes().getTableNames();
- dummyTable = tablesToLookup.stream().findAny().get();
+ assertEquals(expectedSameSstFiles, actualSameSstFiles.keySet());
+ assertEquals(expectedDiffSstFiles, actualDiffSstFiles.keySet());
+ when(srcSnapshot.getSstFiles(eq(0), eq(tablesToLookup)))
+ .thenAnswer(invocation -> srcSnapshotSstFiles.stream()
+ .map(file -> metaDataMap.getOrDefault(file, new SstFileInfo(file,
null, null, null)))
+ .collect(Collectors.toList()));
+ when(destSnapshot.getSstFiles(eq(0), eq(tablesToLookup)))
+ .thenAnswer(invocation -> destSnapshotSstFiles.stream()
+ .map(file -> metaDataMap.getOrDefault(file, new SstFileInfo(file,
null, null, null)))
+ .collect(Collectors.toList()));
+
+ try {
+ Assertions.assertEquals(Optional.ofNullable(expectedSSTDiffFiles)
+ .map(files ->
files.stream().sorted().collect(Collectors.toList())).orElse(null),
+ rocksDBCheckpointDiffer.getSSTDiffList(
+ new DifferSnapshotVersion(srcSnapshot, 0, tablesToLookup),
+ new DifferSnapshotVersion(destSnapshot, 0, tablesToLookup),
prefixInfo, tablesToLookup,
+ true)
+ .map(i ->
i.stream().map(SstFileInfo::getFileName).sorted().collect(Collectors.toList())).orElse(null));
+ } catch (RuntimeException rtEx) {
+ if (!expectingException) {
+ rtEx.printStackTrace();
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]