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

Chen Liang commented on HDFS-13848:
-----------------------------------

v002 patch LGTM overall, only some minor comments:
- In FailoverProxyProvider, ProxyInfo could potentially return an uninitialized 
address. Also, if initialized with {{ProxyInfo(InetSocketAddress address)}}, 
proxy could be null, in which case seems NPE if later on a {{getString}} gets 
called. Maybe better add some Precoditions check or throw RuntimeException?
- In ConfiguredFailoverProxyProvider constructor, {{this.factory = factory}} 
seems unnecessary, because super(conf, uri, xfact, factory) should have handled 
this.
- In IPFailoverProxyProvider, the initialization of nnProxyInfo is moved from 
getProxy() to initialization of IPFailoverProxyProvider. Is there a particular 
reason this is more preferred?


> Refactor NameNode failover proxy providers
> ------------------------------------------
>
>                 Key: HDFS-13848
>                 URL: https://issues.apache.org/jira/browse/HDFS-13848
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ha, hdfs-client
>    Affects Versions: 2.7.5
>            Reporter: Konstantin Shvachko
>            Assignee: Konstantin Shvachko
>            Priority: Major
>         Attachments: HDFS-13848.002.patch, HDFS-13848.patch
>
>
> Looking at NN failover proxy providers in the context of HDFS-13782 I noticed 
> that {{ConfiguredFailoverProxyProvider}} and {{IPFailoverProxyProvider}} have 
> a lot of common logic. We can move this common logic into 
> {{AbstractNNFailoverProxyProvider}}, which simplifies things a lot.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to