You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/03/29 21:58:34 UTC
[GitHub] [ozone] ghuangups opened a new pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
ghuangups opened a new pull request #2094:
URL: https://github.com/apache/ozone/pull/2094
… information.
## What changes were proposed in this pull request?
"ozone admin scm roles" command currently returns scm role info like this: [scm1:9865, scm2:9865, scm3:9865]. This change is to adding: LEADER and FOLLOWER info in the return. The new return result would look like this: [scm3:9865:FOLLOWER, scm2:9865:FOLLOWER, scm1:9865:LEADER]
## What is the link to the Apache JIRA
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
https://issues.apache.org/jira/browse/HDDS-5022
## How was this patch tested?
Manually tested locally by creating a SCM HA cluster with 3 SCM nodes and executed command: ozone admin scm roles from one of the SCM node. With this change, the command result was: [scm3:9865:FOLLOWER, scm2:9865:FOLLOWER, scm1:9865:LEADER].
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ghuangups closed pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
Posted by GitBox <gi...@apache.org>.
ghuangups closed pull request #2094:
URL: https://github.com/apache/ozone/pull/2094
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] GlenGeng commented on a change in pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #2094:
URL: https://github.com/apache/ozone/pull/2094#discussion_r603749073
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -225,7 +225,7 @@ public void stop() throws IOException {
@Override
public List<String> getRatisRoles() {
return division.getGroup().getPeers().stream()
- .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
+ .map(peer -> peer.getAddress() == null ? "" : peer.getAddress().concat(peer.getAdminAddress() == null ? ":FOLLOWER" : ":LEADER"))
Review comment:
```
private static RaftPeer.Builder raftPeerBuilderFor(DatanodeDetails dn) {
return RaftPeer.newBuilder()
.setId(toRaftPeerId(dn))
.setAddress(toRaftPeerAddress(dn, Port.Name.RATIS_SERVER))
.setAdminAddress(toRaftPeerAddress(dn, Port.Name.RATIS_ADMIN))
.setClientAddress(toRaftPeerAddress(dn, Port.Name.RATIS));
}
```
I consider that each raft peer has its own admin address, no matter if it is leader or follower.
Better use ```NetUtils.isLocalAddress()``` to determine the peer whose address equals local, and assign him the leader role.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] GlenGeng commented on a change in pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #2094:
URL: https://github.com/apache/ozone/pull/2094#discussion_r603749073
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -225,7 +225,7 @@ public void stop() throws IOException {
@Override
public List<String> getRatisRoles() {
return division.getGroup().getPeers().stream()
- .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
+ .map(peer -> peer.getAddress() == null ? "" : peer.getAddress().concat(peer.getAdminAddress() == null ? ":FOLLOWER" : ":LEADER"))
Review comment:
```
private static RaftPeer.Builder raftPeerBuilderFor(DatanodeDetails dn) {
return RaftPeer.newBuilder()
.setId(toRaftPeerId(dn))
.setAddress(toRaftPeerAddress(dn, Port.Name.RATIS_SERVER))
.setAdminAddress(toRaftPeerAddress(dn, Port.Name.RATIS_ADMIN))
.setClientAddress(toRaftPeerAddress(dn, Port.Name.RATIS));
}
```
I consider that each raft peer has its own admin address, no matter if it is leader or follower.
Better use ```NetUtils.isLocalAddress()``` to determine the peer whose address equals local, and assign him the leader role. Since SCM will do leader check before handle this request, the local SCM will be the leader.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2094:
URL: https://github.com/apache/ozone/pull/2094#discussion_r603651548
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -225,7 +225,7 @@ public void stop() throws IOException {
@Override
public List<String> getRatisRoles() {
return division.getGroup().getPeers().stream()
- .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
+ .map(peer -> peer.getAddress() == null ? "" : peer.getAddress().concat(peer.getAdminAddress() == null ? ":FOLLOWER" : ":LEADER"))
Review comment:
I am not sure if we should rely on the getAdminAddress() call to be null to determine if this node is LEADER or FOLLOWER. Can we do something similar to OM? The SCM that is returning the answer will be leader (since read calls go only to Leader), and the other nodes will be followers.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -225,7 +225,7 @@ public void stop() throws IOException {
@Override
public List<String> getRatisRoles() {
return division.getGroup().getPeers().stream()
- .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
+ .map(peer -> peer.getAddress() == null ? "" : peer.getAddress().concat(peer.getAdminAddress() == null ? ":FOLLOWER" : ":LEADER"))
Review comment:
Also, can we use org.apache.ratis.proto.RaftProtos.RaftPeerRole#LEADER/FOLLOWER instead of hardcoded strings?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ghuangups commented on pull request #2094: HDDS-5022. SCM get roles command should provide Ratis Leader/Follower…
Posted by GitBox <gi...@apache.org>.
ghuangups commented on pull request #2094:
URL: https://github.com/apache/ozone/pull/2094#issuecomment-810843931
Somehow the repo rejected my push. I had to use -f to overwrite. Created a new PR: https://github.com/apache/ozone/pull/2098.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org