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 so...@apache.org on 2022/04/20 20:04:48 UTC
[hadoop] branch branch-3.2 updated: Revert "HDFS-16531. Avoid setReplication writing an edit record if old replication equals the new value (#4148). Contributed by Stephen O'Donnell."
This is an automated email from the ASF dual-hosted git repository.
sodonnell pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.2 by this push:
new bacd0b9a928 Revert "HDFS-16531. Avoid setReplication writing an edit record if old replication equals the new value (#4148). Contributed by Stephen O'Donnell."
bacd0b9a928 is described below
commit bacd0b9a9289172efb21f9c9afcd7b04ea0732b3
Author: S O'Donnell <so...@cloudera.com>
AuthorDate: Wed Apr 20 20:57:23 2022 +0100
Revert "HDFS-16531. Avoid setReplication writing an edit record if old replication equals the new value (#4148). Contributed by Stephen O'Donnell."
This reverts commit 045d48530cbbd7baa2b67937e103b9400487f1a4.
---
.../hadoop/hdfs/server/namenode/FSDirAttrOp.java | 31 +++++++---------------
.../hadoop/hdfs/server/namenode/FSNamesystem.java | 11 ++++----
.../apache/hadoop/hdfs/TestSetrepIncreasing.java | 6 -----
3 files changed, 15 insertions(+), 33 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 99ce83d0bdb..d65b84cc610 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,11 +50,6 @@ 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 {
@@ -135,11 +130,11 @@ public class FSDirAttrOp {
return fsd.getAuditFileInfo(iip);
}
- static SetRepStatus setReplication(
+ static boolean setReplication(
FSDirectory fsd, FSPermissionChecker pc, BlockManager bm, String src,
final short replication) throws IOException {
bm.verifyReplication(src, replication, null);
- final SetRepStatus status;
+ final boolean isFile;
fsd.writeLock();
try {
final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE);
@@ -147,14 +142,16 @@ public class FSDirAttrOp {
fsd.checkPathAccess(pc, iip, FsAction.WRITE);
}
- status = unprotectedSetReplication(fsd, iip, replication);
- if (status == SetRepStatus.SUCCESS) {
+ final BlockInfo[] blocks = unprotectedSetReplication(fsd, iip,
+ replication);
+ isFile = blocks != null;
+ if (isFile) {
fsd.getEditLog().logSetReplication(iip.getPath(), replication);
}
} finally {
fsd.writeUnlock();
}
- return status;
+ return isFile;
}
static FileStatus unsetStoragePolicy(FSDirectory fsd, FSPermissionChecker pc,
@@ -379,7 +376,7 @@ public class FSDirAttrOp {
}
}
- static SetRepStatus unprotectedSetReplication(
+ static BlockInfo[] unprotectedSetReplication(
FSDirectory fsd, INodesInPath iip, short replication)
throws QuotaExceededException, UnresolvedLinkException,
SnapshotAccessControlException, UnsupportedActionException {
@@ -389,20 +386,12 @@ 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
- // We return invalid here, so we skip writing an edit, but also write an
- // unsuccessful audit message.
- return SetRepStatus.INVALID;
+ return null;
}
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
@@ -433,7 +422,7 @@ public class FSDirAttrOp {
oldBR, iip.getPath());
}
}
- return SetRepStatus.SUCCESS;
+ return file.getBlocks();
}
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 f1b339ceba8..3b54509eafa 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";
- FSDirAttrOp.SetRepStatus status;
+ boolean success = false;
checkOperation(OperationCategory.WRITE);
final FSPermissionChecker pc = getPermissionChecker();
writeLock();
try {
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot set replication for " + src);
- status = FSDirAttrOp.setReplication(dir, pc, blockManager, src,
+ success = FSDirAttrOp.setReplication(dir, pc, blockManager, src,
replication);
} catch (AccessControlException e) {
logAuditEvent(false, operationName, src);
@@ -2254,12 +2254,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
} finally {
writeUnlock(operationName);
}
- if (status == FSDirAttrOp.SetRepStatus.SUCCESS) {
+ if (success) {
getEditLog().logSync();
+ logAuditEvent(true, operationName, src);
}
- logAuditEvent(status != FSDirAttrOp.SetRepStatus.INVALID,
- operationName, src);
- return status != FSDirAttrOp.SetRepStatus.INVALID;
+ return success;
}
/**
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 d89b0777633..497d450de25 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,12 +82,6 @@ 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: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org