Skip to content

RATIS-1470. Add peerId option to group list#564

Merged
szetszwo merged 7 commits intoapache:masterfrom
ferhui:RATIS-1470
Dec 16, 2021
Merged

RATIS-1470. Add peerId option to group list#564
szetszwo merged 7 commits intoapache:masterfrom
ferhui:RATIS-1470

Conversation

@ferhui
Copy link
Contributor

@ferhui ferhui commented Dec 15, 2021

What changes were proposed in this pull request?

Add one new option peerId to shell command of group list

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-1470

How was this patch tested?

Test by command line manually

@ferhui
Copy link
Contributor Author

ferhui commented Dec 15, 2021

@maobaolong @szetszwo Can you please take a look? Thanks

@ferhui
Copy link
Contributor Author

ferhui commented Dec 15, 2021

Here is the testing result.

bin/ratis sh group list -peers 'x01:9872,x02:9872,x03:9872' -peerId 'om1'
The peerId om1 (server x01:9872) is in 1 groups, and the groupIds is: [group-20E477FC5582]

Copy link
Member

@maobaolong maobaolong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ferhui Thanks for this great contribution, this looks good overall,

Would you like to use org.apache.commons.cli.OptionGroup to simplify the exclusive logic between the SERVER_ADDRESS_OPTION_NAME and PEER_ID_OPTION_NAME options?

@ferhui
Copy link
Contributor Author

ferhui commented Dec 15, 2021

@maobaolong Thanks for review! Updated

Copy link
Member

@maobaolong maobaolong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It LGTM, thanks @ferhui

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ferhui , thanks for working on this. Some minor comments inlined. Below is the suggested change.

  public int run(CommandLine cl) throws IOException {
    super.run(cl);
    final RaftPeerId peerId;
    final String address;
    if (cl.hasOption(PEER_ID_OPTION_NAME)) {
      peerId = RaftPeerId.getRaftPeerId(cl.getOptionValue(PEER_ID_OPTION_NAME));
      address = getRaftGroup().getPeer(peerId).getAddress();
    } else if (cl.hasOption(SERVER_ADDRESS_OPTION_NAME)) {
      address = cl.getOptionValue(SERVER_ADDRESS_OPTION_NAME);
      peerId = RaftUtils.getPeerId(parseInetSocketAddress(address));
    } else {
      throw new IllegalArgumentException(
          "Both " + PEER_ID_OPTION_NAME + " and " + SERVER_ADDRESS_OPTION_NAME + " options are missing.");
    }

    try(final RaftClient raftClient = RaftUtils.createClient(getRaftGroup())) {
      GroupListReply reply = raftClient.getGroupManagementApi(peerId).list();
      processReply(reply, () -> String.format("Failed to get group information of peerId %s (server %s)",
          peerId, address));
      printf(String.format("The peerId %s (server %s) is in %d groups, and the groupIds is: %s",
          peerId, address, reply.getGroupIds().size(), reply.getGroupIds()));
    }
    return 0;

  }

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the change looks good.

@szetszwo szetszwo merged commit e7509ea into apache:master Dec 16, 2021
@ferhui
Copy link
Contributor Author

ferhui commented Dec 17, 2021

@szetszwo @maobaolong Thanks for your review!

@ferhui ferhui deleted the RATIS-1470 branch December 17, 2021 06:02
symious pushed a commit to symious/ratis that referenced this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants