[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16568558#comment-16568558
]
Konstantin Shvachko commented on HDFS-13779:
Also reuse one of the existing retry policies for ORPP.
> 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
>Priority: Major
>
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16574081#comment-16574081
]
Erik Krogen commented on HDFS-13779:
Attaching a WIP patch which covers this and HDFS-13780, and relies on some more
refactoring in ConfiguredFailoverProxyProvider.
The idea is basically to continue allowing the CFPP layer to manage only the
Active/Standby NameNodes, and ORPP to manage the observers (then fall back to
CFPP's non-observer proxies). _Failover_ refers only to switching Active
NameNodes, _not_ switching between observers.
* Refactor CFPP a bit to create a separate {{getProxies()}} method where the
initialization can be done (solving HDFS-13780), and also where it can be
overridden by ORPP.
* On proxy initialization, also fetch all of the NameNode states. When CFPP
requests proxies, filter to only non-observers.
* Method invocation, on read methods, tries all of the NNs thought to
currently be in observer state. If any throws a StandbyException, mark it as
non-observer. Unfortunately we have no way to tell here if one of the
thought-to-be-observers has actually become Active, but in this case failover
will happen soon (at the next write request) and the situation will be fixed
(see below).
* If all observer NNs fail, or it is a write method, pass the request up to
CFPP, which will try the current Active. This may trigger failover. If so,
before picking a new node from the list of non-observers, refresh the states of
all of the NameNodes. This handles the case where one of the previous observers
is now active.
[~shv], [~chliang], let me know your thoughts on the above.
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581593#comment-16581593
]
Erik Krogen commented on HDFS-13779:
Thanks for looking [~vagarychen]!
For #1, the {{currentIndex}} only increases, then you take the modulo vs. the
current list size to get the actual index. So in the case you've discussed with
{{currentIndex = 2}} (not 3, since it is 0-indexed), before we have (caret
indicating current index):
{code}
1 2 3 4 5
^
{code}
Then we try 3 and 4, they both throw standby so we remove them, so the
remaining list is ({{currentIndex = 2}} still):
{code}
1 2 5
^
{code}
Next we try 5, is says RETRY, so we increment to get {{currentIndex = 3}}, then
we try 1 ({{3 % 3 == 0}}) as expected.
I went with {{currentIndex}} increasing and taking modulo, rather than always
maintaining an actual index, since the size of the list may change and
otherwise we would have to potentially update {{currentIndex}} every time the
list size changes (to avoid AIOOBE). Modulus gives us AIOOBE avoidance for free.
For #2, good point. For #3 / #4, yes, it needs cleanup, just a WIP for now.
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581729#comment-16581729
]
Chen Liang commented on HDFS-13779:
---
I see, so I guess the incremented {{currentIndex}} only takes effect when
{{invoke}} gets called next time, by which time {{getFilteredProxies}} will
return a different list with those proxies being removed. This is the part I
was missing, thanks for the clarification!
But this led me think about another situation: it seems to me that {{getProxy}}
can be called at any point of time. And {{getProxy}} will trigger
{{getAllProxies}} which further triggers {{refreshProxyState}}. And
{{refreshProxyState}} calls {{refreshState}} of each NN, which may change
{{isObserver}} to either true or false. In short, the value of {{isObserver}}
could change at any point of time due to {{getProxy}}. So when {{invoke}} gets
called each time, there seems no guarantee what will (not) be in the list
returned from {{getFilteredProxies}} with regard to the previous state. Is this
a valid case?
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16583012#comment-16583012
]
Erik Krogen commented on HDFS-13779:
{quote}
And {{getProxy}} will trigger {{getAllProxies}} which further triggers
{{refreshProxyState}}.
{quote}
Actually this is not quite true; {{getAllProxies}} only calls
{{refreshProxyState}} when {{proxies}} is empty (i.e., initialization). After
that it just returns the current value of {{proxies}}. However,
{{refreshProxyState}} may be called at any time by, for example,
{{performFailover}}, so your point below about {{isObserver}} is still valid:
{quote}
In short, the value of {{isObserver}} could change at any point of time So
when invoke gets called each time, there seems no guarantee what will (not) be
in the list returned from {{getFilteredProxies}} with regard to the previous
state.
{quote}
So yes, I agree that there isn't a guarantee that {{getFilteredProxies}} will
return the same thing when invoked multiple times. That's part of why, at the
top of {{invoke}}, I call {{getFilteredProxies}} a single time and then use
that value throughout the function. But the list should only change upon
failover-style actions (e.g. Observer changes to Standby) and the only thing
that happens if the list changes is to start talking to a different observer,
so I don't think it's an issue, but let me know if there's something I'm
missing.
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16583130#comment-16583130
]
Chen Liang commented on HDFS-13779:
---
Thanks for the correction [~xkrogen]! Makes sense to me. I recall one example
case I was thinking of was say there are 0, 1, 2 proxies, and currentIndex = 2.
If 1 gets removed and the list shrinks to 0, 2. Due to modulo on size, seems
currentIndex would effectively point to 2%size (2) = 0, but it was meant to
point to 2. I guess the whole point I was concerning was that since
{{currentObservers}} is based on modulo of size, but size can be changing at
any point of time, {{currentIndex}} can become not so current over time. But
seems even if {{currentIndex}} is wrong, it should be eventually fixed by the
for loop, and thanks for point out that this should only happen in failover
actions. I agree this should not be an issue.
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590979#comment-16590979
]
Erik Krogen commented on HDFS-13779:
I had some offline discussion with [~shv] and we realized that the complicated
logic I use to try to be as efficient as possible about passing CFPP only
active/standby NameNodes is not really necessary. Given how infrequently
failover occurs, the few RPCs saved are not really worth the extra complexity
of the logic and intermingling between CFPP and ORPP. I will put up a new patch
with simpler logic based on Konstantin's patch for HDFS-13782.
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16594397#comment-16594397
]
genericqa commented on HDFS-13779:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
27s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
|| || || || {color:brown} HDFS-12943 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 2m
3s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
27s{color} | {color:green} HDFS-12943 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
43s{color} | {color:green} HDFS-12943 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
8s{color} | {color:green} HDFS-12943 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m
55s{color} | {color:green} HDFS-12943 passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
14m 39s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
31s{color} | {color:green} HDFS-12943 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
18s{color} | {color:green} HDFS-12943 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
10s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
42s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 3m
9s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 3m
9s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 59s{color} | {color:orange} hadoop-hdfs-project: The patch generated 21 new
+ 11 unchanged - 7 fixed = 32 total (was 18) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 0s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
10s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
28s{color} | {color:red} hadoop-hdfs-project_hadoop-hdfs-client generated 10
new + 0 unchanged - 0 fixed = 10 total (was 0) {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
51s{color} | {color:red} hadoop-hdfs-project_hadoop-hdfs generated 2 new + 1
unchanged - 0 fixed = 3 total (was 1) {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 1m
36s{color} | {color:green} hadoop-hdfs-client in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red}103m 2s{color}
| {color:red} hadoop-hdfs in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
36s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}180m 38s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hdfs.TestDFSInotifyEventInputStreamKerberized |
| | hadoop.hdfs.TestDFSStripedOutputStreamWithFailureWithRandomECPolicy |
| | hadoop.hdfs.server.namenode.ha.TestObserverNode |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:9b55946 |
| JIRA Issue | HDFS-13779 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12937344/HDFS-13779-HDFS-12943.000.patch
|
| Option
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16595716#comment-16595716
]
Chen Liang commented on HDFS-13779:
---
Thanks for the patch [~xkrogen]! LGTM overall, only some minor comments:
# {{ObserverReadProxyProvider#invoke}}, change {{observerCount}} to some like
{{failedObserverCount}} ?
# {{ObserverReadProxyProvider#changeProxy}}, seems we can reduce the scope of
locking to after the {{if (currentProxy != initial)}} check, so that it does
not need to grab the lock just for this check and then return. May not be that
necessary, I'm okay with either way.
# {{CachingHAProxyFactory}}, move the initialization of {{cache}} into
constructor?
# More of a question, do we really need to have {{CachingHAProxyFactory}}?
because for now it seems to me that even without caching,
{{createProxyIfNeeded}} is doing the job that only when proxy is null it will
create proxy. Am I missing something here?
> 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.000.patch,
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16595743#comment-16595743
]
Erik Krogen commented on HDFS-13779:
Thanks for the review [~vagarychen]!
# Sure, it is only used for logging how many failed, so sounds good to me.
# The lock is needed before the if-check, else the {{currentProxy}} could have
changed after checking but before locking. We could add a double check (once
before lock, once after lock), but since {{changeProxy}} is only called when an
observer has a failure I don't think we need to try too hard to optimize its
synchronization. {{getCurrentProxy}}, on the other hand, is called on every RPC
which is why I wanted to make sure it wasn't synchronized.
# Sure, seems good
# {{createProxyIfNeeded}} will only create it once, yes, but the
{{NNProxyInfo}} instances within {{ObserverReadProxyProvider}} are not the same
as the ones within {{ConfiguredFailoverProxyProvider}} (it is delegation, not
inheritance), so the proxy will still be created twice if
{{CachingHAProxyFactory}} is not used. I am open to cleaner solutions to avoid
the redundant proxy creation, though.
> 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.000.patch,
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16597827#comment-16597827
]
Erik Krogen commented on HDFS-13779:
Just attached v001 patch addressing the review comments as above, filling out
the remaining Javadoc, fixing TestObserverNode, and a few other minor fixes.
> 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.000.patch,
> HDFS-13779-HDFS-12943.001.patch, 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16597855#comment-16597855
]
Konstantin Shvachko commented on HDFS-13779:
Looks good, Erik. Two comments from me:
# In ORPP you use raw types: "References to generic type
AbstractNNFailoverProxyProvider should be parameterized"
# Unused imports (2) in {{TestObserverReadProxyProvider}}.
# For {{CachingHAProxyFactory}} I don't think we achieve much here. It still
stores the the Proxy twice - one in the {{backingFactory}} and another in the
cache. If want to cache proxies we should do it in the RPC engine globally for
all proxies. I suggest we skip this optimization for now.
# Also {{TestObserverNode}} fails for me with your patch. I can debug if it is
only on my box.
> 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.000.patch,
> HDFS-13779-HDFS-12943.001.patch, 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16598050#comment-16598050
]
Erik Krogen commented on HDFS-13779:
Looks like me and Konstantin hit a race condition :) Comments 2 and 4 were
fixed in v001 patch.
I just attached v002 which addresses 1 and 3. Konstantin and I discussed
offline and realized that the caching done in {{ipc.Client}} will make the two
ProxyProviders share a single connection per NameNode regardless of the caching
of the proxies themselves, so I agree that the {{CachingHAProxyFactory}} is not
necessary. This also makes my changes to the constructor (accepting a class and
using reflection, instead of directly accepting an instance) unnecessary.
> 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.000.patch,
> HDFS-13779-HDFS-12943.001.patch, HDFS-13779-HDFS-12943.002.patch,
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16598123#comment-16598123
]
Konstantin Shvachko commented on HDFS-13779:
Erik, this looks good. Two minor nits from my IDE
# Unused import of Constructor in ORPP
# {{TestObserverReadProxyProvider.proxyFactoryInvocations}} is used for
incrementing it. You might want to get rid of it, or check its value somewhere.
I recommend committing this without waiting for Jenkins to unblock other jiras.
We should fix warnings if there will be any later on.
> 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.000.patch,
> HDFS-13779-HDFS-12943.001.patch, HDFS-13779-HDFS-12943.002.patch,
> 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
[jira] [Commented] (HDFS-13779) Implement performFailover logic for ObserverReadProxyProvider.
[
https://issues.apache.org/jira/browse/HDFS-13779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16598955#comment-16598955
]
Erik Krogen commented on HDFS-13779:
Good catches, Konstantin! I should have taken a closer look after my v001 ->
v002 revisions. I also removed the {{throws IOException}} clause from the
{{ObserverReadProxyProvider}} constructors, as it is no longer applicable.
Attaching v003 patch, which I also just committed to HDFS-12943 branch. Thanks
for the help [~shv] and [~vagarychen]!
> 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
> Fix For: HDFS-12943
>
> Attachments: HDFS-13779-HDFS-12943.000.patch,
> HDFS-13779-HDFS-12943.001.patch, HDFS-13779-HDFS-12943.002.patch,
> HDFS-13779-HDFS-12943.003.patch, 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
