[
https://issues.apache.org/jira/browse/HDFS-6022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13918484#comment-13918484
]
Colin Patrick McCabe commented on HDFS-6022:
--------------------------------------------
Hi Jack. Please only attach a patch for trunk. We can backport to other
branches later if it seems like a good idea.
Also see https://wiki.apache.org/hadoop/CodeReviewChecklist for things like
code style... it looks like you've got some hard tabs mixed in there, which we
don't use.
{code}
+ /* this is a thread that takes care of deadNodes maintenance
+ * every 5 seconds we check to see if we can open a connection to a
datanode
+ * on in DeadNodes, if we can, we remove it
+ * if we can not, we go to the next node, and check again, until we
+ * traverse the whole list.
+ */
+ public static class DeadNodeVerifier extends Thread {
{code}
This seems unnecessary to me. We don't need to be constantly trying to connect
to dead nodes. Just expire them from a Guava cache after a few minutes, like
we do in {{DFSOutputStream}} now.
Finally, we'd want this information to be part of {{ClientContext}}--
definitely not a static variable inside {{DFSClient}}.
> Moving deadNodes from being thread local. Improving dead datanode handling in
> DFSClient
> ----------------------------------------------------------------------------------------
>
> Key: HDFS-6022
> URL: https://issues.apache.org/jira/browse/HDFS-6022
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: hdfs-client
> Affects Versions: 3.0.0, 0.23.9, 0.23.10, 2.2.0, 2.3.0
> Reporter: Jack Levin
> Labels: patch
> Fix For: 3.0.0, 2.3.0
>
> Attachments: HADOOP-6022.patch
>
> Original Estimate: 0h
> Remaining Estimate: 0h
>
> This patch solves an issue of deadNodes list being thread local. deadNodes
> list is created by DFSClient when some problems with write/reading, or
> contacting datanode exist. The problem is that deadNodes is not visible to
> other DFSInputStream threads, hence every DFSInputStream ends up building its
> own deadNodes. This affect performance of DFSClient to a large degree
> especially when a datanode goes completely offline (there is a tcp connect
> delay experienced by all DFSInputStream threads affecting performance of the
> whole cluster).
> This patch moves deadNodes to be global in DFSClient class so that as soon as
> a single DFSInputStream thread reports a dead datanode, all other
> DFSInputStream threads are informed, negating the need to create their own
> independent lists (concurrent Map really).
> Further, a global deadNodes health check manager thread (DeadNodeVerifier) is
> created to verify all dead datanodes every 5 seconds, and remove the same
> list as soon as it is up. That thread under normal conditions (deadNodes
> empty) would be sleeping. If deadNodes is not empty, the thread will attempt
> to open tcp connection every 5 seconds to affected datanodes.
> This patch has a test (TestDFSClientDeadNodes) that is quite simple, since
> the deadNodes creation is not affected by the patch, we only test datanode
> removal from deadNodes by the health check manager thread. Test will create
> a file in dfs minicluster, read from the same file rapidly, cause datanode to
> restart, and test is the health check manager thread does the right thing,
> removing the alive datanode from the global deadNodes list.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
