HBASE-24684 Fetch ReplicationSink servers list from HMaster instead o… #2077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
infraio
reviewed
Jul 16, 2020
| * @return list of region server addresses or an empty list if the slave is unavailable | ||
| */ | ||
| protected static List<ServerName> fetchSlavesAddresses(ZKWatcher zkw) |
There was a problem hiding this comment.
Should be compatible with the old ZK impl. Cannot remove them directly?
There was a problem hiding this comment.
OK，try new impl firstly and old ZK impl secondly, is that OK?
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
ddupg
force-pushed
the
HBASE-24684
branch
3 times, most recently
from
000e874 to
069337f
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
1 similar comment
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
wchevreuil
reviewed
Jul 21, 2020
There was a problem hiding this comment.
Since we are relying on the periodic chore run to update the list of available sinks, we might need to handle possible failure if the cached sink is not available anymore. Maybe we should add an extra check to re-fetch the sinks at HBaseInterClusterReplicationEndpoint.replicateEntries, in case the current one fails?
| try (Admin admin = getPeerConnection().getAdmin()) { | ||
| String version = admin.getClusterMetrics().getHBaseVersion(); | ||
| LOG.info("Peer cluster version is {} for peer {}", version, ctx.getPeerId()); | ||
| if (Integer.parseInt(version.split("\\.")[0]) >= 3) { |
There was a problem hiding this comment.
Would it be interesting to also add a config flag allowing 3+ versions to optionally still use ZK?
There was a problem hiding this comment.
Thanks @wchevreuil for reviewing.
ReplicationSinkManager.chooseSinks will be called to re-fetch the sinks when HBaseInterClusterReplicationEndpoint.replicate catches ConnectException or UnknownHostException, It's same as before.
And I've added a config "hbase.replication.fetch.servers.usezk" to optionally still use ZK impl.
There was a problem hiding this comment.
ReplicationSinkManager.chooseSinks will be called to re-fetch the sinks when HBaseInterClusterReplicationEndpoint.replicate catches ConnectException or UnknownHostException, It's same as before.
Yeah, haven't looked at the outer catch from replicate. Another thought them is if we really need the new chore? I wonder how effective it would really be to avoid trying stale sinks.
There was a problem hiding this comment.
Another thought them is if we really need the new chore? I wonder how effective it would really be to avoid trying stale sinks.
Thank @wchevreuil for thinking about this.
I had a thought, there is some differences between regionServers stored in HBaseReplicationEndpoint and sinks stored in ReplicationSinkManager. The latter is a random part of the former. The new chore can remove abandoned servers and discover new servers. If there is no such chore, new servers can't be sinks unless exceptions happen due to RSs crash.
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…plicationSourceManager (apache#2019) Signed-off-by: Wellington Chevreuil <[email protected]>
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
asfgit
pushed a commit
that referenced
this pull request
Sep 23, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Sep 24, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
infraio
pushed a commit
that referenced
this pull request
Sep 26, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Sep 29, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Oct 22, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
infraio
pushed a commit
that referenced
this pull request
Nov 9, 2020
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
ddupg
added a commit
that referenced
this pull request
Feb 23, 2021
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
ddupg
added a commit
that referenced
this pull request
Jul 9, 2021
#2077) Signed-off-by: Wellington Chevreuil <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…f ZooKeeper