-
Couldn't load subscription status.
- Fork 9.1k
HDFS-17362. RBF: Implement RouterObserverReadConfiguredFailoverProxyProvider #6510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
hmm, change looks fine to me, but I didn't track this initiative initially, so, not sure if someone did it initially, who could be a good reviewer? seeing this class for the first time. juzz random unrelated thoughts. this line looks like a copy paste error, it should be this method is if we don't get anyone more experienced to review, then it is a +1 from me :-), do put a "Co-authored-by" line while committing for @chunyiyang so he gets his credits |
|
@ayushtkn Thanks for the review. I've updated the PR to use It seems that By the way, @chunyiyang is actually a woman. :) |
|
🎊 +1 overall
This message was automatically generated. |
...src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/RouterObserverReadProxyProvider.java
Outdated
Show resolved
Hide resolved
|
There is an issue that we cannot run unit tests in TestObserverWithRouter using |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…dFailoverProxyProvider internally
|
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @tasanuma and @chunyiyang for the contribution.
|
@simbadzina Thanks for your review! I will merge it in a couple of days. |
|
Merged. Thank you all. |
…rovider (#6510) Co-authored-by: Chunyi Yang <[email protected]> Co-authored-by: Takanobu Asanuma <[email protected]> Reviewed-by: Simbarashe Dzinamarira <[email protected]> (cherry picked from commit 5cbe52f)
Description of PR
JIRA: HDFS-17362
Currently, RouterObserverReadProxyProvider is using IPFailoverProxyProvider, while ObserverReadProxyProvider is using ConfiguredFailoverProxyProvider. If we are to align RouterObserverReadProxyProvider with ObserverReadProxyProvider, RouterObserverReadProxyProvider should internally use ConfiguredFailoverProxyProvider. Moreover, IPFailoverProxyProvider has an issue with resolving HA configurations. (For example, IPFailoverProxyProvider cannot resolve hdfs://router-service.)
This PR is co-authored by @chunyiyang and me.
How was this patch tested?
mvn test -Dtest=TestObserverWithRoutersucceeded in my local computer.For code changes: