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

Chen Liang commented on HDFS-13779:
-----------------------------------

Thanks for the patch [~xkrogen], looking good to me overall, some comments:
1. I was too sure about the comment
{code}
// Only move the index if a new node should be tried.
// Don't move the index if the node will be removed from the list
// of observers, else one node will be skipped.
{code}
For example, it seems to me that, say there are 5 proxies and {{currentIndex}} 
is currently 3. If no succeed, the for loop will traverse all 5 in the order of 
3, 4, 5, 1, 2. Say 3, 4 both throw {{isStandbyException}} and removed from 
list, then for 5 the policy returns RETRY. Then {{currentIndex}} gets 
incremented by 1 to 4? Seems setting to 1 in this case makes more sense?
2. {{ObserverReadProxyProvider#getProxy}} the current code inserts a ',' 
between proxy strings, seems this is removed in the patch. Should we keep it?
3. need adding some new lines in class {{NameNodeProxyInfo}} 
4. would be great if we can add a unit test.

> Implement performFailover logic for ObserverReadProxyProvider.
> --------------------------------------------------------------
>
>                 Key: HDFS-13779
>                 URL: https://issues.apache.org/jira/browse/HDFS-13779
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs-client
>            Reporter: Konstantin Shvachko
>            Assignee: Erik Krogen
>            Priority: Major
>         Attachments: HDFS-13779-HDFS-12943.WIP00.patch
>
>
> Currently {{ObserverReadProxyProvider}} inherits {{performFailover()}} method 
> from {{ConfiguredFailoverProxyProvider}}, which simply increments the index 
> and switches over to another NameNode. The logic for ORPP should be smart 
> enough to choose another observer, otherwise it can switch to a SBN, where 
> reads are disallowed, or to an ANN, which defeats the purpose of reads from 
> standby.
> This was discussed in HDFS-12976.



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

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

Reply via email to