[
https://issues.apache.org/jira/browse/HDFS-15160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17123634#comment-17123634
]
Stephen O'Donnell commented on HDFS-15160:
------------------------------------------
Looking at this further, I am not even sure if we need to synchronize around
the gen stamp update in getBlockLocalPathInfo.
It looks like the variable block is a block object manufactured by the client
and passed to the DN, so the code:
{code}
synchronized(replica) {
if (replica.getGenerationStamp() < block.getGenerationStamp()) {
throw new IOException(
"Replica generation stamp < block generation stamp, block="
+ block + ", replica=" + replica);
} else if (replica.getGenerationStamp() > block.getGenerationStamp()) {
block.setGenerationStamp(replica.getGenerationStamp());
}
}
{code}
Is simply not trusting the value passed by the client. If the client claims to
have a higher gen stamp, we throw an error, if the client passes a lower
genstamp, we update the local copy to be the current genstamp. There is other
existing code which perform a similar check with no lock at all, so this should
be safe.
Searching for usages for block.setGenerationStamp in Intellij, I believe most
genstamp updates in the DN are of that nature.
DataStreamer, DFSStripedOutputStream, DataXceiver, BlockSender and
BlockReceiver all update genstamps outside of the lock, but that is existing
code untouched by this patch.
In DataNode.transferReplicaForPipelineRecovery() the local variable "b" has its
genstamp updated, but again I believe this is a block object passed from the
client, and it is getting updated to the value stored in the replica map.
In Datanode.updateReplicaUnderRecovery, its also a new block getting the
genstamp copied from the stored block.
FSDatasetImpl.getBlockLocalPathInfo I covered above.
After reviewing this again, I believe this patch is good and any of the
genstamp updates are safe, but I would be happier if I saw this used on a busy
cluster for a while to be sure no problems appear.
{quote}
Besides, FsDatasetImpl#getBlockReports could be changed to read lock in my
opinion, What do you think？
{quote}
This is already a read lock in the latest patch.
> ReplicaMap, Disk Balancer, Directory Scanner and various FsDatasetImpl
> methods should use datanode readlock
> -----------------------------------------------------------------------------------------------------------
>
> Key: HDFS-15160
> URL: https://issues.apache.org/jira/browse/HDFS-15160
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: datanode
> Affects Versions: 3.3.0
> Reporter: Stephen O'Donnell
> Assignee: Stephen O'Donnell
> Priority: Major
> Attachments: HDFS-15160.001.patch, HDFS-15160.002.patch,
> HDFS-15160.003.patch, HDFS-15160.004.patch, HDFS-15160.005.patch,
> HDFS-15160.006.patch, image-2020-04-10-17-18-08-128.png,
> image-2020-04-10-17-18-55-938.png
>
>
> Now we have HDFS-15150, we can start to move some DN operations to use the
> read lock rather than the write lock to improve concurrence. The first step
> is to make the changes to ReplicaMap, as many other methods make calls to it.
> This Jira switches read operations against the volume map to use the readLock
> rather than the write lock.
> Additionally, some methods make a call to replicaMap.replicas() (eg
> getBlockReports, getFinalizedBlocks, deepCopyReplica) and only use the result
> in a read only fashion, so they can also be switched to using a readLock.
> Next is the directory scanner and disk balancer, which only require a read
> lock.
> Finally (for this Jira) are various "low hanging fruit" items in BlockSender
> and fsdatasetImpl where is it fairly obvious they only need a read lock.
> For now, I have avoided changing anything which looks too risky, as I think
> its better to do any larger refactoring or risky changes each in their own
> Jira.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
