This is an automated email from the ASF dual-hosted git repository.
sodonnell pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new 8ae033d1a3d HDFS-16531. Avoid setReplication writing an edit record if
old replication equals the new value (#4148). Contributed by Stephen O'Donnell.
8ae033d1a3d is described below
commit 8ae033d1a3d65281d3ad1f07f430e74d3a01b1b9
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Sun Apr 17 13:05:11 2022 +0100
HDFS-16531. Avoid setReplication writing an edit record if old replication
equals the new value (#4148). Contributed by Stephen O'Donnell.
(cherry picked from commit dbeeee03639f41a022dd07d5fc04e3aa65a94b5f)
---
.../hadoop/hdfs/server/namenode/FSDirAttrOp.java | 31 +++++++++++++++-------
.../hadoop/hdfs/server/namenode/FSNamesystem.java | 11 ++++----
.../apache/hadoop/hdfs/TestSetrepIncreasing.java | 6 +++++
3 files changed, 33 insertions(+), 15 deletions(-)
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 5c347655fc8..9654f599ad9 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
@@ -49,6 +49,11 @@ import java.util.List;
import static
org.apache.hadoop.hdfs.DFSConfigKeys.DFS_QUOTA_BY_STORAGETYPE_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 {
@@ -129,11 +134,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);
@@ -141,16 +146,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,
@@ -374,7 +377,7 @@ public class FSDirAttrOp {
return dirNode;
}
- static BlockInfo[] unprotectedSetReplication(
+ static SetRepStatus unprotectedSetReplication(
FSDirectory fsd, INodesInPath iip, short replication)
throws QuotaExceededException, UnresolvedLinkException,
SnapshotAccessControlException, UnsupportedActionException {
@@ -384,12 +387,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
@@ -420,7 +431,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 3ce8a80be65..c74286ece09 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
@@ -2370,7 +2370,7 @@ 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();
FSPermissionChecker.setOperationType(operationName);
@@ -2379,7 +2379,7 @@ public class FSNamesystem implements Namesystem,
FSNamesystemMBean,
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);
} finally {
writeUnlock(operationName);
@@ -2388,11 +2388,12 @@ public class FSNamesystem implements Namesystem,
FSNamesystemMBean,
logAuditEvent(false, operationName, src);
throw e;
}
- 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);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
