[ https://issues.apache.org/jira/browse/HDFS-1093?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886948#action_12886948 ]
Suresh Srinivas commented on HDFS-1093: --------------------------------------- Comments: # General - try-catch block is not indented right in many places. This seems like a good idea to keep the code changes to minimum. Is it a good idea to leave this code as it is, which violates the coding standard? Should we do formatting in another jira or this jira? # FSNamesystem.java #* ReadWriteLock import not used #* make fsLock final and initialize it in declaration? #* getBlockLocations() - should we aquire writeLock if doAccessTime is true and readLock otherwise? #* Move check for safemode outside the lock for all the methods #* setTimes() move isAccessTimeSupported() call out side the lock? #* CommitBlockSynchronization() - minor: declare String src where it is needed. #* ProcessReport - move declaring now() and first log outside synchronized section. Similar pattern of code that can moved outside lock in methods createSymLinkInternal, startFileInternal, abandonBlock, completeFileInternal, renameToInternal, mkdirsInternal, commitBlockSynchronization, registerDatanode, startCheckpoint, endCheckpoint, updatePipeline, getDecommissioningNodes #* getDatanodeListForReport - move boolean and HashMap out of read lock. #* getSafeModeTip - readLock instead of writeLock? # BlockManager.java #* chooseUnderReplicatedBlocks - should we grab readLock instead of writeLock? #* removeStoredBlock - assert to be replaced by grab writeLock #* invalidateWorkForOneNode - safemode check outside the lock # DecommissionManager.java - readLock instead of writeLock? # FSDirectory.java #* ReadWriteLock import not used #* bLock and cond must be final variables and initialize in declaration? #* The new lock introduced replaces previous synchronization on FSDirectory object and FSDirectory.root object #* FSDirectory.root synchronization is still used in BackupStorage.loadCheckPoint() #* FSPermissionChecker.checkPermission is synchronized on INodeDirectory, which happens to be FSDirectory.root. This is an ugly code to change to use the lock given the method could get any other INodeDirectory. > Improve namenode scalability by splitting the FSNamesystem synchronized > section in a read/write lock > ---------------------------------------------------------------------------------------------------- > > Key: HDFS-1093 > URL: https://issues.apache.org/jira/browse/HDFS-1093 > Project: Hadoop HDFS > Issue Type: Improvement > Components: name-node > Reporter: dhruba borthakur > Assignee: dhruba borthakur > Attachments: NNreadwriteLock.txt, NNreadwriteLock_trunk_1.txt, > NNreadwriteLock_trunk_2.txt > > > Most critical data structures in the NameNode (NN) are protected by a > syncronized methods in the FSNamesystem class. This essentially makes > critical code paths in the NN single-threaded. However, a large percentage of > the NN calls are listStatus, getBlockLocations, etc which do not change > internal data structures at all, these are read-only calls. If we change the > FSNamesystem lock to a read/write lock, many of the above operations can > occur in parallel, thus improving the scalability of the NN. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.