RATIS-1526. Add ratis-shell pause&resume leader election command by codings-dan · Pull Request #603 · apache/ratis

codings-dan · 2022-02-17T13:24:47Z

What changes were proposed in this pull request?

Add a sub command to pause and resume leader election on the specific peer.

What is the link to the Apache JIRA

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

How was this patch tested?

UT

szetszwo · 2022-02-19T15:32:53Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/LeaderElectionCommand.java

+
+  @Override
+  public String getCommandName() {
+    return "leaderElection";


This command and the ElectCommand are closely related. How about we have a base election command and then have the following subcommands?

transfer

pause

resume

codings-dan · 2022-02-21T12:11:55Z

@szetszwo I did code splitting related to election command, PTAL, thx!

szetszwo · 2022-02-21T14:38:08Z

+++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
@@ -72,9 +72,12 @@ public class RatisShell extends AbstractShell {
       // Add commands from <pkgName>.command.*
       if (cls.getPackage().getName().equals(pkgName + ".command")
           && !Modifier.isAbstract(cls.getModifiers())) {
+        System.out.println("Found " + cls);
         // Only instantiate a concrete class
         final Command cmd = ReflectionUtils.newInstance(cls, classArgs, objectArgs);
         commandsMap.put(cmd.getCommandName(), cmd);
+      } else {
+        System.out.println("Ignored " + cls);
       }
     }

@codings-dan , if we add the code above, the new subcommnads will be ignored.

Ignored class org.apache.ratis.shell.cli.sh.command.AbstractRatisCommand
Found class org.apache.ratis.shell.cli.sh.command.PeerCommand
Found class org.apache.ratis.shell.cli.sh.command.ElectionCommand
Ignored class org.apache.ratis.shell.cli.sh.group.GroupListCommand
Found class org.apache.ratis.shell.cli.sh.command.SetPriorityCommand
Ignored class org.apache.ratis.shell.cli.sh.group.GroupInfoCommand
Ignored class org.apache.ratis.shell.cli.sh.snapshot.TakeSnapshotCommand
Ignored class org.apache.ratis.shell.cli.sh.election.PauseCommand
Found class org.apache.ratis.shell.cli.sh.command.SnapshotCommand
Found class org.apache.ratis.shell.cli.sh.command.GroupCommand
Ignored class org.apache.ratis.shell.cli.sh.election.TransferCommand
Ignored class org.apache.ratis.shell.cli.sh.election.ResumeCommand
Usage: ratis sh [generic options]
	 [election [resume] [transfer] [pause]]                    
	 [group [list] [info]]                                     
	 [peer -peers <PEER0_HOST:PEER0_PORT,PEER1_HOST:PEER1_PORT,PEER2_HOST:PEER2_PORT> [-groupid <RAFT_GROUP_ID>] -remove <PEER_HOST:PEER_PORT> -add <PEER_HOST:PEER_PORT>]
	 [setPriority -peers <PEER0_HOST:PEER0_PORT,PEER1_HOST:PEER1_PORT,PEER2_HOST:PEER2_PORT> [-groupid <RAFT_GROUP_ID>] -addressPriority <PEER_HOST:PEER_PORT|PRIORITY>]
	 [snapshot [create]]

codings-dan · 2022-02-22T01:59:05Z

+++ b/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/RatisShell.java
@@ -72,9 +72,12 @@ public class RatisShell extends AbstractShell {
       // Add commands from <pkgName>.command.*
       if (cls.getPackage().getName().equals(pkgName + ".command")
           && !Modifier.isAbstract(cls.getModifiers())) {
+        System.out.println("Found " + cls);
         // Only instantiate a concrete class
         final Command cmd = ReflectionUtils.newInstance(cls, classArgs, objectArgs);
         commandsMap.put(cmd.getCommandName(), cmd);
+      } else {
+        System.out.println("Ignored " + cls);
       }
     }

@codings-dan , if we add the code above, the new subcommnads will be ignored.

Ignored class org.apache.ratis.shell.cli.sh.command.AbstractRatisCommand
Found class org.apache.ratis.shell.cli.sh.command.PeerCommand
Found class org.apache.ratis.shell.cli.sh.command.ElectionCommand
Ignored class org.apache.ratis.shell.cli.sh.group.GroupListCommand
Found class org.apache.ratis.shell.cli.sh.command.SetPriorityCommand
Ignored class org.apache.ratis.shell.cli.sh.group.GroupInfoCommand
Ignored class org.apache.ratis.shell.cli.sh.snapshot.TakeSnapshotCommand
Ignored class org.apache.ratis.shell.cli.sh.election.PauseCommand
Found class org.apache.ratis.shell.cli.sh.command.SnapshotCommand
Found class org.apache.ratis.shell.cli.sh.command.GroupCommand
Ignored class org.apache.ratis.shell.cli.sh.election.TransferCommand
Ignored class org.apache.ratis.shell.cli.sh.election.ResumeCommand
Usage: ratis sh [generic options]
	 [election [resume] [transfer] [pause]]                    
	 [group [list] [info]]                                     
	 [peer -peers <PEER0_HOST:PEER0_PORT,PEER1_HOST:PEER1_PORT,PEER2_HOST:PEER2_PORT> [-groupid <RAFT_GROUP_ID>] -remove <PEER_HOST:PEER_PORT> -add <PEER_HOST:PEER_PORT>]
	 [setPriority -peers <PEER0_HOST:PEER0_PORT,PEER1_HOST:PEER1_PORT,PEER2_HOST:PEER2_PORT> [-groupid <RAFT_GROUP_ID>] -addressPriority <PEER_HOST:PEER_PORT|PRIORITY>]
	 [snapshot [create]]

Do you mean we add these lines of code and then display this information during compilation?

szetszwo · 2022-02-22T09:33:20Z

Do you mean we add these lines of code and then display this information during compilation?

@codings-dan, The code lines are for printing debug messages. Currently, the debug messages show that many of the command classes are ignored since they are not under the org.apache.ratis.shell.cli.sh.command package.

codings-dan · 2022-02-22T10:52:23Z

@szetszwo We don't need load classes outside the org.apache.ratis.shell.cli.sh.command package, they are all sub-commands, and their baseCommand are in the command package. BaseCommand maintains a map of the mapping relationship between sub-command name and sub-command. See org.apache.ratis.shell.cli.AbstractShell.java#run

szetszwo · 2022-02-22T12:13:48Z

@codings-dan , I see. The print out is a little bit confusing. Thanks for explaining it.

I suggest that we move the setPriority command as a subcommand of the peer command and change the peer command with the following subcommands.

add
remove
setPriority

See if you are interested working on it.

codings-dan · 2022-02-22T12:19:52Z

@codings-dan , I see. The print out is a little bit confusing. Thanks for explaining it.

I suggest that we move the setPriority command as a subcommand of the peer command and change the peer command with the following subcommands.

add

remove

setPriority

See if you are interested working on it.

Good idea, since this change has nothing to do with this jira, I will complete the code change in the future.

szetszwo

@codings-dan , thanks a lot. Let's move the common code to a new AbstractParentCommand class. Could you also clean up the GroupCommnad and the SnapshotCommand? See https://issues.apache.org/jira/secure/attachment/13040333/603_review.patch

szetszwo · 2022-02-22T12:25:24Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/ElectionCommand.java

+      = Collections.unmodifiableList(Arrays.asList(
+      TransferCommand::new, PauseCommand::new, ResumeCommand::new));
+
+  private final Map<String, Command> subs;


Let's move the common code to a parent class:

public abstract class AbstractParentCommand extends AbstractRatisCommand{ private final Map<String, Command> subs; public AbstractParentCommand(Context context, List<Function<Context, AbstractRatisCommand>> subCommandConstructors) { super(context); this.subs = Collections.unmodifiableMap(subCommandConstructors.stream() .map(constructor -> constructor.apply(context)) .collect(Collectors.toMap(Command::getCommandName, Function.identity()))); } @Override public final Map<String, Command> getSubCommands() { return subs; } @Override public final String getUsage() { final StringBuilder usage = new StringBuilder(getCommandName()); for (String cmd : getSubCommands().keySet()) { usage.append(" [").append(cmd).append("]"); } return usage.toString(); } }

szetszwo · 2022-02-22T12:26:23Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/ElectionCommand.java

+      TransferCommand::new, PauseCommand::new, ResumeCommand::new));
+
+  private final Map<String, Command> subs;
+  public static final String ADDRESS_OPTION_NAME = "address";


Let's remove it. This is not really used.

szetszwo · 2022-02-22T12:26:34Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/ElectionCommand.java

+  }
+
+  @Override
+  public Options getOptions() {


Let's remove it. This is not really used.

szetszwo · 2022-02-22T12:38:08Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/election/PauseCommand.java

+    final RaftPeerId peerId;
+    Optional<RaftPeer> peer =
+        getRaftGroup().getPeers().stream().filter(p -> p.getAddress().equals(strAddr)).findAny();
+    if (peer.isPresent()) {
+      peerId = peer.get().getId();
+    } else {
+      printf("Can't find a sever with the address:%s", strAddr);
+      return -1;
+    }


The code can be simplified as below.

final RaftPeerId peerId = getRaftGroup().getPeers().stream() .filter(p -> p.getAddress().equals(strAddr)).findAny() .map(RaftPeer::getId) .orElse(null); if (peerId == null) { printf("Peer not found: %s", strAddr); return -1; }

szetszwo · 2022-02-22T12:45:52Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/election/ResumeCommand.java

+    final RaftPeerId peerId;
+    Optional<RaftPeer> peer =
+        getRaftGroup().getPeers().stream().filter(p -> p.getAddress().equals(strAddr)).findAny();
+    if (peer.isPresent()) {
+      peerId = peer.get().getId();
+    } else {


We may simplify the code as in the PauseCommnad.

codings-dan · 2022-02-23T12:53:50Z

@codings-dan , I see. The print out is a little bit confusing. Thanks for explaining it.

I suggest that we move the setPriority command as a subcommand of the peer command and change the peer command with the following subcommands.

add

remove

setPriority

See if you are interested working on it.

@szetszwo I have refactored the code, PTAL, thx!

szetszwo

+1 the change looks good.

…che#603) (cherry picked from commit 069f9d8)

…che#603)

Add ratis-shell pause&resume leader election command

8437752

codings-dan changed the title ~~Add ratis-shell pause&resume leader election command~~ RATIS-1526. Add ratis-shell pause&resume leader election command Feb 17, 2022

szetszwo reviewed Feb 19, 2022

View reviewed changes

codings-dan added 2 commits February 21, 2022 19:54

refactor code

765fe92

make code better

bd8571b

szetszwo reviewed Feb 22, 2022

View reviewed changes

refactor code

734cf0f

szetszwo approved these changes Feb 23, 2022

View reviewed changes

szetszwo merged commit 069f9d8 into apache:master Feb 23, 2022

codings-dan deleted the 1526 branch February 24, 2022 01:44

SincereXIA pushed a commit to SincereXIA/ratis that referenced this pull request Mar 1, 2022

RATIS-1526. Add ratis-shell pause&resume leader election command (apa…

dd5931c

…che#603) (cherry picked from commit 069f9d8)

symious pushed a commit to symious/ratis that referenced this pull request Feb 29, 2024

RATIS-1526. Add ratis-shell pause&resume leader election command (apa…

1f5d80a

…che#603)

Conversation

codings-dan commented Feb 17, 2022

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codings-dan commented Feb 21, 2022

Uh oh!

szetszwo commented Feb 21, 2022

Uh oh!

codings-dan commented Feb 22, 2022

Uh oh!

szetszwo commented Feb 22, 2022

Uh oh!

codings-dan commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szetszwo commented Feb 22, 2022

Uh oh!

codings-dan commented Feb 22, 2022

Uh oh!

szetszwo left a comment

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!

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!

codings-dan commented Feb 23, 2022

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codings-dan commented Feb 22, 2022 •

edited

Loading