RATIS-1860. Add ratis-shell cmd to generate a new raft-meta.conf. #901

chunxiaozheng · 2023-08-10T07:28:44Z

What changes were proposed in this pull request?

By reading the code related to updating peers, I found that the currentConf in the ConfigurationManager class maintains the current peers information, and a TreeMap object maintains the historical peers information, the TreeMap object uses the log entry index as the key. Normally, if the index of the newly added Configuration is the largest, the currentConf will be set to the latest added configuration information. Therefore, as long as modify the raft-meta.conf, follow the above rules, set the appropriate index and the latest peering information, we can solve the problem raised in RATIS-1860.
Through analysis, reading the index in old raft-meta.conf and add 1 as the new index is a suitable choice. If we directly use the index in the old raft-meta.conf, an exception will be throw during Preconditions.assertTrue(found.equals(conf)).If we directly set a larger value, such as Long.MAX_Value, an exception will be throw when switch leader.

In this PR, a ratis-shell cmd has been added, which will read raft-meta.conf in the specified path, get the log entry index and construct a new index(old index + 1), then use the new index and the newly passed peers to construct a new raft-meta.conf file to support normal startup when moving log entries and checkpoints to other nodes.
In addition, it is also supported to directly pass an new index, which can be used directly when generating a new raft-meta.conf file. However, this method requires observing the old index. If the input index is inappropriate, it may also cause the new node to not start properly.

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. RATIS-XXXX. Fix a typo in YYY.)

Please replace this section with the link to the Apache JIRA)

How was this patch tested?

(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)

chunxiaozheng · 2023-08-11T01:58:54Z

@codings-dan Can you help me review this PR? Thanks!

codings-dan

Thanks a lot for working on this! Left some comment

codings-dan · 2023-08-16T02:09:18Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/generation/RaftMetaConfCommand.java

+    List<RaftProtos.RaftPeerProto> raftPeerProtos = new ArrayList<>();
+    for (String peer : peersStr.split(",")) {
+      String[] peerInfos = peer.split(":");
+      String peerId = peerInfos[0] + "_" + peerInfos[1];


final InetSocketAddress serverAddress = parseInetSocketAddress(address);
final String peerId = RaftUtils.getPeerId(serverAddress);

codings-dan · 2023-08-16T02:11:27Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/generation/RaftMetaConfCommand.java

+  @Override
+  public int run(CommandLine cl) throws IOException {
+    String peersStr = cl.getOptionValue(PEER_OPTION_NAME);
+    String path = cl.getOptionValue(PATH_OPTION_NAME);


peersStr and path need check not null.

codings-dan · 2023-08-16T02:13:06Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/generation/RaftMetaConfCommand.java

+import java.util.ArrayList;
+import java.util.List;
+
+public class RaftMetaConfCommand extends AbstractRatisCommand {


How about use GroupCommand as the parent command? BTW, please add some necessary notes.

codings-dan

Thanks for the update! Could you add some doc about this command? see https://github.com/apache/ratis/blob/master/ratis-docs/src/site/markdown/cli.md

chunxiaozheng · 2023-08-16T03:32:05Z

Thanks for the update! Could you add some doc about this command? see https://github.com/apache/ratis/blob/master/ratis-docs/src/site/markdown/cli.md

@codings-dan Thanks for your review, i have updated the cli.md.

codings-dan

the change looks good, +1

codings-dan · 2023-08-16T03:54:44Z

@szetszwo Could you help take a look at the pull request? Thanks in advance

szetszwo · 2023-08-16T18:17:39Z

@codings-dan , sure, will review this.

szetszwo

@chunxiaozheng , thanks a lot for working on this!

The new command here is very different from the other existing commands since it does not connect the servers. It just reads and writes the raft-meta file.

We have a raft log tool similar to this in the ratis-tools module. How about moving this to ratis-tools?

chunxiaozheng · 2023-08-17T05:07:45Z

@szetszwo Thanks for your review! But I have a question, if we put this command in the ratis-tool module, how should we use it?

szetszwo · 2023-08-17T17:49:37Z

@chunxiaozheng , do you mean how to run it? We may run it directly with the class (or using maven).

Another option is to add a new parent command, say local, and add the new raftMetaConf under it. Then, it may be easier to run it. Later on, we may include the log dump tool as a subcommand.

 [local [raftMetaConf] [dumpLog]

chunxiaozheng · 2023-08-18T03:12:02Z

@chunxiaozheng , do you mean how to run it? We may run it directly with the class (or using maven).

Another option is to add a new parent command, say local, and add the new raftMetaConf under it. Then, it may be easier to run it. Later on, we may include the log dump tool as a subcommand.
 [local [raftMetaConf] [dumpLog]

@szetszwo Thanks! Adding a local command is a good idea, which makes it very easy to use this tool, and I have already followed this approach to modify PR.

szetszwo · 2023-08-18T03:30:57Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java

+ * Command for generate a new raft-meta.conf file based on original raft-meta.conf and new peers,
+ * which is used to move a raft node to a new node.
+ */
+public class RaftMetaConfCommand extends AbstractRatisCommand {


@chunxiaozheng , thanks a lot for the update. If we implement a local command, we should not extend AbstractRatisCommand since it will connect to remote servers.

@szetszwo Thanks for your review! I have updated this PR.
In addition, AbstractParentCommand should not extends AbstractRatisCommand too, since the commands that extends AbstractParentCommand means that it is a parent command, it does not need to connect to remote servers too.

szetszwo

Thanks for the update. Please see the comments inlined and also https://issues.apache.org/jira/secure/attachment/13062287/901_review.patch

szetszwo · 2023-08-18T21:11:45Z

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

+public abstract class AbstractCommand implements Command {
+
+  public static InetSocketAddress parseInetSocketAddress(String address) {


Let's also move the Context and PrintStream related code from AbstractRatisCommand to AbstractCommand.

szetszwo · 2023-08-19T03:18:22Z

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

+public abstract class AbstractParentCommand implements Command {
  private final Map<String, Command> subs;

  public AbstractParentCommand(Context context, List<Function<Context, AbstractRatisCommand>> subCommandConstructors) {


Let's change the Function to Function<Context, Command>. Then, the new LocalCommand can also use it.

szetszwo · 2023-08-19T03:21:16Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java

+    String peersStr = cl.getOptionValue(PEER_OPTION_NAME);
+    String path = cl.getOptionValue(PATH_OPTION_NAME);
+    if (peersStr == null || path == null || peersStr.isEmpty() || path.isEmpty()) {
+      System.out.println("peers or path can't be empty.");


We should use the PrintStream from Context instead of System.out.

szetszwo · 2023-08-19T03:22:10Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java

+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.ratis.proto.RaftProtos;


Let's import the inner classes. Then, the code is easier to read.

import org.apache.ratis.proto.RaftProtos.LogEntryProto; import org.apache.ratis.proto.RaftProtos.RaftConfigurationProto; import org.apache.ratis.proto.RaftProtos.RaftPeerProto; import org.apache.ratis.proto.RaftProtos.RaftPeerRole;

szetszwo · 2023-08-19T03:23:28Z

ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/local/RaftMetaConfCommand.java

+    try (InputStream in = new FileInputStream(new File(path, RAFT_META_CONF));
+         OutputStream out = new FileOutputStream(new File(path, NEW_RAFT_META_CONF))) {


Use the Files api instead.

chunxiaozheng · 2023-08-20T14:14:58Z

@szetszwo I have updated the PR according to the comments，thanks for your review!

szetszwo

+1 change looks good.

szetszwo · 2023-08-21T16:51:11Z

@chunxiaozheng , thanks again for working on this! Could you post a comment on RATIS-1860 so that we can assign the JIRA to you?

chunxiaozheng · 2023-08-22T06:51:13Z

@chunxiaozheng , thanks again for working on this! Could you post a comment on RATIS-1860 so that we can assign the JIRA to you?

@szetszwo of course, but the page display Public signup for this instance is disabled. I have submitted the request to create jira account and i am waiting for a review.

szetszwo · 2023-08-22T10:23:08Z

@chunxiaozheng , I see. What username have you submitted?

chunxiaozheng · 2023-08-23T03:07:47Z

@chunxiaozheng , I see. What username have you submitted?

@szetszwo I'm very sorry, I just saw the email, my registered username is chunxiaozheng, and I have seen your reply. Thank you very much!

szetszwo · 2023-08-23T03:31:48Z

@chunxiaozheng , don't worry. Thank you!

…ache#901) * Add ratis-shell cmd to generate a new raft-meta.conf. * Remove index option. * Style fix. * Use GroupCommand as the parent of RaftMetaConfCommand. * Modified by review comments. * Update cli.md * Use LocalCommand as the parent of RaftMetaConfCommand. * ReConstruct some abstract command. * Modified by review comments. * Use println instead of printf. * Checkstyle error fix(delete some unuse import). --------- Co-authored-by: idellzheng <idellzheng@tencent.com>

### What changes were proposed in this pull request? Bump Ratis version from 2.5.1 to 3.0.1. Address incompatible changes: - RATIS-589. Eliminate buffer copying in SegmentedRaftLogOutputStream.(apache/ratis#964) - RATIS-1677. Do not auto format RaftStorage in RECOVER.(apache/ratis#718) - RATIS-1710. Refactor metrics api and implementation to separated modules. (apache/ratis#749) ### Why are the changes needed? Bump Ratis version from 2.5.1 to 3.0.1. Ratis has released v3.0.0, v3.0.1, which release note refers to [3.0.0](https://ratis.apache.org/post/3.0.0.html), [3.0.1](https://ratis.apache.org/post/3.0.1.html). The 3.0.x version include new features like pluggable metrics and lease read, etc, some improvements and bugfixes including: - 3.0.0: Change list of ratis 3.0.0 In total, there are roughly 100 commits diffing from 2.5.1 including: - Incompatible Changes - RaftStorage Auto-Format - RATIS-1677. Do not auto format RaftStorage in RECOVER. (apache/ratis#718) - RATIS-1694. Fix the compatibility issue of RATIS-1677. (apache/ratis#731) - RATIS-1871. Auto format RaftStorage when there is only one directory configured. (apache/ratis#903) - Pluggable Ratis-Metrics (RATIS-1688) - RATIS-1689. Remove the use of the thirdparty Gauge. (apache/ratis#728) - RATIS-1692. Remove the use of the thirdparty Counter. (apache/ratis#732) - RATIS-1693. Remove the use of the thirdparty Timer. (apache/ratis#734) - RATIS-1703. Move MetricsReporting and JvmMetrics to impl. (apache/ratis#741) - RATIS-1704. Fix SuppressWarnings(“VisibilityModifier”) in RatisMetrics. (apache/ratis#742) - RATIS-1710. Refactor metrics api and implementation to separated modules. (apache/ratis#749) - RATIS-1712. Add a dropwizard 3 implementation of ratis-metrics-api. (apache/ratis#751) - RATIS-1391. Update library dropwizard.metrics version to 4.x (apache/ratis#632) - RATIS-1601. Use the shaded dropwizard metrics and remove the dependency (apache/ratis#671) - Streaming Protocol Change - RATIS-1569. Move the asyncRpcApi.sendForward(..) call to the client side. (apache/ratis#635) - New Features - Leader Lease (RATIS-1864) - RATIS-1865. Add leader lease bound ratio configuration (apache/ratis#897) - RATIS-1866. Maintain leader lease after AppendEntries (apache/ratis#898) - RATIS-1894. Implement ReadOnly based on leader lease (apache/ratis#925) - RATIS-1882. Support read-after-write consistency (apache/ratis#913) - StateMachine API - RATIS-1874. Add notifyLeaderReady function in IStateMachine (apache/ratis#906) - RATIS-1897. Make TransactionContext available in DataApi.write(..). (apache/ratis#930) - New Configuration Properties - RATIS-1862. Add the parameter whether to take Snapshot when stopping to adapt to different services (apache/ratis#896) - RATIS-1930. Add a conf for enable/disable majority-add. (apache/ratis#961) - RATIS-1918. Introduces parameters that separately control the shutdown of RaftServerProxy by JVMPauseMonitor. (apache/ratis#950) - RATIS-1636. Support re-config ratis properties (apache/ratis#800) - RATIS-1860. Add ratis-shell cmd to generate a new raft-meta.conf. (apache/ratis#901) - Improvements & Bug Fixes - Netty - RATIS-1898. Netty should use EpollEventLoopGroup by default (apache/ratis#931) - RATIS-1899. Use EpollEventLoopGroup for Netty Proxies (apache/ratis#932) - RATIS-1921. Shared worker group in WorkerGroupGetter should be closed. (apache/ratis#955) - RATIS-1923. Netty: atomic operations require side-effect-free functions. (apache/ratis#956) - RaftServer - RATIS-1924. Increase the default of raft.server.log.segment.size.max. (apache/ratis#957) - RATIS-1892. Unify the lifetime of the RaftServerProxy thread pool (apache/ratis#923) - RATIS-1889. NoSuchMethodError: RaftServerMetricsImpl.addNumPendingRequestsGauge apache/ratis#922 (apache/ratis#922) - RATIS-761. Handle writeStateMachineData failure in leader. (apache/ratis#927) - RATIS-1902. The snapshot index is set incorrectly in InstallSnapshotReplyProto. (apache/ratis#933) - RATIS-1912. Fix infinity election when perform membership change. (apache/ratis#954) - RATIS-1858. Follower keeps logging first election timeout. (apache/ratis#894) - 3.0.1:This is a bugfix release. See the [changes between 3.0.0 and 3.0.1](apache/ratis@ratis-3.0.0...ratis-3.0.1) releases. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Cluster manual test. Closes #2480 from SteNicholas/CELEBORN-1400. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>

### What changes were proposed in this pull request? Add `local` command in `celeborn_ratis_shell.md` to sync document [cli.md](https://github.com/apache/ratis/blob/ratis-3.0.1/ratis-docs/src/site/markdown/cli.md). ### Why are the changes needed? Celeborn has already bumped Ratis version from 2.5.1 to 3.0.1. Ratis v3.0.1 supports `local` command to process local operation, which no need to connect to ratis server. `celeborn_ratis_shell.md` should add local command to guide users to process local operation. Backport: apache/ratis#901 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No. Closes #2575 from SteNicholas/CELEBORN-1466. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com>

### What changes were proposed in this pull request? Add `local` command in `celeborn_ratis_shell.md` to sync document [cli.md](https://github.com/apache/ratis/blob/ratis-3.0.1/ratis-docs/src/site/markdown/cli.md). ### Why are the changes needed? Celeborn has already bumped Ratis version from 2.5.1 to 3.0.1. Ratis v3.0.1 supports `local` command to process local operation, which no need to connect to ratis server. `celeborn_ratis_shell.md` should add local command to guide users to process local operation. Backport: apache/ratis#901 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No. Closes #2575 from SteNicholas/CELEBORN-1466. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com> (cherry picked from commit e4d3b80) Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com>

Add ratis-shell cmd to generate a new raft-meta.conf.

672d5f9

chunxiaozheng force-pushed the master-generate-dev branch from e1f47d5 to 672d5f9 Compare August 10, 2023 09:05

chunxiaozheng added 2 commits August 15, 2023 20:48

Remove index option.

9b13aae

Style fix.

8a5556d

codings-dan reviewed Aug 16, 2023

View reviewed changes

chunxiaozheng added 2 commits August 16, 2023 10:35

Use GroupCommand as the parent of RaftMetaConfCommand.

24b7d1f

Modified by review comments.

74ca5ae

codings-dan reviewed Aug 16, 2023

View reviewed changes

Update cli.md

f3c9be2

codings-dan approved these changes Aug 16, 2023

View reviewed changes

szetszwo reviewed Aug 16, 2023

View reviewed changes

Use LocalCommand as the parent of RaftMetaConfCommand.

1d56bcc

szetszwo reviewed Aug 18, 2023

View reviewed changes

ReConstruct some abstract command.

7237234

szetszwo reviewed Aug 19, 2023

View reviewed changes

Modified by review comments.

cb6398a

Use println instead of printf.

fece8fb

szetszwo approved these changes Aug 20, 2023

View reviewed changes

Checkstyle error fix(delete some unuse import).

f645ea8

codings-dan merged commit 9095c89 into apache:master Aug 21, 2023

SteNicholas mentioned this pull request Apr 24, 2024

[CELEBORN-1400] Bump Ratis version from 2.5.1 to 3.0.1 apache/celeborn#2480

Closed

SteNicholas mentioned this pull request Jun 18, 2024

[CELEBORN-1466] Add local command in celeborn_ratis_shell.md apache/celeborn#2575

Closed

		public abstract class AbstractCommand implements Command {

		public static InetSocketAddress parseInetSocketAddress(String address) {

		try (InputStream in = new FileInputStream(new File(path, RAFT_META_CONF));
		OutputStream out = new FileOutputStream(new File(path, NEW_RAFT_META_CONF))) {

RATIS-1860. Add ratis-shell cmd to generate a new raft-meta.conf. #901

RATIS-1860. Add ratis-shell cmd to generate a new raft-meta.conf. #901

Uh oh!

Conversation

chunxiaozheng commented Aug 10, 2023

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

chunxiaozheng commented Aug 11, 2023

Uh oh!

codings-dan 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!

codings-dan left a comment

Choose a reason for hiding this comment

Uh oh!

chunxiaozheng commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codings-dan left a comment

Choose a reason for hiding this comment

Uh oh!

codings-dan commented Aug 16, 2023

Uh oh!

szetszwo commented Aug 16, 2023

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

chunxiaozheng commented Aug 17, 2023

Uh oh!

szetszwo commented Aug 17, 2023

Uh oh!

chunxiaozheng commented Aug 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

chunxiaozheng commented Aug 20, 2023

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

szetszwo commented Aug 21, 2023

Uh oh!

chunxiaozheng commented Aug 22, 2023

Uh oh!

szetszwo commented Aug 22, 2023

Uh oh!

chunxiaozheng commented Aug 23, 2023

Uh oh!

szetszwo commented Aug 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

chunxiaozheng commented Aug 16, 2023 •

edited

Loading