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 ki...@apache.org on 2016/10/04 20:06:41 UTC
hadoop git commit: HDFS-10956. Remove rename/delete performance
penalty when not using snapshots. Contributed by Daryn Sharp.
Repository: hadoop
Updated Branches:
refs/heads/trunk 88b9444a8 -> 44f48ee96
HDFS-10956. Remove rename/delete performance penalty when not using snapshots. Contributed by Daryn Sharp.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/44f48ee9
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/44f48ee9
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/44f48ee9
Branch: refs/heads/trunk
Commit: 44f48ee96ee6b2a3909911c37bfddb0c963d5ffc
Parents: 88b9444
Author: Kihwal Lee <ki...@apache.org>
Authored: Tue Oct 4 15:05:09 2016 -0500
Committer: Kihwal Lee <ki...@apache.org>
Committed: Tue Oct 4 15:05:09 2016 -0500
----------------------------------------------------------------------
.../hdfs/server/namenode/FSDirDeleteOp.java | 4 ++--
.../hdfs/server/namenode/FSDirRenameOp.java | 12 +++++------
.../hdfs/server/namenode/FSDirSnapshotOp.java | 22 ++++++++++++++++++--
.../server/namenode/TestSnapshotPathINodes.java | 16 ++++++++++++++
4 files changed, 44 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44f48ee9/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
index 13f1092..21ee3ce 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
@@ -57,7 +57,7 @@ class FSDirDeleteOp {
try {
if (deleteAllowed(iip, iip.getPath()) ) {
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
- FSDirSnapshotOp.checkSnapshot(iip.getLastINode(), snapshottableDirs);
+ FSDirSnapshotOp.checkSnapshot(fsd, iip, snapshottableDirs);
ReclaimContext context = new ReclaimContext(
fsd.getBlockStoragePolicySuite(), collectedBlocks, removedINodes,
removedUCFiles);
@@ -140,7 +140,7 @@ class FSDirDeleteOp {
return;
}
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
- FSDirSnapshotOp.checkSnapshot(iip.getLastINode(), snapshottableDirs);
+ FSDirSnapshotOp.checkSnapshot(fsd, iip, snapshottableDirs);
boolean filesRemoved = unprotectedDelete(fsd, iip,
new ReclaimContext(fsd.getBlockStoragePolicySuite(),
collectedBlocks, removedINodes, removedUCFiles),
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44f48ee9/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
----------------------------------------------------------------------
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 0fdc545..911b178 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
@@ -156,7 +156,7 @@ class FSDirRenameOp {
assert fsd.hasWriteLock();
final INode srcInode = srcIIP.getLastINode();
try {
- validateRenameSource(srcIIP);
+ validateRenameSource(fsd, srcIIP);
} catch (SnapshotException e) {
throw e;
} catch (IOException ignored) {
@@ -365,7 +365,7 @@ class FSDirRenameOp {
final String dst = dstIIP.getPath();
final String error;
final INode srcInode = srcIIP.getLastINode();
- validateRenameSource(srcIIP);
+ validateRenameSource(fsd, srcIIP);
// validate the destination
if (dst.equals(src)) {
@@ -387,7 +387,7 @@ class FSDirRenameOp {
List<INodeDirectory> snapshottableDirs = new ArrayList<>();
if (dstInode != null) { // Destination exists
validateOverwrite(src, dst, overwrite, srcInode, dstInode);
- FSDirSnapshotOp.checkSnapshot(dstInode, snapshottableDirs);
+ FSDirSnapshotOp.checkSnapshot(fsd, dstIIP, snapshottableDirs);
}
INode dstParent = dstIIP.getINode(-2);
@@ -559,8 +559,8 @@ class FSDirRenameOp {
}
}
- private static void validateRenameSource(INodesInPath srcIIP)
- throws IOException {
+ private static void validateRenameSource(FSDirectory fsd,
+ INodesInPath srcIIP) throws IOException {
String error;
final INode srcInode = srcIIP.getLastINode();
// validate source
@@ -578,7 +578,7 @@ class FSDirRenameOp {
}
// srcInode and its subtree cannot contain snapshottable directories with
// snapshots
- FSDirSnapshotOp.checkSnapshot(srcInode, null);
+ FSDirSnapshotOp.checkSnapshot(fsd, srcIIP, null);
}
private static class RenameOperation {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44f48ee9/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java
index c565a6a..ad282d1 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java
@@ -35,7 +35,6 @@ import org.apache.hadoop.util.ChunkedArrayList;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.ListIterator;
import java.util.List;
class FSDirSnapshotOp {
@@ -252,7 +251,7 @@ class FSDirSnapshotOp {
* @param snapshottableDirs The list of directories that are snapshottable
* but do not have snapshots yet
*/
- static void checkSnapshot(
+ private static void checkSnapshot(
INode target, List<INodeDirectory> snapshottableDirs)
throws SnapshotException {
if (target.isDirectory()) {
@@ -276,4 +275,23 @@ class FSDirSnapshotOp {
}
}
}
+
+ /**
+ * Check if the given path (or one of its descendants) is snapshottable and
+ * already has snapshots.
+ *
+ * @param fsd the FSDirectory
+ * @param iip inodes of the path
+ * @param snapshottableDirs The list of directories that are snapshottable
+ * but do not have snapshots yet
+ */
+ static void checkSnapshot(FSDirectory fsd, INodesInPath iip,
+ List<INodeDirectory> snapshottableDirs) throws SnapshotException {
+ // avoid the performance penalty of recursing the tree if snapshots
+ // are not in use
+ SnapshotManager sm = fsd.getFSNamesystem().getSnapshotManager();
+ if (sm.getNumSnapshottableDirs() > 0) {
+ checkSnapshot(iip.getLastINode(), snapshottableDirs);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/44f48ee9/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
index 3a318bc..07f01d0 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.io.FileNotFoundException;
+import java.util.ArrayList;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
@@ -30,12 +31,15 @@ import org.apache.hadoop.hdfs.DFSTestUtil;
import org.apache.hadoop.hdfs.DFSUtil;
import org.apache.hadoop.hdfs.DistributedFileSystem;
import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.hdfs.protocol.SnapshotException;
import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.mockito.Mockito;
/** Test snapshot related operations. */
public class TestSnapshotPathINodes {
@@ -426,4 +430,16 @@ public class TestSnapshotPathINodes {
hdfs.deleteSnapshot(sub1, "s3");
hdfs.disallowSnapshot(sub1);
}
+
+ @Test
+ public void testShortCircuitSnapshotSearch() throws SnapshotException {
+ FSNamesystem fsn = cluster.getNamesystem();
+ SnapshotManager sm = fsn.getSnapshotManager();
+ assertEquals(0, sm.getNumSnapshottableDirs());
+
+ INodesInPath iip = Mockito.mock(INodesInPath.class);
+ List<INodeDirectory> snapDirs = new ArrayList<>();
+ FSDirSnapshotOp.checkSnapshot(fsn.getFSDirectory(), iip, snapDirs);
+ Mockito.verifyZeroInteractions(iip);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org