hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uma Maheswara Rao G (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-2928) HA: ConfiguredFailoverProxyProvider should not create a NameNode proxy with an underlying retry proxy
Date Thu, 16 Feb 2012 01:35:00 GMT

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

Uma Maheswara Rao G commented on HDFS-2928:
-------------------------------------------

Aaron, Thanks a lot for the review!

{quote}
Let's add a default impl for createNNProxyWithNamenodeProtocol as well, so we don't have to
modify NameNodeConnector at all.
{quote}

 Added the default api.

{quote}
I see that you addressed my comment about setting up retries in DFSUtil, but then you made
ClientNamenodeProtocolTranslatorPB call DFSUtil, which wasn't my intention. I see why you
did this (several other places call the ClientNamenodeProtocolTranslatorPB constructor) but
I actually think that ideally the translators wouldn't be responsible for setting up  the
underlying proxy at all. i.e. the only translator ctor should be one which takes an already-constructed
proxy as a parameter. This is a separate refactor, though, and so can be punted to another
JIRA, if you want.
{quote}
Yes, that should not be ralated to this JIRA. filed HDFS-2959

{quote}
Let's implement ClientNamenodeProtocolTranslatorPB(InetSocketAddress, Configuration, UserGroupInformation)
in terms of ClientNamenodeProtocolTranslatorPB(ClientNamenodeProtocolPB), rather than assigning
rpcProxy in both constructors.
{quote}
can do it. Done.

{quote}
Rather than return early in the case of "!withRetries" in both createNNProxyWithNamenodeProtocol
and createNNProxyWithClientProtocol, let's just only set up the retryProxy in the case of
"withRetries" being true. Seems a little clearer to me.
{quote}
done.

{quote}
I see what you mean about the difficulty of testing this change, since it's mostly just a
refactor. The only test I can imagine which would test actual functionality, rather than code
structure, would be to somehow verify that the create() call isn't retried without failing
over in the case of AlreadyBeingCreatedExceptions. But, that also seems difficult to write
a test for and not really worth it. The test as implemented seems fragile and is really just
asserting current code structure, so I think we should just rip it out. Running the existing
tests as you did should be sufficient to test this change. Sorry I misdirected you there.
Removing that also means we can scrap the changes in NamenodeProtocolTranslatorPB.
{quote}
Thanks for understanding :-). Tested with existing tests.
Observed few failures due to HDFS-2955. Remaining all are passing.

Thanks
Uma





                
> HA: ConfiguredFailoverProxyProvider should not create a NameNode proxy with an underlying
retry proxy
> -----------------------------------------------------------------------------------------------------
>
>                 Key: HDFS-2928
>                 URL: https://issues.apache.org/jira/browse/HDFS-2928
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ha, hdfs client
>    Affects Versions: HA branch (HDFS-1623)
>            Reporter: Aaron T. Myers
>            Assignee: Uma Maheswara Rao G
>            Priority: Minor
>         Attachments: HDFS-2928.patch, HDFS-2928.patch, HDFS-2928.patch
>
>
> This is to address the following TODO in ConfiguredFailoverProxyProvider:
> {quote}
> // TODO(HA): This will create a NN proxy with an underlying retry
> // proxy. We don't want this.
> {quote}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message