hemantk-12 commented on code in PR #4617:
URL: https://github.com/apache/ozone/pull/4617#discussion_r1186429739
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -666,114 +687,60 @@ private void generateSnapshotDiffReport(final String
jobKey,
final BucketLayout bucketLayout = getBucketLayout(volume, bucket,
fromSnapshot.getMetadataManager());
- final Table<String, OmKeyInfo> fsKeyTable =
- fromSnapshot.getMetadataManager().getKeyTable(bucketLayout);
- final Table<String, OmKeyInfo> tsKeyTable =
- toSnapshot.getMetadataManager().getKeyTable(bucketLayout);
-
- boolean useFullDiff = ozoneManager.getConfiguration().getBoolean(
- OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF,
- OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF_DEFAULT);
- if (forceFullDiff) {
- useFullDiff = true;
- }
-
Map<String, String> tablePrefixes =
getTablePrefixes(toSnapshot.getMetadataManager(), volume, bucket);
- List<String> tablesToLookUp =
- Collections.singletonList(fsKeyTable.getName());
- Set<String> deltaFilesForKeyOrFileTable = getDeltaFiles(fromSnapshot,
- toSnapshot, tablesToLookUp, fsInfo, tsInfo, useFullDiff,
- tablePrefixes, path.toString());
-
- // Workaround to handle deletes if native rocksDb tool for reading
- // tombstone is not loaded.
- // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
- if (!isNativeRocksToolsLoaded) {
- deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
- fromSnapshot, tablesToLookUp));
- }
- try {
- addToObjectIdMap(fsKeyTable, tsKeyTable,
- Pair.of(isNativeRocksToolsLoaded, deltaFilesForKeyOrFileTable),
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
- tablePrefixes);
- } catch (NativeLibraryNotLoadedException e) {
- // Workaround to handle deletes if use of native rocksDb tool for
- // reading tombstone fails.
- // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
- deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
- fromSnapshot, tablesToLookUp));
- try {
- addToObjectIdMap(fsKeyTable, tsKeyTable,
- Pair.of(false, deltaFilesForKeyOrFileTable),
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
- tablePrefixes);
- } catch (NativeLibraryNotLoadedException ex) {
- // This code should be never executed.
- throw new IllegalStateException(ex);
- }
- }
+
+ boolean useFullDiff = snapshotForceFullDiff || forceFullDiff;
+
+ validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+ toSnapshotName);
+ Table<String, OmKeyInfo> fsKeyTable = fromSnapshot.getMetadataManager()
+ .getKeyTable(bucketLayout);
+ Table<String, OmKeyInfo> tsKeyTable = toSnapshot.getMetadataManager()
+ .getKeyTable(bucketLayout);
+
+ getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsKeyTable, tsKeyTable,
+ fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+ tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+ path.toString());
if (bucketLayout.isFileSystemOptimized()) {
- final Table<String, OmDirectoryInfo> fsDirTable =
+ validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+ toSnapshotName);
+
+ Table<String, OmDirectoryInfo> fsDirTable =
fromSnapshot.getMetadataManager().getDirectoryTable();
- final Table<String, OmDirectoryInfo> tsDirTable =
+ Table<String, OmDirectoryInfo> tsDirTable =
toSnapshot.getMetadataManager().getDirectoryTable();
- tablesToLookUp = Collections.singletonList(fsDirTable.getName());
- final Set<String> deltaFilesForDirTable =
- getDeltaFiles(fromSnapshot, toSnapshot, tablesToLookUp, fsInfo,
- tsInfo, useFullDiff, tablePrefixes, path.toString());
- if (!isNativeRocksToolsLoaded) {
- deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
- fromSnapshot, tablesToLookUp));
- }
- try {
- addToObjectIdMap(fsDirTable, tsDirTable,
- Pair.of(isNativeRocksToolsLoaded, deltaFilesForDirTable),
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot,
- objectIDsToCheckMap,
- tablePrefixes);
- } catch (NativeLibraryNotLoadedException e) {
- try {
- // Workaround to handle deletes if use of native rockstools for
- // reading tombstone fails.
- // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read
tombstone
- deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
- fromSnapshot, tablesToLookUp));
- addToObjectIdMap(fsDirTable, tsDirTable,
- Pair.of(false, deltaFilesForDirTable),
- objectIdToKeyNameMapForFromSnapshot,
- objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
- tablePrefixes);
- } catch (NativeLibraryNotLoadedException ex) {
- // This code should be never executed.
- throw new IllegalStateException(ex);
- }
- }
+
+ getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsDirTable, tsDirTable,
+ fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+ tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+ objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+ path.toString());
}
+ validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+ toSnapshotName);
long totalDiffEntries = generateDiffReport(jobId,
objectIDsToCheckMap,
objectIdToKeyNameMapForFromSnapshot,
objectIdToKeyNameMapForToSnapshot);
updateJobStatusToDone(jobKey, totalDiffEntries);
- } catch (IOException | RocksDBException exception) {
+ } catch (ExecutionException | IOException | RocksDBException exception) {
updateJobStatus(jobKey, IN_PROGRESS, FAILED);
- LOG.error("Caught checked exception during diff report generation for " +
+ LOG.info("Caught checked exception during diff report generation for " +
Review Comment:
My understanding was that checked exceptions (known exceptions) should be
logged at `INFO` level and handle properly. But then giving it more thoughts,
checked exceptions do not they are expected, especially in this case. It make
sense to keep log level `ERROR`, Hence reverted the log level.
--
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]
