hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yiqun Lin (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HDFS-13418) NetworkTopology should be configurable when enable DFSNetworkTopology
Date Wed, 11 Apr 2018 09:38:00 GMT

    [ https://issues.apache.org/jira/browse/HDFS-13418?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433619#comment-16433619
] 

Yiqun Lin edited comment on HDFS-13418 at 4/11/18 9:37 AM:
-----------------------------------------------------------

Thanks [~Tao Jie] for updating the patch. Almost looks good, some comments from me:
 * Can we define a var {{DFS_NET_TOPOLOGY_IMPL_DEFAULT}} as the default NetworkTopology impl
class used in HDFS rather than hard-coded in {{DFSNetworkTopology#getInstance}}.
 * For the description of {{dfs.net.topology.impl}}, {{The implementation class of NetworkTopology
used in block placement.}} can be updated to {{The implementation class of NetworkTopology
used in HDFS.}} And then adding new one sentence to introduce the default impl class: {{By
default, the class org.apache.hadoop.hdfs.net.DFSNetworkTopology is specified and used in
block placement.}}

For the unit test, seems we don't need to extend the new NetworkTopology to test. Current
existing {{NetworkTopology}} impls are enough. I think there will be three cases we need
to cover.
 * Switch {{dfs.use.dfs.network.topology}} to true, then verify if the default DFSNetworkTopology
impl is got.
 * Switch {{dfs.use.dfs.network.topology}} to false, then verify if the default NetworkTopology
impl is got.
 * Switch {{dfs.use.dfs.network.topology}} to false, and change the value of {{net.topology.impl}},
then verify if the desired NetworkTopology impl is got.

Would you mind make a minor adjustment for addressing these three cases?


was (Author: linyiqun):
Thanks [~Tao Jie] for updating the patch. Almost looks good, some comments from me:
 * Can we define a var {{DFS_NET_TOPOLOGY_IMPL_DEFAULT}} as the default NetworkTopology impl
class used in HDFS rather than hard-coded in {{DFSNetworkTopology#getInstance}}.
 * For the description of {{dfs.net.topology.impl}}, {{The implementation class of NetworkTopology
used in block placement.}} can be updated to {{The implementation class of NetworkTopology
used in HDFS.}} And then adding new one sentence to introduce the default impl class: {{By
default, the class org.apache.hadoop.hdfs.net.DFSNetworkTopology is specified and used in
block placement.}}

For the unit test, seems we don't need to extend the new NetworkTopology to test. Current
existing {{NetworkTopology}} impls are enough. I think there will be three cases we need
to cover.
 * Switch {{dfs.use.dfs.network.topology}} to true, then verify if the default DFSNetworkTopology
impl is got.
 * Switch {{dfs.use.dfs.network.topology}} to false, then verify if the default NetworkTopology
impl is got.
 * Switch {{dfs.use.dfs.network.topology}} to false, and change the value of {{net.topology.impl}},
then verify if the desired NetworkTopology impl is got.

>  NetworkTopology should be configurable when enable DFSNetworkTopology
> ----------------------------------------------------------------------
>
>                 Key: HDFS-13418
>                 URL: https://issues.apache.org/jira/browse/HDFS-13418
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>    Affects Versions: 3.0.1
>            Reporter: Tao Jie
>            Assignee: Tao Jie
>            Priority: Major
>         Attachments: HDFS-13418.001.patch, HDFS-13418.002.patch
>
>
> In HDFS-11530 we introduce DFSNetworkTopology and in HDFS-11998 we set DFSNetworkTopology
as the default implementation.
> We still have {{net.topology.impl=org.apache.hadoop.net.NetworkTopology}} in core-site.default.
Actually this property does not effect once {{dfs.use.dfs.network.topology}} is true. 
> in {{DatanodeManager}},networkTopology is initialized as 
> {code}
>     if (useDfsNetworkTopology) {
>       networktopology = DFSNetworkTopology.getInstance(conf);
>     } else {
>       networktopology = NetworkTopology.getInstance(conf);
>     }
> {code}
> I think we should still make the NetworkTopology  configurable rather than hard code
the implementation since we may need another NetworkTopology impl.
> I am not sure if there is other consideration. Any thought? [~vagarychen] [~linyiqun]



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message