You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by we...@apache.org on 2019/11/01 23:46:25 UTC
[hadoop] branch branch-3.1 updated: HDFS-14925. Rename operation
should check nest snapshot (#1670)
This is an automated email from the ASF dual-hosted git repository.
weichiu pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.1 by this push:
new cda8b2a HDFS-14925. Rename operation should check nest snapshot (#1670)
cda8b2a is described below
commit cda8b2ac698197b62acb40ea7192ef2747c5936f
Author: Zhao Junwang <zh...@gmail.com>
AuthorDate: Sat Nov 2 07:37:53 2019 +0800
HDFS-14925. Rename operation should check nest snapshot (#1670)
If the src directory or any of its descendant is snapshottable
and the dst directory or any of its ancestors is snapshottable,
we consider this as nested snapshot, which should be denied.
Reviewed-by: Shashikant Banerjee <sh...@apache.org>
(cherry picked from commit de6b8b0c0b1933aab2af3e8adc50a2091d428238)
(cherry picked from commit c9fc118991104acf9109aa769fe1a0e84f425a61)
---
.../hadoop/hdfs/server/namenode/FSDirRenameOp.java | 57 +++++++++++++++++-----
.../server/namenode/snapshot/SnapshotManager.java | 17 +++++++
.../namenode/snapshot/TestRenameWithSnapshots.java | 40 ++++++++++++++-
3 files changed, 101 insertions(+), 13 deletions(-)
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
index 6ca6568..af156e9 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
@@ -143,8 +143,8 @@ class FSDirRenameOp {
* Change a path name
*
* @param fsd FSDirectory
- * @param src source path
- * @param dst destination path
+ * @param srcIIP source path
+ * @param dstIIP destination path
* @return true INodesInPath if rename succeeds; null otherwise
* @deprecated See {@link #renameToInt(FSDirectory, String, String,
* boolean, Options.Rename...)}
@@ -155,8 +155,9 @@ class FSDirRenameOp {
throws IOException {
assert fsd.hasWriteLock();
final INode srcInode = srcIIP.getLastINode();
+ List<INodeDirectory> snapshottableDirs = new ArrayList<>();
try {
- validateRenameSource(fsd, srcIIP);
+ validateRenameSource(fsd, srcIIP, snapshottableDirs);
} catch (SnapshotException e) {
throw e;
} catch (IOException ignored) {
@@ -190,6 +191,8 @@ class FSDirRenameOp {
return null;
}
+ validateNestSnapshot(fsd, src, dstParent.asDirectory(), snapshottableDirs);
+
fsd.ezManager.checkMoveValidity(srcIIP, dstIIP);
// Ensure dst has quota to accommodate rename
verifyFsLimitsForRename(fsd, srcIIP, dstIIP);
@@ -342,8 +345,8 @@ class FSDirRenameOp {
* for details related to rename semantics and exceptions.
*
* @param fsd FSDirectory
- * @param src source path
- * @param dst destination path
+ * @param srcIIP source path
+ * @param dstIIP destination path
* @param timestamp modification time
* @param collectedBlocks blocks to be removed
* @param options Rename options
@@ -361,7 +364,8 @@ class FSDirRenameOp {
final String dst = dstIIP.getPath();
final String error;
final INode srcInode = srcIIP.getLastINode();
- validateRenameSource(fsd, srcIIP);
+ List<INodeDirectory> srcSnapshottableDirs = new ArrayList<>();
+ validateRenameSource(fsd, srcIIP, srcSnapshottableDirs);
// validate the destination
if (dst.equals(src)) {
@@ -380,10 +384,10 @@ class FSDirRenameOp {
BlockStoragePolicySuite bsps = fsd.getBlockStoragePolicySuite();
fsd.ezManager.checkMoveValidity(srcIIP, dstIIP);
final INode dstInode = dstIIP.getLastINode();
- List<INodeDirectory> snapshottableDirs = new ArrayList<>();
+ List<INodeDirectory> dstSnapshottableDirs = new ArrayList<>();
if (dstInode != null) { // Destination exists
validateOverwrite(src, dst, overwrite, srcInode, dstInode);
- FSDirSnapshotOp.checkSnapshot(fsd, dstIIP, snapshottableDirs);
+ FSDirSnapshotOp.checkSnapshot(fsd, dstIIP, dstSnapshottableDirs);
}
INode dstParent = dstIIP.getINode(-2);
@@ -400,6 +404,9 @@ class FSDirRenameOp {
throw new ParentNotDirectoryException(error);
}
+ validateNestSnapshot(fsd, src,
+ dstParent.asDirectory(), srcSnapshottableDirs);
+
// Ensure dst has quota to accommodate rename
verifyFsLimitsForRename(fsd, srcIIP, dstIIP);
verifyQuotaForRename(fsd, srcIIP, dstIIP);
@@ -439,10 +446,10 @@ class FSDirRenameOp {
}
}
- if (snapshottableDirs.size() > 0) {
+ if (dstSnapshottableDirs.size() > 0) {
// There are snapshottable directories (without snapshots) to be
// deleted. Need to update the SnapshotManager.
- fsd.getFSNamesystem().removeSnapshottableDirs(snapshottableDirs);
+ fsd.getFSNamesystem().removeSnapshottableDirs(dstSnapshottableDirs);
}
tx.updateQuotasInSourceTree(bsps);
@@ -556,7 +563,8 @@ class FSDirRenameOp {
}
private static void validateRenameSource(FSDirectory fsd,
- INodesInPath srcIIP) throws IOException {
+ INodesInPath srcIIP, List<INodeDirectory> snapshottableDirs)
+ throws IOException {
String error;
final INode srcInode = srcIIP.getLastINode();
// validate source
@@ -574,7 +582,32 @@ class FSDirRenameOp {
}
// srcInode and its subtree cannot contain snapshottable directories with
// snapshots
- FSDirSnapshotOp.checkSnapshot(fsd, srcIIP, null);
+ FSDirSnapshotOp.checkSnapshot(fsd, srcIIP, snapshottableDirs);
+ }
+
+ private static void validateNestSnapshot(FSDirectory fsd, String srcPath,
+ INodeDirectory dstParent, List<INodeDirectory> snapshottableDirs)
+ throws SnapshotException {
+
+ if (fsd.getFSNamesystem().getSnapshotManager().isAllowNestedSnapshots()) {
+ return;
+ }
+
+ /*
+ * snapshottableDirs is a list of snapshottable directory (child of rename
+ * src) which do not have snapshots yet. If this list is not empty, that
+ * means rename src has snapshottable descendant directories.
+ */
+ if (snapshottableDirs != null && snapshottableDirs.size() > 0) {
+ if (fsd.getFSNamesystem().getSnapshotManager()
+ .isDescendantOfSnapshotRoot(dstParent)) {
+ String dstPath = dstParent.getFullPathName();
+ throw new SnapshotException("Unable to rename because " + srcPath
+ + " has snapshottable descendant directories and " + dstPath
+ + " is a descent of a snapshottable directory, and HDFS does"
+ + " not support nested snapshottable directory.");
+ }
+ }
}
private static class RenameOperation {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
index 1786346..b908ee9 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
@@ -155,6 +155,10 @@ public class SnapshotManager implements SnapshotStatsMXBean {
this.allowNestedSnapshots = allowNestedSnapshots;
}
+ public boolean isAllowNestedSnapshots() {
+ return allowNestedSnapshots;
+ }
+
private void checkNestedSnapshottable(INodeDirectory dir, String path)
throws SnapshotException {
if (allowNestedSnapshots) {
@@ -288,6 +292,19 @@ public class SnapshotManager implements SnapshotStatsMXBean {
}
}
+ public boolean isDescendantOfSnapshotRoot(INodeDirectory dir) {
+ if (dir.isSnapshottable()) {
+ return true;
+ } else {
+ for (INodeDirectory p = dir; p != null; p = p.getParent()) {
+ if (this.snapshottables.containsValue(p)) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
/**
* Create a snapshot of the given path.
* It is assumed that the caller will perform synchronization.
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java
index 1f624d0..8d54e15 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java
@@ -1109,7 +1109,45 @@ public class TestRenameWithSnapshots {
assertEquals(bar, dirs[0].getFullPath());
assertEquals(fooId, dirs[0].getDirStatus().getFileId());
}
-
+
+ /**
+ * Test rename where src has snapshottable descendant directories and
+ * dst is a descent of a snapshottable directory. Such case will cause
+ * nested snapshot which HDFS currently not fully supported.
+ */
+ @Test
+ public void testRenameWithNestedSnapshottableDirs() throws Exception {
+ final Path sdir1 = new Path("/dir1");
+ final Path sdir2 = new Path("/dir2");
+ final Path foo = new Path(sdir1, "foo");
+ final Path bar = new Path(sdir2, "bar");
+
+ hdfs.mkdirs(foo);
+ hdfs.mkdirs(bar);
+
+ hdfs.allowSnapshot(foo);
+ hdfs.allowSnapshot(sdir2);
+
+ try {
+ hdfs.rename(foo, bar, Rename.OVERWRITE);
+ fail("Except exception since " + "Unable to rename because "
+ + foo.toString() + " has snapshottable descendant directories and "
+ + sdir2.toString() + " is a descent of a snapshottable directory, "
+ + "and HDFS does not support nested snapshottable directory.");
+ } catch (IOException e) {
+ GenericTestUtils.assertExceptionContains("Unable to rename because "
+ + foo.toString() + " has snapshottable descendant directories and "
+ + sdir2.toString() + " is a descent of a snapshottable directory, "
+ + "and HDFS does not support nested snapshottable directory.", e);
+ }
+
+ hdfs.disallowSnapshot(foo);
+ hdfs.rename(foo, bar, Rename.OVERWRITE);
+ SnapshottableDirectoryStatus[] dirs = fsn.getSnapshottableDirListing();
+ assertEquals(1, dirs.length);
+ assertEquals(sdir2, dirs[0].getFullPath());
+ }
+
/**
* After rename, delete the snapshot in src
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org