hemantk-12 commented on code in PR #5301:
URL: https://github.com/apache/ozone/pull/5301#discussion_r1342966162
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -338,24 +398,137 @@ private void processSnapshotDeepClean(int delCount)
if (delCount < keyLimitPerTask) {
// Deep clean is completed, we can update the SnapInfo.
deepCleanedSnapshots.add(currSnapInfo.getTableKey());
+ // exclusiveSizeList contains check is used to prevent
+ // case where there is no entry in deletedTable, this
+ // will throw NPE when we submit request.
+ if (previousSnapshot != null && exclusiveSizeMap
+ .containsKey(previousSnapshot.getTableKey())) {
+ completedExclusiveSizeSet.add(
+ previousSnapshot.getTableKey());
+ }
}
if (!keysToPurge.isEmpty()) {
processKeyDeletes(keysToPurge, currOmSnapshot.getKeyManager(),
keysToModify, currSnapInfo.getTableKey());
}
} finally {
- if (previousSnapshot != null) {
- rcPrevOmSnapshot.close();
- }
+ IOUtils.closeQuietly(rcPrevOmSnapshot, rcPrevToPrevOmSnapshot);
}
}
}
}
+
+ updateSnapshotExclusiveSize();
updateDeepCleanedSnapshots(deepCleanedSnapshots);
}
+ @SuppressWarnings("checkstyle:ParameterNumber")
+ private void calculateExclusiveSize(
+ SnapshotInfo previousSnapshot,
+ SnapshotInfo previousToPrevSnapshot,
+ OmKeyInfo keyInfo,
+ OmBucketInfo bucketInfo, long volumeId,
+ Table<String, String> snapRenamedTable,
+ Table<String, OmKeyInfo> previousKeyTable,
+ Table<String, String> prevRenamedTable,
+ Table<String, OmKeyInfo> previousToPrevKeyTable) throws IOException {
+ String prevSnapKey = previousSnapshot.getTableKey();
+ long exclusiveReplicatedSize =
+ exclusiveReplicatedSizeMap.getOrDefault(
+ prevSnapKey, 0L) + keyInfo.getReplicatedSize();
+ long exclusiveSize = exclusiveSizeMap.getOrDefault(
+ prevSnapKey, 0L) + keyInfo.getDataSize();
+
+ // If there is no previous to previous snapshot, then
+ // the previous snapshot is the first snapshot.
+ if (previousToPrevSnapshot == null) {
+ exclusiveSizeMap.put(prevSnapKey, exclusiveSize);
+ exclusiveReplicatedSizeMap.put(prevSnapKey,
+ exclusiveReplicatedSize);
+ } else {
+ OmKeyInfo keyInfoPrevSnapshot = getPreviousSnapshotKeyName(
+ keyInfo, bucketInfo, volumeId,
+ snapRenamedTable, previousKeyTable);
+ OmKeyInfo keyInfoPrevToPrevSnapshot = getPreviousSnapshotKeyName(
+ keyInfoPrevSnapshot, bucketInfo, volumeId,
+ prevRenamedTable, previousToPrevKeyTable);
+ // If the previous to previous snapshot doesn't
+ // have the key, then it is exclusive size for the
+ // previous snapshot.
+ if (keyInfoPrevToPrevSnapshot == null) {
+ exclusiveSizeMap.put(prevSnapKey, exclusiveSize);
+ exclusiveReplicatedSizeMap.put(prevSnapKey,
+ exclusiveReplicatedSize);
+ }
+ }
+ }
+
+ private OmKeyInfo getPreviousSnapshotKeyName(
+ OmKeyInfo keyInfo, OmBucketInfo bucketInfo, long volumeId,
+ Table<String, String> snapRenamedTable,
+ Table<String, OmKeyInfo> previousKeyTable) throws IOException {
+
+ if (keyInfo == null) {
+ return null;
+ }
+
+ String dbKeyPrevSnap;
+ if (bucketInfo.getBucketLayout().isFileSystemOptimized()) {
+ dbKeyPrevSnap = getOzoneManager().getMetadataManager().getOzonePathKey(
+ volumeId,
+ bucketInfo.getObjectID(),
+ keyInfo.getParentObjectID(),
+ keyInfo.getFileName());
+ } else {
+ dbKeyPrevSnap = getOzoneManager().getMetadataManager().getOzoneKey(
+ keyInfo.getVolumeName(),
+ keyInfo.getBucketName(),
+ keyInfo.getKeyName());
+ }
+
+ String dbRenameKey = getOzoneManager().getMetadataManager().getRenameKey(
+ keyInfo.getVolumeName(),
+ keyInfo.getBucketName(),
+ keyInfo.getObjectID());
+
+ String renamedKey = snapRenamedTable.getIfExist(dbRenameKey);
+ dbKeyPrevSnap = renamedKey != null ? renamedKey : dbKeyPrevSnap;
+
+ return previousKeyTable.get(dbKeyPrevSnap);
+ }
+
+ private void updateSnapshotExclusiveSize() {
+
+ if (completedExclusiveSizeSet.isEmpty()) {
+ return;
+ }
+
+ for (String dbKey: completedExclusiveSizeSet) {
+ SnapshotProperty snapshotProperty = SnapshotProperty.newBuilder()
+ .setSnapshotKey(dbKey)
+ .setExclusiveSize(exclusiveSizeMap.get(dbKey))
+ .setExclusiveReplicatedSize(
+ exclusiveReplicatedSizeMap.get(dbKey))
+ .build();
+ SetSnapshotPropertyRequest setSnapshotPropertyRequest =
+ SetSnapshotPropertyRequest.newBuilder()
+ .setSnapshotProperty(snapshotProperty)
+ .build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(Type.SetSnapshotProperty)
+ .setSetSnapshotPropertyRequest(setSnapshotPropertyRequest)
+ .setClientId(clientId.toString())
+ .build();
+ submitRequest(omRequest);
+ exclusiveSizeMap.remove(dbKey);
+ exclusiveReplicatedSizeMap.remove(dbKey);
+ }
+ completedExclusiveSizeSet.clear();
Review Comment:
nit: Should we remove entries from `completedExclusiveSizeSet` in the above
loop? In case, `setSnapshotProperty` request fails for one or more `dbKey`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -285,6 +306,25 @@ private void processSnapshotDeepClean(int delCount)
previousKeyTable = omPreviousSnapshot.getMetadataManager()
.getKeyTable(bucketInfo.getBucketLayout());
+ prevRenamedTable = omPreviousSnapshot
+ .getMetadataManager().getSnapshotRenamedTable();
+ }
+
+ Table<String, OmKeyInfo> previousToPrevKeyTable = null;
+ ReferenceCounted<IOmMetadataReader, SnapshotCache>
+ rcPrevToPrevOmSnapshot = null;
+ OmSnapshot omPreviousToPrevSnapshot = null;
Review Comment:
nit: make `omPreviousToPrevSnapshot` variable local to the `if` block.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -309,6 +349,26 @@ private void processSnapshotDeepClean(int delCount)
RepeatedOmKeyInfo newRepeatedOmKeyInfo =
new RepeatedOmKeyInfo();
for (OmKeyInfo keyInfo : repeatedOmKeyInfo.getOmKeyInfoList())
{
+ // To calculate Exclusive Size for current snapshot, Check
Review Comment:
nit: this could be comment or java doc comment for `calculateExclusiveSize`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -338,24 +398,137 @@ private void processSnapshotDeepClean(int delCount)
if (delCount < keyLimitPerTask) {
// Deep clean is completed, we can update the SnapInfo.
deepCleanedSnapshots.add(currSnapInfo.getTableKey());
+ // exclusiveSizeList contains check is used to prevent
+ // case where there is no entry in deletedTable, this
+ // will throw NPE when we submit request.
+ if (previousSnapshot != null && exclusiveSizeMap
+ .containsKey(previousSnapshot.getTableKey())) {
+ completedExclusiveSizeSet.add(
+ previousSnapshot.getTableKey());
+ }
}
if (!keysToPurge.isEmpty()) {
processKeyDeletes(keysToPurge, currOmSnapshot.getKeyManager(),
keysToModify, currSnapInfo.getTableKey());
}
} finally {
- if (previousSnapshot != null) {
- rcPrevOmSnapshot.close();
- }
+ IOUtils.closeQuietly(rcPrevOmSnapshot, rcPrevToPrevOmSnapshot);
}
}
}
}
+
+ updateSnapshotExclusiveSize();
updateDeepCleanedSnapshots(deepCleanedSnapshots);
}
+ @SuppressWarnings("checkstyle:ParameterNumber")
+ private void calculateExclusiveSize(
+ SnapshotInfo previousSnapshot,
+ SnapshotInfo previousToPrevSnapshot,
+ OmKeyInfo keyInfo,
+ OmBucketInfo bucketInfo, long volumeId,
+ Table<String, String> snapRenamedTable,
+ Table<String, OmKeyInfo> previousKeyTable,
+ Table<String, String> prevRenamedTable,
+ Table<String, OmKeyInfo> previousToPrevKeyTable) throws IOException {
+ String prevSnapKey = previousSnapshot.getTableKey();
+ long exclusiveReplicatedSize =
+ exclusiveReplicatedSizeMap.getOrDefault(
+ prevSnapKey, 0L) + keyInfo.getReplicatedSize();
+ long exclusiveSize = exclusiveSizeMap.getOrDefault(
+ prevSnapKey, 0L) + keyInfo.getDataSize();
Review Comment:
Default is supposed to be 0 or current exclusive size of snapshot? For
example, key1 is share between snapshot1 and snapshot2. When one of the
snapshot gets deleted, key1 will be become exclusive for other snapshot. In
that case, it should be on top of existing exclusive size for the snapshot.
--
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]
