smengcl commented on code in PR #4607:
URL: https://github.com/apache/ozone/pull/4607#discussion_r1178493929
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -209,12 +209,12 @@ public BackgroundTaskResult call() throws Exception {
// or keep it in current snapshot deleted table.
List<SnapshotMoveKeyInfos> toReclaimList = new ArrayList<>();
List<SnapshotMoveKeyInfos> toNextDBList = new ArrayList<>();
- List<HddsProtos.KeyValue> renamedKeysList = new ArrayList<>();
+ List<HddsProtos.KeyValue> renamedList = new ArrayList<>();
Review Comment:
```suggestion
// A list of renamed keys/files/dirs
List<HddsProtos.KeyValue> renamedList = new ArrayList<>();
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -196,7 +197,10 @@ public class OmMetadataManagerImpl implements
OMMetadataManager,
*
|-------------------------------------------------------------------------|
* | snapshotInfoTable | /volume/bucket/snapshotName -> SnapshotInfo
|
*
|-------------------------------------------------------------------------|
- * |snapshotRenamedKeyTable| /volumeName/bucketName/objectID -> /v/b/origKey
|
+ * | snapshotRenamedTable | /volumeName/bucketName/objectID ->
|
+ * | | /volumeId/bucketId/parentId/dirName
|
+ * | | /volumeId/bucketId/parentId/fileName
|
+ * | | /volumeName/bucketName/keyName
|
Review Comment:
further clarification
```suggestion
* | snapshotRenamedTable | /volumeName/bucketName/objectID -> One of:
|
* | | 1. /volumeId/bucketId/parentId/dirName
|
* | | 2. /volumeId/bucketId/parentId/fileName
|
* | | 3. /volumeName/bucketName/keyName
|
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponseWithFSO.java:
##########
@@ -91,20 +91,19 @@ public void addToDBBatch(OMMetadataManager
omMetadataManager,
.deleteWithBatch(batchOperation, getFromKeyName());
omMetadataManager.getKeyTable(getBucketLayout())
.putWithBatch(batchOperation, getToKeyName(), getRenameKeyInfo());
+ }
Review Comment:
good catch
can we add a line in the PR description to say this bug is fixed?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -231,22 +231,22 @@ String.class, new StringCodec(), OmKeyInfo.class,
new OmDBSnapshotInfoCodec());
/**
- * SnapshotRenamedKeyTable that complements the keyTable (or fileTable)
- * entries of the immediately previous snapshot in the same snapshot
- * scope (bucket or volume).
+ * SnapshotRenamedTable that complements the keyTable (or fileTable)
+ * and dirTable entries of the immediately previous snapshot in the
+ * same snapshot scope (bucket or volume).
* <p>
- * Key renames between the two subsequent snapshots are captured, this
+ * Key/Dir renames between the two subsequent snapshots are captured, this
* information is used in {@link SnapshotDeletingService} to check if the
- * renamedKey is present in the previous snapshot's keyTable
+ * renamedKey or renamedDir is present in the previous snapshot's keyTable
* (or fileTable).
*/
public static final DBColumnFamilyDefinition<String, String>
SNAPSHOT_RENAMED_KEY_TABLE =
Review Comment:
Rename this var as well?
```suggestion
public static final DBColumnFamilyDefinition<String, String>
SNAPSHOT_RENAMED_TABLE =
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -449,18 +452,42 @@ private void submitSnapshotMoveDeletedKeys(SnapshotInfo
snapInfo,
private boolean checkDirReclaimable(
Table.KeyValue<String, OmKeyInfo> deletedDir,
- Table<String, OmDirectoryInfo> previousDirTable) throws IOException {
+ Table<String, OmDirectoryInfo> previousDirTable,
+ Table<String, String> renamedTable,
+ List<HddsProtos.KeyValue> renamedList) throws IOException {
if (previousDirTable == null) {
return true;
}
String deletedDirDbKey = deletedDir.getKey();
OmKeyInfo deletedDirInfo = deletedDir.getValue();
- // In OMKeyDeleteResponseWithFSO OzonePathKey is converted to
- // OzoneDeletePathKey. Changing it back to check the previous DirTable.
- String prevDbKey = ozoneManager.getMetadataManager()
- .getOzoneDeletePathDirKey(deletedDirDbKey);
+ String dbRenameKey = ozoneManager.getMetadataManager().getRenameKey(
+ deletedDirInfo.getVolumeName(), deletedDirInfo.getBucketName(),
+ deletedDirInfo.getObjectID());
+
+ /*
+ snapshotRenamedTable: /volumeName/bucketName/objectID ->
+ /volumeId/bucketId/parentId/dirName
+ */
+ String renamedKey = renamedTable.getIfExist(dbRenameKey);
Review Comment:
```suggestion
String dbKeyBeforeRename = renamedTable.getIfExist(dbRenameKey);
```
--
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]
