-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-24999 Master manages ReplicationServers #2397
Conversation
…o ReplicationSourceManager (apache#2020) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…plicationSourceManager (apache#2019) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…eanOldLogs/cleanUpHFileRefs to ReplicationSource inside (apache#2064) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…ationSink Service (apache#2111) Signed-off-by: Guanghao Zhang <zghao@apache.org>
…e to a new interface ReplicationServerService (apache#2360) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
💔 -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. |
fc3de0f
to
7d0763e
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. |
String version = "0.0.0"; | ||
VersionInfo versionInfo = VersionInfoUtil.getCurrentClientVersionInfo(); | ||
if (versionInfo != null) { | ||
version = versionInfo.getVersion(); |
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.
The difference of version and versionNumber is?
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.
versionNumber
is packed by number part of version
, can't display string suffixes like "-SNAPSHOT".
} | ||
ClusterStatusProtos.ServerLoad sl = request.getLoad(); | ||
ServerName serverName = ProtobufUtil.toServerName(request.getServer()); | ||
ServerMetrics oldLoad = master.getReplicationServerManager().getLoad(serverName); |
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.
oldLoad => oldMetrics
getLoad => getServerMetrics
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.
OK
ClusterStatusProtos.ServerLoad sl = request.getLoad(); | ||
ServerName serverName = ProtobufUtil.toServerName(request.getServer()); | ||
ServerMetrics oldLoad = master.getReplicationServerManager().getLoad(serverName); | ||
ServerMetrics newLoad = |
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.
newLoad => newMetrics
* Assumes onlineServers is locked. | ||
* @return ServerName with matching hostname and port. | ||
*/ | ||
private ServerName findServerWithSameHostnamePortWithLock(final ServerName serverName) { |
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.
"WithLock"... But where is the lock?
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.
Just keep same style with ServerManager. It means that onlineServers
must be locked to access this method.
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.
But the impl didn't have a lock. And don't need to keep same style with ServerManager.
return null; | ||
} | ||
|
||
private void recordNewServerWithLock(final ServerName serverName, final ServerMetrics sl) { |
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.
Ditto.
/** | ||
* Expire the passed server. Remove it from list of online servers | ||
*/ | ||
public void expireServerWithLock(final ServerName serverName) { |
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.
Ditto.
* was started). This is used to differentiate a restarted instance of a given | ||
* server from the original instance. | ||
* <p> | ||
* If a sever is known not to be running any more, it is called dead. The dead |
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.
Not record dead server now? Don't need these comments now.
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.
OK
…and decouple ReplicationSourceManager and ReplicationSource (apache#2364) Signed-off-by: meiyi <myimeiyi@gmail.com>
apache#2077) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
8dd91f4
to
6008ffe
Compare
No description provided.