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

Erik Krogen commented on HDFS-13848:
------------------------------------

Cool, thanks for the quick turnaround [~shv]! Overall things look great with 
v004; I just have a few nits:
* In the ConfiguredFailoverProxyProvider constructor, the {{addressKey}} 
parameter is ignored, we need to have the ability to pass this into 
{{getProxyAddresses}} to avoid a regression
* Now that {{createProxyIfNeeded()}} mutates its input, rather than creating a 
new object, should it be changed to have a void return?
* Is {{getProxyAddresses()}} really the right place for the call to 
{{HAUtilClient.cloneDelegationTokenForLogicalUri()}}? The method name implies 
to me that it should just be fetching addresses, not mutating the UGI. Then the 
method could be made static (just have {{conf}} passed in as an argument). This 
is just a small semantics issue so I don't mind if you don't change it.


> 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-003.patch, HDFS-13848-004.patch, 
> 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