[
https://issues.apache.org/jira/browse/HDFS-11530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15957343#comment-15957343
]
Chen Liang edited comment on HDFS-11530 at 4/5/17 6:22 PM:
-----------------------------------------------------------
Hi [~linyiqun], after revisiting the code, I have a question about the change
in the earlier patch.
I think when {{getInstance()}} is called, it is always a fresh network topology
instance being created, this applies to both the old and the new
{{NetworkTopology}} classes. Given this, I wonder, for classes where the new
{{chooseRandom}} is *not* going to be called, is it necessary at all to switch
to new topology class ?
More specifically, in the changes from v2 patch, {{Dispatcher}} switched to the
new topology class, but they never called the new {{chooseRandom}} method. So
in this case, is it really needed to switch to the new topology at all?
I digged down to this when I was looking at {{TestMover}} fail, which failed
because it adds {{DatanodeInfo}}, instead of {{DatanodeDescriptor}} to the
topology. However {{DFSNetworkTopology}} requires a {{DatanodeDescriptor}} as
the leaf nodes, because this class is where storage type info lives. This
appears to me that certain classes run with a slightly more abstracted datanode
representation that is not {{DatanodeDescriptor}}, in which case a more
detailed {{DFSNetworkTopology}} class might not be necessary.
An alternative way is to still replacing with {{DFSNetworkTopology}}, but
within {{DFSNetworkTopology}}, it checks if it is {{DatanodeInfo}} instead, if
yes, then it does not do anything fancy about storage type info, but pretty
much just stick to whatever it was in {{NetworkTopology}}.
Any comments? cc. [~arpitagarwal].
was (Author: vagarychen):
Hi [~linyiqun], after revisiting the code, I have a question about the change
in the earlier patch.
I think when {{getInstance()}} is called, it is always a fresh network topology
instance being created, this applies to both the old and the new
{{NetworkTopology}} classes. Given this, I wonder, for classes where the new
{{chooseRandom}} is *not* going to be called, is it necessary at all to switch
to new topology class ?
More specifically, in the changes from v2 patch, {{DatanodeManager}} and
{{Dispatcher}} switched to the new topology class, but they never called the
new {{chooseRandom}} method. So in this case, is it really needed to switch to
the new topology at all? Appears to me that only the changes to
{{BlockPlacementPolicyDefault}} are required.
I digged down to this when I was looking at {{TestMover}} fail, which failed
because it adds {{DatanodeInfo}}, instead of {{DatanodeDescriptor}} to the
topology. However {{DFSNetworkTopology}} requires a {{DatanodeDescriptor}} as
the leaf nodes, because this class is where storage type info lives. This
appears to me that certain classes run with a slightly more abstracted datanode
representation that is not {{DatanodeDescriptor}}, in which case a more
detailed {{DFSNetworkTopology}} class might not be necessary.
An alternative way is to still replacing with {{DFSNetworkTopology}}, but
within {{DFSNetworkTopology}}, it checks if it is {{DatanodeInfo}} instead, if
yes, then it does not do anything fancy about storage type info, but pretty
much just stick to whatever it was in {{NetworkTopology}}.
Any comments? cc. [~arpitagarwal].
> Use HDFS specific network topology to choose datanode in
> BlockPlacementPolicyDefault
> ------------------------------------------------------------------------------------
>
> Key: HDFS-11530
> URL: https://issues.apache.org/jira/browse/HDFS-11530
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: 3.0.0-alpha2
> Reporter: Yiqun Lin
> Assignee: Yiqun Lin
> Attachments: HDFS-11530.001.patch, HDFS-11530.002.patch,
> HDFS-11530.003.patch, HDFS-11530.004.patch
>
>
> The work for {{chooseRandomWithStorageType}} has been merged in HDFS-11482.
> But this method is contained in new topology {{DFSNetworkTopology}} which is
> specified for HDFS. We should update this and let
> {{BlockPlacementPolicyDefault}} use the new way since the original way is
> inefficient.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
