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/05/18 11:58:49 UTC

[GitHub] [ozone] sadanand48 opened a new pull request #2260: HDDS-5241.SCM UI should have leader/follower and Primordial SCM infor…

sadanand48 opened a new pull request #2260:
URL: https://github.com/apache/ozone/pull/2260


   …mation
   
   ## What changes were proposed in this pull request?
   Add the following info to SCM Web UI
   1. SCM leader/follower information.
   2. Primordial SCM / CA server information.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-5241
   
   ## How was this patch tested?
   <img width="1505" alt="Screenshot 2021-05-18 at 5 17 06 PM" src="https://user-images.githubusercontent.com/31859223/118646793-46109480-b7fe-11eb-81ac-57a5094d5e2a.png">
   
   


-- 
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] bshashikant commented on a change in pull request #2260: HDDS-5241.SCM UI should have leader/follower and Primordial SCM infor…

Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r634911366



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();
+  }
+
+  @Override
+  public String getPrimordialNode() {
+    if (SCMHAUtils.isSCMHAEnabled(configuration)) {
+      return SCMHAUtils.getPrimordialSCM(configuration);
+    }
+    return null;

Review comment:
       If the prmiodial node is not defined, let's not not show it at all.




-- 
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] sadanand48 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r635455354



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();
+  }
+
+  @Override
+  public String getPrimordialNode() {
+    if (SCMHAUtils.isSCMHAEnabled(configuration)) {
+      return SCMHAUtils.getPrimordialSCM(configuration);
+    }
+    return null;

Review comment:
       Addressed comment. If primordial node is not defined , the  field will be hidden.




-- 
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] bharatviswa504 commented on a change in pull request #2260: HDDS-5241.SCM UI should have leader/follower and Primordial SCM infor…

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r634484439



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();
+  }
+
+  @Override
+  public String getPrimordialNode() {
+    if (SCMHAUtils.isSCMHAEnabled(configuration)) {
+      return SCMHAUtils.getPrimordialSCM(configuration);
+    }
+    return null;

Review comment:
       If config primordial node is not defined, the node which is performed is considered as primordial SCM.
   We persist scmID in the version file but not the hostname. So, I don't see a way we can figure out primordial SCM. So, I believe we should be okay here if the config is not defined return null (And print some message that init performed node is considered as primordial SCM)
   
   cc @bshashikant for comments
   
   And also primordial SCM can be nodeID/hostName. If it is nodeID, get the hostName from SCMNodeDetails and use that.

##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       Can we format as String in a readable way, instead of the current format using StringBuilder?




-- 
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] sadanand48 commented on pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#issuecomment-844671126


   Thanks @bharatviswa504 @bshashikant for the review. Addressed comments.


-- 
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] sadanand48 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r635455923



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       Changed the format to :
   { HostName : scm2, Port : 9865, Role : FOLLOWER } { HostName : scm1, Port : 9865, Role : LEADER } { HostName : scm3, Port : 9865, Role : FOLLOWER }




-- 
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] bharatviswa504 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r636006165



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMMXBean.java
##########
@@ -70,4 +71,9 @@
   String getScmId();
 
   String getClusterId();
+
+  String getScmRatisRoles() throws IOException;
+
+  String getPrimordialNode();

Review comment:
       Can we also add JavaDoc for this?




-- 
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] bharatviswa504 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r636005382



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       Port looks too general, can we specifically mention ratis port?
   As SCM has multiple RPC endpoints.
   




-- 
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] bharatviswa504 commented on a change in pull request #2260: HDDS-5241.SCM UI should have leader/follower and Primordial SCM infor…

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r634913658



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       I believe this is already returning hostnames. In docker hostnames are scm1,scm2. 




-- 
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] bharatviswa504 merged pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2260:
URL: https://github.com/apache/ozone/pull/2260


   


-- 
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] bshashikant commented on a change in pull request #2260: HDDS-5241.SCM UI should have leader/follower and Primordial SCM infor…

Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r634912297



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       Also, it better to show the hostname than the nodeId in the UI.




-- 
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] sadanand48 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r636148177



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1708,4 +1708,17 @@ ContainerTokenSecretManager getContainerTokenSecretManager() {
     return containerTokenMgr;
   }
 
+  @Override
+  public List<String> getScmRatisRoles() throws IOException {
+    return getScmHAManager().getRatisServer().getRatisRoles();

Review comment:
       Done.




-- 
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] sadanand48 commented on a change in pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#discussion_r636150400



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMMXBean.java
##########
@@ -70,4 +71,9 @@
   String getScmId();
 
   String getClusterId();
+
+  String getScmRatisRoles() throws IOException;
+
+  String getPrimordialNode();

Review comment:
       Done. Also added the logic for if primordial node is nodeId.




-- 
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] bharatviswa504 commented on pull request #2260: HDDS-5241. SCM UI should have leader/follower and Primordial SCM information

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2260:
URL: https://github.com/apache/ozone/pull/2260#issuecomment-848922433


   Thank You @sadanand48 for the contribution and @bshashikant for the review.


-- 
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