This is an automated email from the ASF dual-hosted git repository. zanderxu pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit 7577f3ad6194a24c1ed544e23d50ee4f1a6dfa5b Author: ZanderXu <zande...@apache.org> AuthorDate: Fri Mar 29 18:16:07 2024 +0800 HDFS-17388. [FGL] Client RPCs involving write process supports fine-grained lock (#6589) --- .../hdfs/server/blockmanagement/BlockManager.java | 2 +- .../hadoop/hdfs/server/namenode/FSDirAppendOp.java | 5 +- .../hadoop/hdfs/server/namenode/FSDirDeleteOp.java | 4 +- .../server/namenode/FSDirEncryptionZoneOp.java | 8 +- .../server/namenode/FSDirStatAndListingOp.java | 25 ++++-- .../hdfs/server/namenode/FSDirWriteFileOp.java | 43 +++++----- .../hadoop/hdfs/server/namenode/FSNamesystem.java | 96 +++++++++++----------- .../hadoop/hdfs/server/namenode/INodeFile.java | 6 ++ 8 files changed, 107 insertions(+), 82 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 4d405443290..69438c7a647 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -1584,7 +1584,7 @@ public LocatedBlocks createLocatedBlocks(final BlockInfo[] blocks, final boolean inSnapshot, FileEncryptionInfo feInfo, ErasureCodingPolicy ecPolicy) throws IOException { - assert namesystem.hasReadLock(); + assert namesystem.hasReadLock(FSNamesystemLockMode.BM); if (blocks == null) { return null; } else if (blocks.length == 0) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java index ba00e8ae936..058928c41ab 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java @@ -38,6 +38,7 @@ import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem.RecoverLeaseOp; import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion.Feature; +import org.apache.hadoop.hdfs.server.namenode.fgl.FSNamesystemLockMode; import org.apache.hadoop.ipc.RetriableException; import org.apache.hadoop.util.Preconditions; @@ -82,7 +83,7 @@ static LastBlockWithStatus appendFile(final FSNamesystem fsn, final String srcArg, final FSPermissionChecker pc, final String holder, final String clientMachine, final boolean newBlock, final boolean logRetryCache) throws IOException { - assert fsn.hasWriteLock(); + assert fsn.hasWriteLock(FSNamesystemLockMode.GLOBAL); final LocatedBlock lb; final FSDirectory fsd = fsn.getFSDirectory(); @@ -180,7 +181,7 @@ static LocatedBlock prepareFileForAppend(final FSNamesystem fsn, final String clientMachine, final boolean newBlock, final boolean writeToEditLog, final boolean logRetryCache) throws IOException { - assert fsn.hasWriteLock(); + assert fsn.hasWriteLock(FSNamesystemLockMode.GLOBAL); final INodeFile file = iip.getLastINode().asFile(); final QuotaCounts delta = verifyQuotaForUCBlock(fsn, file, iip); 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 2dfb90ee672..f513357d9c5 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 @@ -24,6 +24,7 @@ import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo; import org.apache.hadoop.hdfs.server.namenode.INode.ReclaimContext; +import org.apache.hadoop.hdfs.server.namenode.fgl.FSNamesystemLockMode; import org.apache.hadoop.util.ChunkedArrayList; import java.io.IOException; @@ -170,7 +171,8 @@ static void deleteForEditLog(FSDirectory fsd, INodesInPath iip, long mtime) static BlocksMapUpdateInfo deleteInternal( FSNamesystem fsn, INodesInPath iip, boolean logRetryCache) throws IOException { - assert fsn.hasWriteLock(); + // Delete INode and modify BlockInfo + assert fsn.hasWriteLock(FSNamesystemLockMode.GLOBAL); if (NameNode.stateChangeLog.isDebugEnabled()) { NameNode.stateChangeLog.debug("DIR* NameSystem.delete: " + iip.getPath()); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java index 8463d828312..8ca1957c0a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java @@ -84,8 +84,8 @@ private FSDirEncryptionZoneOp() {} private static EncryptedKeyVersion generateEncryptedDataEncryptionKey( final FSDirectory fsd, final String ezKeyName) throws IOException { // must not be holding lock during this operation - assert !fsd.getFSNamesystem().hasReadLock(); - assert !fsd.getFSNamesystem().hasWriteLock(); + assert !fsd.getFSNamesystem().hasReadLock(FSNamesystemLockMode.FS); + assert !fsd.getFSNamesystem().hasWriteLock(FSNamesystemLockMode.FS); if (ezKeyName == null) { return null; } @@ -657,13 +657,13 @@ static EncryptionKeyInfo getEncryptionKeyInfo(FSNamesystem fsn, Preconditions.checkNotNull(ezKeyName); // Generate EDEK while not holding the fsn lock. - fsn.writeUnlock("getEncryptionKeyInfo"); + fsn.writeUnlock(FSNamesystemLockMode.FS, "getEncryptionKeyInfo"); try { EncryptionFaultInjector.getInstance().startFileBeforeGenerateKey(); return new EncryptionKeyInfo(protocolVersion, suite, ezKeyName, generateEncryptedDataEncryptionKey(fsd, ezKeyName)); } finally { - fsn.writeLock(); + fsn.writeLock(FSNamesystemLockMode.FS); EncryptionFaultInjector.getInstance().startFileAfterGenerateKey(); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java index 4547228364d..2fe8c307d2c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs.server.namenode; +import org.apache.hadoop.hdfs.server.namenode.fgl.FSNamesystemLockMode; import org.apache.hadoop.util.Preconditions; import org.apache.hadoop.fs.ContentSummary; @@ -444,16 +445,22 @@ private static HdfsFileStatus createFileStatus( if (isEncrypted) { feInfo = FSDirEncryptionZoneOp.getFileEncryptionInfo(fsd, iip); } + // ComputeFileSize and needLocation need BM lock. if (needLocation) { - final boolean inSnapshot = snapshot != Snapshot.CURRENT_STATE_ID; - final boolean isUc = !inSnapshot && fileNode.isUnderConstruction(); - final long fileSize = !inSnapshot && isUc - ? fileNode.computeFileSizeNotIncludingLastUcBlock() : size; - loc = fsd.getBlockManager().createLocatedBlocks( - fileNode.getBlocks(snapshot), fileSize, isUc, 0L, size, - needBlockToken, inSnapshot, feInfo, ecPolicy); - if (loc == null) { - loc = new LocatedBlocks(); + fsd.getFSNamesystem().readLock(FSNamesystemLockMode.BM); + try { + final boolean inSnapshot = snapshot != Snapshot.CURRENT_STATE_ID; + final boolean isUc = !inSnapshot && fileNode.isUnderConstruction(); + final long fileSize = !inSnapshot && isUc + ? fileNode.computeFileSizeNotIncludingLastUcBlock() : size; + loc = fsd.getBlockManager().createLocatedBlocks( + fileNode.getBlocks(snapshot), fileSize, isUc, 0L, size, + needBlockToken, inSnapshot, feInfo, ecPolicy); + if (loc == null) { + loc = new LocatedBlocks(); + } + } finally { + fsd.getFSNamesystem().readUnlock(FSNamesystemLockMode.BM, "createFileStatus"); } } } else if (node.isDirectory()) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java index 37000421abc..3067162e3a1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java @@ -364,7 +364,7 @@ static HdfsFileStatus startFile( boolean shouldReplicate, String ecPolicyName, String storagePolicy, boolean logRetryEntry) throws IOException { - assert fsn.hasWriteLock(); + assert fsn.hasWriteLock(FSNamesystemLockMode.FS); boolean overwrite = flag.contains(CreateFlag.OVERWRITE); boolean isLazyPersist = flag.contains(CreateFlag.LAZY_PERSIST); @@ -372,22 +372,27 @@ static HdfsFileStatus startFile( FSDirectory fsd = fsn.getFSDirectory(); if (iip.getLastINode() != null) { - if (overwrite) { - List<INode> toRemoveINodes = new ChunkedArrayList<>(); - List<Long> toRemoveUCFiles = new ChunkedArrayList<>(); - long ret = FSDirDeleteOp.delete(fsd, iip, toRemoveBlocks, - toRemoveINodes, toRemoveUCFiles, now()); - if (ret >= 0) { - iip = INodesInPath.replace(iip, iip.length() - 1, null); - FSDirDeleteOp.incrDeletedFileCount(ret); - fsn.removeLeasesAndINodes(toRemoveUCFiles, toRemoveINodes, true); + fsn.writeLock(FSNamesystemLockMode.BM); + try { + if (overwrite) { + List<INode> toRemoveINodes = new ChunkedArrayList<>(); + List<Long> toRemoveUCFiles = new ChunkedArrayList<>(); + long ret = FSDirDeleteOp.delete(fsd, iip, toRemoveBlocks, + toRemoveINodes, toRemoveUCFiles, now()); + if (ret >= 0) { + iip = INodesInPath.replace(iip, iip.length() - 1, null); + FSDirDeleteOp.incrDeletedFileCount(ret); + fsn.removeLeasesAndINodes(toRemoveUCFiles, toRemoveINodes, true); + } + } else { + // If lease soft limit time is expired, recover the lease + fsn.recoverLeaseInternal(FSNamesystem.RecoverLeaseOp.CREATE_FILE, iip, + src, holder, clientMachine, false); + throw new FileAlreadyExistsException(src + " for client " + + clientMachine + " already exists"); } - } else { - // If lease soft limit time is expired, recover the lease - fsn.recoverLeaseInternal(FSNamesystem.RecoverLeaseOp.CREATE_FILE, iip, - src, holder, clientMachine, false); - throw new FileAlreadyExistsException(src + " for client " + - clientMachine + " already exists"); + } finally { + fsn.writeUnlock(FSNamesystemLockMode.BM, "create"); } } fsn.checkFsObjectLimit(); @@ -597,7 +602,7 @@ private static FileState analyzeFileState( FSNamesystem fsn, INodesInPath iip, long fileId, String clientName, ExtendedBlock previous, LocatedBlock[] onRetryBlock) throws IOException { - assert fsn.hasReadLock(); + assert fsn.hasReadLock(FSNamesystemLockMode.GLOBAL); String src = iip.getPath(); checkBlock(fsn, previous); onRetryBlock[0] = null; @@ -695,7 +700,7 @@ private static boolean completeFileInternal( FSNamesystem fsn, INodesInPath iip, String holder, Block last, long fileId) throws IOException { - assert fsn.hasWriteLock(); + assert fsn.hasWriteLock(FSNamesystemLockMode.GLOBAL); final String src = iip.getPath(); final INodeFile pendingFile; INode inode = null; @@ -779,7 +784,7 @@ private static void persistNewBlock( static void saveAllocatedBlock(FSNamesystem fsn, String src, INodesInPath inodesInPath, Block newBlock, DatanodeStorageInfo[] targets, BlockType blockType) throws IOException { - assert fsn.hasWriteLock(); + assert fsn.hasWriteLock(FSNamesystemLockMode.GLOBAL); BlockInfo b = addBlock(fsn.dir, src, inodesInPath, newBlock, targets, blockType); logAllocatedBlock(src, b); 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 2a27e8c0273..6f032811b88 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 @@ -2439,14 +2439,14 @@ void createSymlink(String target, String link, checkOperation(OperationCategory.WRITE); FSPermissionChecker.setOperationType(operationName); try { - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot create symlink " + link); auditStat = FSDirSymlinkOp.createSymlinkInt(this, target, link, dirPerms, createParent, logRetryCache); } finally { - writeUnlock(operationName, + writeUnlock(FSNamesystemLockMode.FS, operationName, getLockReportInfoSupplier(link, target, auditStat)); } } catch (AccessControlException e) { @@ -2796,7 +2796,7 @@ private HdfsFileStatus startFileInt(String src, checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot create file" + src); @@ -2858,7 +2858,8 @@ private HdfsFileStatus startFileInt(String src, dir.writeUnlock(); } } finally { - writeUnlock("create", getLockReportInfoSupplier(src, null, stat)); + writeUnlock(FSNamesystemLockMode.FS, "create", + getLockReportInfoSupplier(src, null, stat)); // There might be transactions logged while trying to recover the lease. // They need to be sync'ed even when an exception was thrown. if (!skipSync) { @@ -2892,7 +2893,7 @@ boolean recoverLease(String src, String holder, String clientMachine) checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot recover the lease of " + src); @@ -2912,7 +2913,7 @@ boolean recoverLease(String src, String holder, String clientMachine) skipSync = true; throw se; } finally { - writeUnlock("recoverLease"); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName); // There might be transactions logged while trying to recover the lease. // They need to be sync'ed even when an exception was thrown. if (!skipSync) { @@ -3032,7 +3033,7 @@ LastBlockWithStatus appendFile(String srcArg, String holder, checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot append to file" + srcArg); @@ -3042,7 +3043,7 @@ LastBlockWithStatus appendFile(String srcArg, String holder, skipSync = true; throw se; } finally { - writeUnlock(operationName, getLockReportInfoSupplier(srcArg)); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName, getLockReportInfoSupplier(srcArg)); // There might be transactions logged while trying to recover the lease // They need to be sync'ed even when an exception was thrown. if (!skipSync) { @@ -3089,13 +3090,13 @@ LocatedBlock getAdditionalBlock( checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - readLock(); + readLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); r = FSDirWriteFileOp.validateAddBlock(this, pc, src, fileId, clientName, previous, onRetryBlock); } finally { - readUnlock(operationName); + readUnlock(FSNamesystemLockMode.GLOBAL, operationName); } if (r == null) { @@ -3108,14 +3109,14 @@ LocatedBlock getAdditionalBlock( blockManager, src, excludedNodes, favoredNodes, flags, r); checkOperation(OperationCategory.WRITE); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); LocatedBlock lb; try { checkOperation(OperationCategory.WRITE); lb = FSDirWriteFileOp.storeAllocatedBlock( this, src, fileId, clientName, previous, targets); } finally { - writeUnlock(operationName); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName); } getEditLog().logSync(); return lb; @@ -3141,7 +3142,7 @@ LocatedBlock getAdditionalDatanode(String src, long fileId, checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - readLock(); + readLock(FSNamesystemLockMode.FS); try { // Changing this operation category to WRITE instead of making getAdditionalDatanode as a // read method is aim to let Active NameNode to handle this RPC, because Active NameNode @@ -3166,7 +3167,7 @@ LocatedBlock getAdditionalDatanode(String src, long fileId, "src=%s, fileId=%d, blk=%s, clientName=%s, clientMachine=%s", src, fileId, blk, clientName, clientMachine)); } finally { - readUnlock("getAdditionalDatanode"); + readUnlock(FSNamesystemLockMode.FS, operationName); } if (clientnode == null) { @@ -3193,7 +3194,7 @@ void abandonBlock(ExtendedBlock b, long fileId, String src, String holder) checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot abandon block " + b + " for file" + src); @@ -3201,7 +3202,7 @@ void abandonBlock(ExtendedBlock b, long fileId, String src, String holder) NameNode.stateChangeLog.debug( "BLOCK* NameSystem.abandonBlock: {} is removed from pendingCreates", b); } finally { - writeUnlock("abandonBlock"); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName); } getEditLog().logSync(); } @@ -3216,7 +3217,7 @@ INodeFile checkLease(INodesInPath iip, String holder, long fileId) throws LeaseExpiredException, FileNotFoundException { String src = iip.getPath(); INode inode = iip.getLastINode(); - assert hasReadLock(); + assert hasReadLock(FSNamesystemLockMode.FS); if (inode == null) { throw new FileNotFoundException("File does not exist: " + leaseExceptionString(src, fileId, holder)); @@ -3260,14 +3261,14 @@ boolean completeFile(final String src, String holder, checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot complete file " + src); success = FSDirWriteFileOp.completeFile(this, pc, src, holder, last, fileId); } finally { - writeUnlock("completeFile"); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName); } getEditLog().logSync(); if (success) { @@ -3282,6 +3283,7 @@ boolean completeFile(final String src, String holder, * @param blockType is the file under striping or contiguous layout? */ Block createNewBlock(BlockType blockType) throws IOException { + // nextBlockId and nextGenerationStamp need to write edit log, so it needs FSLock. assert hasWriteLock(FSNamesystemLockMode.GLOBAL); Block b = new Block(nextBlockId(blockType), 0, 0); // Increment the generation stamp for every new block. @@ -3295,7 +3297,7 @@ Block createNewBlock(BlockType blockType) throws IOException { * all blocks, otherwise check only penultimate block. */ boolean checkFileProgress(String src, INodeFile v, boolean checkall) { - assert hasReadLock(); + assert hasReadLock(FSNamesystemLockMode.GLOBAL); if (checkall) { return checkBlocksComplete(src, true, v.getBlocks()); } else { @@ -3341,14 +3343,14 @@ boolean renameTo(String src, String dst, boolean logRetryCache) final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); try { - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot rename " + src); ret = FSDirRenameOp.renameToInt(dir, pc, src, dst, logRetryCache); } finally { FileStatus status = ret != null ? ret.auditStat : null; - writeUnlock(operationName, + writeUnlock(FSNamesystemLockMode.FS, operationName, getLockReportInfoSupplier(src, dst, status)); } } catch (AccessControlException e) { @@ -3373,7 +3375,7 @@ void renameTo(final String src, final String dst, final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); try { - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot rename " + src); @@ -3381,7 +3383,7 @@ void renameTo(final String src, final String dst, options); } finally { FileStatus status = res != null ? res.auditStat : null; - writeUnlock(operationName, + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName, getLockReportInfoSupplier(src, dst, status)); } } catch (AccessControlException e) { @@ -3416,7 +3418,7 @@ boolean delete(String src, boolean recursive, boolean logRetryCache) FSPermissionChecker.setOperationType(operationName); boolean ret = false; try { - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot delete " + src); @@ -3424,7 +3426,7 @@ boolean delete(String src, boolean recursive, boolean logRetryCache) this, pc, src, recursive, logRetryCache); ret = toRemovedBlocks != null; } finally { - writeUnlock(operationName, getLockReportInfoSupplier(src)); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName, getLockReportInfoSupplier(src)); } } catch (AccessControlException e) { logAuditEvent(false, operationName, src); @@ -3454,7 +3456,7 @@ FSPermissionChecker getPermissionChecker() void removeLeasesAndINodes(List<Long> removedUCFiles, List<INode> removedINodes, final boolean acquireINodeMapLock) { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.FS); for(long i : removedUCFiles) { leaseManager.removeLease(i); } @@ -3559,14 +3561,14 @@ boolean mkdirs(String src, PermissionStatus permissions, final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); try { - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot create directory " + src); auditStat = FSDirMkdirOp.mkdirs(this, pc, src, permissions, createParent); } finally { - writeUnlock(operationName, + writeUnlock(FSNamesystemLockMode.FS, operationName, getLockReportInfoSupplier(src, null, auditStat)); } } catch (AccessControlException e) { @@ -3707,7 +3709,7 @@ void fsync(String src, long fileId, String clientName, long lastBlockLength) checkOperation(OperationCategory.WRITE); final FSPermissionChecker pc = getPermissionChecker(); FSPermissionChecker.setOperationType(operationName); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot fsync file " + src); @@ -3720,7 +3722,7 @@ void fsync(String src, long fileId, String clientName, long lastBlockLength) } FSDirWriteFileOp.persistBlocks(dir, src, pendingFile, false); } finally { - writeUnlock("fsync"); + writeUnlock(FSNamesystemLockMode.GLOBAL, operationName); } getEditLog().logSync(); } @@ -3743,7 +3745,8 @@ boolean internalReleaseLease(Lease lease, String src, INodesInPath iip, String recoveryLeaseHolder) throws IOException { LOG.info("Recovering " + lease + ", src=" + src); assert !isInSafeMode(); - assert hasWriteLock(); + // finalizeINodeFileUnderConstruction needs global write lock. + assert hasWriteLock(FSNamesystemLockMode.GLOBAL); final INodeFile pendingFile = iip.getLastINode().asFile(); int nrBlocks = pendingFile.numBlocks(); @@ -3905,7 +3908,7 @@ boolean internalReleaseLease(Lease lease, String src, INodesInPath iip, private Lease reassignLease(Lease lease, String src, String newHolder, INodeFile pendingFile) { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.FS); if(newHolder == null) return lease; // The following transaction is not synced. Make sure it's sync'ed later. @@ -3914,7 +3917,7 @@ private Lease reassignLease(Lease lease, String src, String newHolder, } Lease reassignLeaseInternal(Lease lease, String newHolder, INodeFile pendingFile) { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.FS); pendingFile.getFileUnderConstructionFeature().setClientName(newHolder); return leaseManager.reassignLease(lease, pendingFile, newHolder); } @@ -5888,6 +5891,7 @@ public String getTopUserOpCounts() { */ long nextGenerationStamp(boolean legacyBlock) throws IOException { + // TODO: Use FSLock to make nextGenerationStamp thread safe. assert hasWriteLock(FSNamesystemLockMode.GLOBAL); checkNameNodeSafeMode("Cannot get next generation stamp"); @@ -5907,7 +5911,7 @@ long nextGenerationStamp(boolean legacyBlock) * @param blockType is the file under striping or contiguous layout? */ private long nextBlockId(BlockType blockType) throws IOException { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.GLOBAL); checkNameNodeSafeMode("Cannot get next block ID"); final long blockId = blockManager.nextBlockId(blockType); getEditLog().logAllocateBlockId(blockId); @@ -5956,7 +5960,7 @@ boolean isFileDeleted(INodeFile file) { private INodeFile checkUCBlock(ExtendedBlock block, String clientName) throws IOException { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.GLOBAL); checkNameNodeSafeMode("Cannot get a new generation stamp and an " + "access token for block " + block); @@ -6030,7 +6034,7 @@ LocatedBlock bumpBlockGenerationStamp(ExtendedBlock block, String clientName) throws IOException { final LocatedBlock locatedBlock; checkOperation(OperationCategory.WRITE); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); @@ -6064,7 +6068,7 @@ LocatedBlock bumpBlockGenerationStamp(ExtendedBlock block, blockManager.setBlockToken(locatedBlock, BlockTokenIdentifier.AccessMode.WRITE); } finally { - writeUnlock("bumpBlockGenerationStamp"); + writeUnlock(FSNamesystemLockMode.GLOBAL, "bumpBlockGenerationStamp"); } // Ensure we record the new generation stamp getEditLog().logSync(); @@ -6093,7 +6097,7 @@ void updatePipeline( + ", newNodes=" + Arrays.asList(newNodes) + ", client=" + clientName + ")"); - writeLock(); + writeLock(FSNamesystemLockMode.GLOBAL); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Pipeline not updated"); @@ -6102,7 +6106,7 @@ void updatePipeline( updatePipelineInternal(clientName, oldBlock, newBlock, newNodes, newStorageIDs, logRetryCache); } finally { - writeUnlock("updatePipeline"); + writeUnlock(FSNamesystemLockMode.GLOBAL, "updatePipeline"); } getEditLog().logSync(); LOG.info("updatePipeline(" + oldBlock.getLocalBlock() + " => " @@ -6113,7 +6117,7 @@ private void updatePipelineInternal(String clientName, ExtendedBlock oldBlock, ExtendedBlock newBlock, DatanodeID[] newNodes, String[] newStorageIDs, boolean logRetryCache) throws IOException { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.GLOBAL); // check the vadility of the block and lease holder name final INodeFile pendingFile = checkUCBlock(oldBlock, clientName); final String src = pendingFile.getFullPathName(); @@ -6409,7 +6413,7 @@ long renewDelegationToken(Token<DelegationTokenIdentifier> token) long expiryTime; checkOperation(OperationCategory.WRITE); try { - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); @@ -6426,7 +6430,7 @@ long renewDelegationToken(Token<DelegationTokenIdentifier> token) getEditLog().logRenewDelegationToken(id, expiryTime); tokenId = id.toStringStable(); } finally { - writeUnlock(operationName, getLockReportInfoSupplier(tokenId)); + writeUnlock(FSNamesystemLockMode.FS, operationName, getLockReportInfoSupplier(tokenId)); } } catch (AccessControlException ace) { final DelegationTokenIdentifier id = DFSUtil.decodeDelegationToken(token); @@ -6450,7 +6454,7 @@ void cancelDelegationToken(Token<DelegationTokenIdentifier> token) String tokenId = null; checkOperation(OperationCategory.WRITE); try { - writeLock(); + writeLock(FSNamesystemLockMode.FS); try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot cancel delegation token"); @@ -6460,7 +6464,7 @@ void cancelDelegationToken(Token<DelegationTokenIdentifier> token) getEditLog().logCancelDelegationToken(id); tokenId = id.toStringStable(); } finally { - writeUnlock(operationName, getLockReportInfoSupplier(tokenId)); + writeUnlock(FSNamesystemLockMode.FS, operationName, getLockReportInfoSupplier(tokenId)); } } catch (AccessControlException ace) { final DelegationTokenIdentifier id = DFSUtil.decodeDelegationToken(token); @@ -6535,7 +6539,7 @@ public void logExpireDelegationToken(DelegationTokenIdentifier id) { private void logReassignLease(String leaseHolder, String src, String newHolder) { - assert hasWriteLock(); + assert hasWriteLock(FSNamesystemLockMode.FS); getEditLog().logReassignLease(leaseHolder, src, newHolder); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java index dbadb908c5f..b70613301a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java @@ -971,6 +971,12 @@ public final long computeFileSizeNotIncludingLastUcBlock() { /** * Compute file size of the current file. + * + * ComputeFileSize only needs the FSLock even through it involves block. + * BlockSize only be changed by hsync, addBlock, commitBlockSynchronization, + * complete, updatePipeline and forceCompleteBlock, all these operations + * already hold the FSWriteLock. + * CompleteBlock also hold the FSWriteLock since it needs to update Quota * * @param includesLastUcBlock * If the last block is under construction, should it be included? --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org