[
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
