Uh oh!
There was an error while loading. Please reload this page.
HBASE-24999 Master manages ReplicationServers - #2579
Merged
Conversation
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 22, 2020
💔 -1 overall
This message was automatically generated. |
| * server from the original instance. | ||
| */ | ||
| @InterfaceAudience.Private | ||
| public class ReplicationServerManager { |
Contributor
There was a problem hiding this comment.
Would it make sense to extend ServerManager, or maybe both this one and ServerManager could implement a common interface? It seems there is a common workflow for server managers.
Contributor
There was a problem hiding this comment.
Yes. But we want do this refator after we finished this, because ReplicationServerManager may have new featrues but ServerManager not need. It is not easy to decide that extend ServerManager or implement a common interface. Thanks.
Uh oh!
There was an error while loading. Please reload this page.
| /** | ||
| * Get the list of all the servers that are responsible for replication sink | ||
| * from the specified peer master | ||
| * @return list of server addresses or an empty list if the slave is unavailable |
Contributor
There was a problem hiding this comment.
Is it true that an empty list would be returned if we can't reach slave master? Looks like we either throw an IOException or a RemoteException.
MemberAuthor
There was a problem hiding this comment.
Yes, I ignored changing the comment.
Fixed it.
Apache-HBase
commented
Oct 26, 2020
💔 -1 overall
This message was automatically generated. |
infraio
reviewed
Oct 26, 2020
| import org.slf4j.LoggerFactory; | ||
| /** | ||
| * The ServerManager class manages info about replication servers. |
Contributor
There was a problem hiding this comment.
ServerManager => ReplicationServerManager?
infraio
reviewed
Oct 26, 2020
| * @param sl the server load on the server | ||
| * @return true if the server is recorded, otherwise, false | ||
| */ | ||
| boolean checkAndRecordNewServer(final ServerName serverName, final ServerMetrics sl) { |
infraio
reviewed
Oct 26, 2020
Uh oh!
There was an error while loading. Please reload this page.
infraio
reviewed
Oct 26, 2020
| return abortRequested.compareAndSet(false, true); | ||
| } | ||
| protected void tryReplicationServerReport(long reportStartTime, long reportEndTime) |
infraio
reviewed
Oct 26, 2020
| * @param refresh If true then master address will be read from ZK, otherwise use cached data | ||
| * @return master + port, or null if server has been stopped | ||
| */ | ||
| protected synchronized ServerName createReplicationServerStatusStub(boolean refresh) { |
infraio
commented
Oct 26, 2020
Contributor
Add master UI to show ReplicationServer? |
Apache-HBase
commented
Oct 26, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 26, 2020
🎊 +1 overall
This message was automatically generated. |
ddupg
commented
Oct 26, 2020
MemberAuthor
Thanks @infraio for reviewing. I have fixed as comments.
Do it in this PR? I was going to do it in a new PR? |
Apache-HBase
commented
Oct 26, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 26, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 26, 2020
🎊 +1 overall
This message was automatically generated. |
Contributor
Ok. Let's do it in follow-on PR. |
infraio
reviewed
Oct 27, 2020
Uh oh!
There was an error while loading. Please reload this page.
infraio
reviewed
Oct 27, 2020
| * @param refresh If true then master address will be read from ZK, otherwise use cached data | ||
| * @return master + port, or null if server has been stopped | ||
| */ | ||
| private synchronized ServerName createReplicationServerStatusStub(boolean refresh) { |
Contributor
There was a problem hiding this comment.
The return ServerName never used?
MemberAuthor
There was a problem hiding this comment.
It should be useful, let me fix it.
Apache-HBase
commented
Oct 27, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 27, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 27, 2020
🎊 +1 overall
This message was automatically generated. |
infraio
commented
Oct 28, 2020
Contributor
infraio
commented
Oct 28, 2020
Contributor
The spotbugs warnings is about HBASE-24666, not about this PR. And this PR fixed thems. "hbase-server generated 0 new + 0 unchanged - 3 fixed = 0 total (was 3)" |
infraio pushed a commit
that referenced
this pull request
Nov 9, 2020
Signed-off-by: Guanghao Zhang <zghao@apache.org>
ddupg added a commit
that referenced
this pull request
Feb 23, 2021
Signed-off-by: Guanghao Zhang <zghao@apache.org>
ddupg added a commit
that referenced
this pull request
Jul 9, 2021
Signed-off-by: Guanghao Zhang <zghao@apache.org>
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.