HDDS-4115. CLI command to show current SCM leader and follower status. #1346

amaliujia · 2020-08-25T04:41:09Z

What changes were proposed in this pull request?

CLI command to show current SCM leader and follower status. E.g. ozone admin scmha listratisstatus

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4115

How was this patch tested?

Unit Test

Command line output

[180.3.14.5:2333, 180.3.14.21:2333, 180.3.14.145:2333]

amaliujia · 2020-08-25T04:44:06Z

R: @timmylicheng

This is still WIP. Can you give some suggestions? Just want to make sure I am on the right track.

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

...mmon/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestScmAdminHA.java

timmylicheng · 2020-08-27T08:38:11Z

Could you attach your CLI command output?

amaliujia · 2020-08-27T18:34:40Z

@timmylicheng

The output is empty array. In current test setup, scm.getScmHAManager().getRatisServer().getRaftPeers() returns empty list. My current guess is I haven't enabled SCM HA in the test setup in a correct way.

I expect it will print something like [127.0.1.1:1313, 127.0.1.1:1314, 127.0.1.1:1315]. Also I guess this JIRA requires a format of, e.g., [leader: 127.0.1.1:1313, follower: 127.0.1.1:1314, follower: 127.0.1.1:1315]?

So if my approach looks reasonable, I can continue working on this version to have a valid test somehow to make sure it prints desired formats .

amaliujia · 2020-08-28T04:57:59Z

Ok I think I can work to have an example output first.

amaliujia · 2020-08-29T06:09:57Z

@timmylicheng

I found the reason why in my current test empty array was printed. It is because the test is using MockSCMHaManager: https://github.com/apache/hadoop-ozone/blob/HDDS-2823/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java#L191

I manually add a peer locally and get the output print: [localhost:7238], which is expected.

Any suggestion how should I proceed?

.../apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java

timmylicheng · 2020-09-01T13:36:31Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/GetScmRatisStatusSubcommand.java

Name seems a bit verbose. Let me find a syntax to align with OM HA

I think both this one and OM HA getserviceroles can be improved.

@adoroszlai

What kind of improvement OM HA getserviceroles you are thinking of? I am happy to make a separate PR for that.

status or roles would probably be enough to indicate the goal of the subcommand, something like

ozone admin (om|scm) status

Thanks. I will adopt ozone admin scm status in this PR and I will send another PR for ozone admin om status

ozone admin om getserviceroles -id=<>

This is what OM does. I have my +1 on ozone admin (om|scm) roles. Status is more like health check.

I am ok with both.

@adoroszlai what do you think?

I think it depends on whether you want to keep this command specific to roles, or may extend the same command in the future with other status info. Probably "roles" is better now, and "status" can be either a separate command or another alias later.

that makes sense. Indeed status sounds more like a health check and can carry much more information.

Consider OM is also actually getting, essentially, roles. We can start from roles

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestScmAdminHA.java

amaliujia · 2020-09-01T22:44:36Z

Addressed the following comments

Merge getRatisStatus with GetScmInfo
Adopt command syntax ozone admin scm status
added an acceptance test

@timmylicheng I am not sure how to test an acceptance test. Can you share a way to run it locally?

amaliujia · 2020-09-02T07:59:47Z

Uploaded one commit to

use ozone admin scm roles
fix the acceptance test.

amaliujia · 2020-09-08T21:16:47Z

Also R: @nandakumar131 can you please take a look?

amaliujia · 2020-09-09T21:16:08Z

R @timmylicheng @nandakumar131

I am thinking maybe we can first merge this PR and create a JIRA to track left work. Right now per feedback this command could print more information about Ratis peers, e.g. leader/follower roles, leader term, etc.

I took a look at how does OM HA does: https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L2535

Basically it seems such request will directly hit the leader OM, then get status will be much easier. Currently in SCM HA we haven't reached to the point with a robust Ratis setup.

timmylicheng · 2020-09-27T07:05:35Z

...ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/GetScmRatisStatusSubcommand.java

+  @Override
+  public Void call() throws Exception {
+    ScmClient scmClient = parent.createScmClient();
+    List<String> status = scmClient.getScmRatisStatus();


We probably want to align the naming here. Either we use roles or status for all external and internal interfaces.

Good point. I have switched all status to roles

timmylicheng · 2020-09-27T07:08:11Z

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestScmAdminHA.java

+    conf = new OzoneConfiguration();
+
+    // Init HA cluster
+    omServiceId = "om-service-test1";


You mean SCM here?

Actually I meant OM. It turns out that I cannot ignore omServiceId to start the cluster (there is a check for this service id).

timmylicheng · 2020-09-27T07:15:37Z

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java


+  @Override
+  public List<String> getRatisStatus() {
+    return Arrays.asList(


You can use the default SCM RatisServer port. Check ratisBindPort in SCMHAConfiguration

Switched to use the default port. In the future I will think about how to better construct MockSCMHAManager for testing purpose (as the code is evolving we can keep updating MockSCMHAManager)

timmylicheng · 2020-09-27T07:48:06Z

Thanks @amaliujia for the contribution. The patch looks good overall. Just a few comments inline.

amaliujia · 2020-09-27T21:07:48Z

@timmylicheng comments addressed. Can you take another look?

bshashikant · 2020-09-29T05:29:40Z

I feel the CLI should be common for both OM and SCM and probably extended to Datanodes as well.

amaliujia · 2020-09-29T05:39:21Z

Re @bshashikant

Agreed. Right now the command itself is unified (for both OM and SCM, we name this command as roles). Then we should unify the behavior fo both commands (and if there is DN command, that should be the same).

timmylicheng · 2020-09-29T06:57:02Z

+1. Thanks for Rui's contribution.
Merging

elek marked this pull request as draft August 25, 2020 08:20

amaliujia commented Aug 26, 2020

View reviewed changes

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java Outdated Show resolved Hide resolved

amaliujia commented Aug 26, 2020

View reviewed changes

...mmon/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java Outdated Show resolved Hide resolved

amaliujia commented Aug 26, 2020

View reviewed changes

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestScmAdminHA.java Outdated Show resolved Hide resolved

timmylicheng reviewed Sep 1, 2020

View reviewed changes

.../apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java Outdated Show resolved Hide resolved

timmylicheng reviewed Sep 1, 2020

View reviewed changes

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestScmAdminHA.java Outdated Show resolved Hide resolved

amaliujia changed the title ~~[WIP] HDDS-4115. CLI command to show current SCM leader and follower status.~~ HDDS-4115. CLI command to show current SCM leader and follower status. Sep 1, 2020

amaliujia marked this pull request as ready for review September 1, 2020 19:14

amaliujia added 2 commits September 1, 2020 12:20

HDDS-4115. CLI command to show current SCM leader and follower status.

ad3962e

fixup! address comments.

0269d6c

amaliujia force-pushed the HDDS-2823-patch branch from 8cf3741 to 0269d6c Compare September 1, 2020 22:42

amaliujia added 3 commits September 1, 2020 15:54

fixup! fix style

9850815

fixup! fix style

08ce2bc

fixup! add cli command description

56f2036

amaliujia mentioned this pull request Sep 1, 2020

HDDS-4189. Change ozone admin om getserviceroles to ozone admin om roles #1375

Merged

fixup! 1. use ozone admin scm roles 2. fix acceptance test

8373b6d

fixup! fix checkstyle

277089b

timmylicheng reviewed Sep 27, 2020

View reviewed changes

fixup! address comments

02810b2

timmylicheng merged commit 1c2a950 into apache:HDDS-2823 Sep 29, 2020

amaliujia deleted the HDDS-2823-patch branch September 29, 2020 17:22

HDDS-4115. CLI command to show current SCM leader and follower status. #1346

HDDS-4115. CLI command to show current SCM leader and follower status. #1346

Uh oh!

Conversation

amaliujia commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

amaliujia commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timmylicheng commented Aug 27, 2020

Uh oh!

amaliujia commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amaliujia commented Aug 28, 2020

Uh oh!

amaliujia commented Aug 29, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amaliujia Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amaliujia Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amaliujia commented Sep 1, 2020

Uh oh!

amaliujia commented Sep 2, 2020

Uh oh!

amaliujia commented Sep 8, 2020

Uh oh!

amaliujia commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timmylicheng commented Sep 27, 2020

Uh oh!

amaliujia commented Sep 27, 2020

Uh oh!

bshashikant commented Sep 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amaliujia commented Sep 29, 2020

Uh oh!

timmylicheng commented Sep 29, 2020

amaliujia commented Aug 25, 2020 •

edited

Loading

amaliujia commented Aug 25, 2020 •

edited

Loading

amaliujia commented Aug 27, 2020 •

edited

Loading

amaliujia Sep 1, 2020 •

edited

Loading

amaliujia Sep 2, 2020 •

edited

Loading

amaliujia commented Sep 9, 2020 •

edited

Loading

bshashikant commented Sep 29, 2020 •

edited

Loading