[ https://issues.apache.org/jira/browse/HDFS-16819?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Haiyang Hu updated HDFS-16819: ------------------------------ Description: In FsDatasetImpl#createTemporary Line_1840 the writeLock here seems useless. The readLock is already held in volumeMap.get(). From the code logic point of view, the writeLock here maybe to remove {code:java} public ReplicaHandler createTemporary(StorageType storageType, String storageId, ExtendedBlock b, boolean isTransfer) throws IOException { long startTimeMs = Time.monotonicNow(); long writerStopTimeoutMs = datanode.getDnConf().getXceiverStopTimeout(); ReplicaInfo lastFoundReplicaInfo = null; boolean isInPipeline = false; do { try (AutoCloseableLock lock = lockManager.writeLock(LockLevel.BLOCK_POOl, b.getBlockPoolId())) { //the writeLock here maybe to remove ReplicaInfo currentReplicaInfo = volumeMap.get(b.getBlockPoolId(), b.getBlockId()); if (currentReplicaInfo == lastFoundReplicaInfo) { break; } else { isInPipeline = currentReplicaInfo.getState() == ReplicaState.TEMPORARY || currentReplicaInfo.getState() == ReplicaState.RBW; /* * If the current block is not PROVIDED and old, reject. * else If transfer request, then accept it. * else if state is not RBW/Temporary, then reject * If current block is PROVIDED, ignore the replica. */ if (((currentReplicaInfo.getGenerationStamp() >= b .getGenerationStamp()) || (!isTransfer && !isInPipeline)) && !isReplicaProvided(currentReplicaInfo)) { throw new ReplicaAlreadyExistsException("Block " + b + " already exists in state " + currentReplicaInfo.getState() + " and thus cannot be created."); } lastFoundReplicaInfo = currentReplicaInfo; } } {code} was: In FsDatasetImpl#createTemporary Line_1840 the writeLock here seems useless. The readLock is already held in volumeMap.get(). From the code logic point of view, the writeLock here maybe to remove {code:java} // Some comments here public ReplicaHandler createTemporary(StorageType storageType, String storageId, ExtendedBlock b, boolean isTransfer) throws IOException { long startTimeMs = Time.monotonicNow(); long writerStopTimeoutMs = datanode.getDnConf().getXceiverStopTimeout(); ReplicaInfo lastFoundReplicaInfo = null; boolean isInPipeline = false; do { try (AutoCloseableLock lock = lockManager.writeLock(LockLevel.BLOCK_POOl, b.getBlockPoolId())) { //the writeLock here maybe to remove. ReplicaInfo currentReplicaInfo = volumeMap.get(b.getBlockPoolId(), b.getBlockId()); if (currentReplicaInfo == lastFoundReplicaInfo) { break; } else { isInPipeline = currentReplicaInfo.getState() == ReplicaState.TEMPORARY || currentReplicaInfo.getState() == ReplicaState.RBW; /* * If the current block is not PROVIDED and old, reject. * else If transfer request, then accept it. * else if state is not RBW/Temporary, then reject * If current block is PROVIDED, ignore the replica. */ if (((currentReplicaInfo.getGenerationStamp() >= b .getGenerationStamp()) || (!isTransfer && !isInPipeline)) && !isReplicaProvided(currentReplicaInfo)) { throw new ReplicaAlreadyExistsException("Block " + b + " already exists in state " + currentReplicaInfo.getState() + " and thus cannot be created."); } lastFoundReplicaInfo = currentReplicaInfo; } {code} > Remove the redundant write lock in FsDatasetImpl#createTemporary > ------------------------------------------------------------------ > > Key: HDFS-16819 > URL: https://issues.apache.org/jira/browse/HDFS-16819 > Project: Hadoop HDFS > Issue Type: Improvement > Reporter: Haiyang Hu > Assignee: Haiyang Hu > Priority: Major > > In FsDatasetImpl#createTemporary Line_1840 the writeLock here seems useless. > The readLock is already held in volumeMap.get(). From the code logic point > of view, the writeLock here maybe to remove > {code:java} > public ReplicaHandler createTemporary(StorageType storageType, > String storageId, ExtendedBlock b, boolean isTransfer) > throws IOException { > long startTimeMs = Time.monotonicNow(); > long writerStopTimeoutMs = datanode.getDnConf().getXceiverStopTimeout(); > ReplicaInfo lastFoundReplicaInfo = null; > boolean isInPipeline = false; > do { > try (AutoCloseableLock lock = lockManager.writeLock(LockLevel.BLOCK_POOl, > b.getBlockPoolId())) { //the writeLock here maybe to remove > ReplicaInfo currentReplicaInfo = > volumeMap.get(b.getBlockPoolId(), b.getBlockId()); > if (currentReplicaInfo == lastFoundReplicaInfo) { > break; > } else { > isInPipeline = currentReplicaInfo.getState() == ReplicaState.TEMPORARY > || currentReplicaInfo.getState() == ReplicaState.RBW; > /* > * If the current block is not PROVIDED and old, reject. > * else If transfer request, then accept it. > * else if state is not RBW/Temporary, then reject > * If current block is PROVIDED, ignore the replica. > */ > if (((currentReplicaInfo.getGenerationStamp() >= b > .getGenerationStamp()) || (!isTransfer && !isInPipeline)) > && !isReplicaProvided(currentReplicaInfo)) { > throw new ReplicaAlreadyExistsException("Block " + b > + " already exists in state " + currentReplicaInfo.getState() > + " and thus cannot be created."); > } > lastFoundReplicaInfo = currentReplicaInfo; > } > } > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org