diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index d65b84cc610..99ce83d0bdb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -50,6 +50,11 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_QUOTA_BY_STORAGETYPE_ENAB import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_STORAGE_POLICY_ENABLED_KEY; public class FSDirAttrOp { + + protected enum SetRepStatus { + UNCHANGED, INVALID, SUCCESS + } + static FileStatus setPermission( FSDirectory fsd, FSPermissionChecker pc, final String src, FsPermission permission) throws IOException { @@ -130,11 +135,11 @@ public class FSDirAttrOp { return fsd.getAuditFileInfo(iip); } - static boolean setReplication( + static SetRepStatus setReplication( FSDirectory fsd, FSPermissionChecker pc, BlockManager bm, String src, final short replication) throws IOException { bm.verifyReplication(src, replication, null); - final boolean isFile; + final SetRepStatus status; fsd.writeLock(); try { final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE); @@ -142,16 +147,14 @@ public class FSDirAttrOp { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } - final BlockInfo[] blocks = unprotectedSetReplication(fsd, iip, - replication); - isFile = blocks != null; - if (isFile) { + status = unprotectedSetReplication(fsd, iip, replication); + if (status == SetRepStatus.SUCCESS) { fsd.getEditLog().logSetReplication(iip.getPath(), replication); } } finally { fsd.writeUnlock(); } - return isFile; + return status; } static FileStatus unsetStoragePolicy(FSDirectory fsd, FSPermissionChecker pc, @@ -376,7 +379,7 @@ public class FSDirAttrOp { } } - static BlockInfo[] unprotectedSetReplication( + static SetRepStatus unprotectedSetReplication( FSDirectory fsd, INodesInPath iip, short replication) throws QuotaExceededException, UnresolvedLinkException, SnapshotAccessControlException, UnsupportedActionException { @@ -386,12 +389,20 @@ public class FSDirAttrOp { final INode inode = iip.getLastINode(); if (inode == null || !inode.isFile() || inode.asFile().isStriped()) { // TODO we do not support replication on stripe layout files yet - return null; + // We return invalid here, so we skip writing an edit, but also write an + // unsuccessful audit message. + return SetRepStatus.INVALID; } INodeFile file = inode.asFile(); // Make sure the directory has sufficient quotas short oldBR = file.getPreferredBlockReplication(); + if (oldBR == replication) { + // No need to do anything as the requested rep factor is the same as + // existing. Returning UNCHANGED to we can skip writing edits, but still + // log a successful audit message. + return SetRepStatus.UNCHANGED; + } long size = file.computeFileSize(true, true); // Ensure the quota does not exceed @@ -422,7 +433,7 @@ public class FSDirAttrOp { oldBR, iip.getPath()); } } - return file.getBlocks(); + return SetRepStatus.SUCCESS; } static void unprotectedSetStoragePolicy(FSDirectory fsd, BlockManager bm, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 3b54509eafa..f1b339ceba8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -2239,14 +2239,14 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, boolean setReplication(final String src, final short replication) throws IOException { final String operationName = "setReplication"; - boolean success = false; + FSDirAttrOp.SetRepStatus status; checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); writeLock(); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot set replication for " + src); - success = FSDirAttrOp.setReplication(dir, pc, blockManager, src, + status = FSDirAttrOp.setReplication(dir, pc, blockManager, src, replication); } catch (AccessControlException e) { logAuditEvent(false, operationName, src); @@ -2254,11 +2254,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, } finally { writeUnlock(operationName); } - if (success) { + if (status == FSDirAttrOp.SetRepStatus.SUCCESS) { getEditLog().logSync(); - logAuditEvent(true, operationName, src); } - return success; + logAuditEvent(status != FSDirAttrOp.SetRepStatus.INVALID, + operationName, src); + return status != FSDirAttrOp.SetRepStatus.INVALID; } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java index 497d450de25..d89b0777633 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestSetrepIncreasing.java @@ -82,6 +82,12 @@ public class TestSetrepIncreasing { public void testSetrepIncreasing() throws IOException { setrep(3, 7, false); } + + @Test(timeout=120000) + public void testSetrepSameRepValue() throws IOException { + setrep(3, 3, false); + } + @Test(timeout=120000) public void testSetrepIncreasingSimulatedStorage() throws IOException { setrep(3, 7, true);