bshashikant commented on a change in pull request #2172:
URL: https://github.com/apache/hadoop/pull/2172#discussion_r462081243
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
##########
@@ -193,7 +194,7 @@ static INodesInPath unprotectedRenameTo(FSDirectory fsd,
}
validateNestSnapshot(fsd, src, dstParent.asDirectory(), snapshottableDirs);
-
+ FSDirSnapshotOp.checkUnderSameSnapshottableRoot(fsd, srcIIP, dstIIP);
Review comment:
SnapshotRename check should happen irrespective of orderedDeletion
config is set or not. Plus snapshotRename will always within the snapshottable
root implicitly. Any speciofic reason to merge these two checks at once?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
##########
@@ -193,7 +194,7 @@ static INodesInPath unprotectedRenameTo(FSDirectory fsd,
}
validateNestSnapshot(fsd, src, dstParent.asDirectory(), snapshottableDirs);
-
+ FSDirSnapshotOp.checkUnderSameSnapshottableRoot(fsd, srcIIP, dstIIP);
Review comment:
> There are 2 callers of fsd.ezManager.checkMoveValidity(srcIIP, dstIIP);
FSDirSnapshotOp.checkUnderSameSnapshottableRoo() is also called from same
two functions.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java
##########
@@ -341,4 +341,24 @@ static void checkSnapshot(FSDirectory fsd, INodesInPath
iip,
checkSnapshot(iip.getLastINode(), snapshottableDirs);
}
}
+
+ static void checkUnderSameSnapshottableRoot(FSDirectory fsd,
+ INodesInPath srcIIP, INodesInPath dstIIP) throws IOException {
+ // Ensure rename out of a snapshottable root is not permitted if ordered
+ // snapshot deletion feature is enabled
+ if (fsd.isSnapshotDeletionOrdered()) {
+ SnapshotManager snapshotManager = fsd.getFSNamesystem().
+ getSnapshotManager();
+ String errMsg = "Source " + srcIIP.getPath() +
+ " and dest " + dstIIP.getPath() + " are not under " +
+ "the same snapshot root.";
+ INodeDirectory src = snapshotManager.
+ getSnapshottableAncestorDir(srcIIP);
+ INodeDirectory dst = snapshotManager.getSnapshottableAncestorDir(dstIIP);
+ if (!(dstIIP.isDescendant(snapshotManager.
+ getSnapshottableAncestorDir(srcIIP)))) {
Review comment:
done
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
##########
@@ -193,7 +194,7 @@ static INodesInPath unprotectedRenameTo(FSDirectory fsd,
}
validateNestSnapshot(fsd, src, dstParent.asDirectory(), snapshottableDirs);
-
+ FSDirSnapshotOp.checkUnderSameSnapshottableRoot(fsd, srcIIP, dstIIP);
Review comment:
SnapshotRename check should happen irrespective of orderedDeletion
config is set or not. Plus snapshotRename will always within the snapshottable
root implicitly. Also, if just directories are marked snapshottable but
snapshots exist, the rename will still fail across snapshottable roots. The two
checks seem to be mutually exclusive to me.
Any specific reason to merge these two checks at once?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
