You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "DaveTeng0 (via GitHub)" <gi...@apache.org> on 2024/03/01 02:06:22 UTC
[PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
DaveTeng0 opened a new pull request, #1047:
URL: https://github.com/apache/ratis/pull/1047
## What changes were proposed in this pull request?
Add framework of PeerInfoCommand CLI
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2031
## How was this patch tested?
We will have an integration test in 3rd PR for https://issues.apache.org/jira/browse/RATIS-2033, which includes server side's changes.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #1047:
URL: https://github.com/apache/ratis/pull/1047#issuecomment-1974011616
> aNod ca be involed with many projec
oh yeah works on Datanodes too, since user needs to add 'groupId' in this command (https://github.com/apache/ratis/pull/1047/files#diff-b69a99093904ce19e8e368cbf0f92252b53a53a39e1fbcfa38fcbc636b99db4dR73) whenever a ratis server belongs to multiple groups, otherwise the command would just fail.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516902039
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(TermIndex.valueOf(currentTerm, getRaftLog().getLastCommittedIndex()).toProto());
Review Comment:
Thanks Nicholas for the pointer! updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516900648
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
Review Comment:
updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516900189
##########
ratis-common/src/main/java/org/apache/ratis/protocol/GroupInfoReply.java:
##########
@@ -33,25 +34,27 @@ public class GroupInfoReply extends RaftClientReply {
private final RoleInfoProto roleInfoProto;
private final boolean isRaftStorageHealthy;
private final RaftConfigurationProto conf;
+ private LogInfoProto logInfoProto;
Review Comment:
updated!
##########
ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java:
##########
@@ -225,10 +226,20 @@ static String toString(CommitInfoProto proto) {
return RaftPeerId.valueOf(proto.getServer().getId()) + ":c" + proto.getCommitIndex();
}
+ static String toString(LogInfoProto proto) {
+ return "lastSnapshot: " + proto.getLastSnapshot()
+ + ", applied: " + proto.getApplied()
+ + ", committed: " + proto.getCommitted();
+ }
+
static String toString(Collection<CommitInfoProto> protos) {
return protos.stream().map(ProtoUtils::toString).collect(Collectors.toList()).toString();
}
+ static String toStringLogInfo(Collection<LogInfoProto> protos) {
+ return protos.stream().map(ProtoUtils::toString).collect(Collectors.toList()).toString();
+ }
+
Review Comment:
removed!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1513824473
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,13 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+message PeerInfoProto {
+ CommitInfoProto commitInfo = 1;
+ uint64 appliedIndex = 2;
+ uint64 currentTerm = 3;
+ uint64 snapshotIndex = 4;
+}
Review Comment:
sure, updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #1047:
URL: https://github.com/apache/ratis/pull/1047#issuecomment-1981500582
> @DaveTeng0 , thanks for the update!
>
> We should change `GroupInfoReply`. The other protos and appendEntries/CommitInfo code should not be changed.
Hey @szetszwo updated accordingly, I move the logInfo related into GroupInfoReply! Please help to take another look when you have time, thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1513554496
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -554,6 +561,6 @@ message GroupInfoReplyProto {
RaftGroupProto group = 2;
RoleInfoProto role = 3;
bool isRaftStorageHealthy = 4;
- repeated CommitInfoProto commitInfos = 5;
+ repeated PeerInfoProto peerInfos = 5;
Review Comment:
Add the new proto at the end.
```
message GroupInfoReplyProto {
RaftRpcReplyProto rpcReply = 1;
RaftGroupProto group = 2;
RoleInfoProto role = 3;
bool isRaftStorageHealthy = 4;
repeated CommitInfoProto commitInfos = 5;
RaftConfigurationProto conf = 6;
LogInfoProto logInfo = 7;
}
```
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,13 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+message PeerInfoProto {
+ CommitInfoProto commitInfo = 1;
+ uint64 appliedIndex = 2;
+ uint64 currentTerm = 3;
+ uint64 snapshotIndex = 4;
+}
Review Comment:
Since the word `PeerInfo` is already used, let's call it `LogInfoProto`. BTW, let's use `TermIndexProto` instead of `uint64`. Then, everything will have a term.
```
message LogInfoProto {
TermIndexProto lastSnapshot = 1;
TermIndexProto applied = 2;
TermIndexProto committed = 3;
TermIndexProto lastEntry = 4;
}
```
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -418,7 +425,7 @@ message RaftClientReplyProto {
}
uint64 logIndex = 14; // When the request is a write request and the reply is success, the log index of the transaction
- repeated CommitInfoProto commitInfos = 15;
+ repeated PeerInfoProto peerInfos = 15;
Review Comment:
We should not rename the proto. Otherwise, it is an incompatible change.
BTW, we should change only `GroupInfoReplyProto` and leave other protos unchanged.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #1047:
URL: https://github.com/apache/ratis/pull/1047#issuecomment-1974007113
> @DaveTeng0 , Let's add the peer info (i.e. applyIndex etc.) to `GroupInfoReply`. Then, we won't need two different info methods.
>
> Sorry that I said `GroupInfoRequest` was sent to the leader. Indeed, it was sent to a particular server specified in the parameter in getGroupManagementApi
>
> https://github.com/apache/ratis/blob/1bd451046b2d33fe063712d7e811262bef580a4e/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java#L63
>
> .)
oh sure! yeah I definitely could move those into GroupInfoReply! Thanks Nicholas for the suggestion!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #1047:
URL: https://github.com/apache/ratis/pull/1047#issuecomment-1973996458
@DaveTeng0 , Let's add the peer info (i.e. applyIndex etc.) to `GroupInfoReply`. Then, we won't need two different info methods.
Sorry that I said `GroupInfoRequest` was sent to the leader. Indeed, it was sent to a particular server specified in the parameter in getGroupManagementApi https://github.com/apache/ratis/blob/1bd451046b2d33fe063712d7e811262bef580a4e/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java#L63 .)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #1047:
URL: https://github.com/apache/ratis/pull/1047
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516508811
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,12 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+message LogInfoProto {
+ TermIndexProto lastSnapshot = 1;
+ TermIndexProto applied = 2;
+ TermIndexProto committed = 3;
Review Comment:
added! Please take a final look, thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516902645
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,15 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+/** Add new LogInfoProto for RATIS-2030, allow GroupInfoCommand to show each server's last committed log,
+ last applied log, last snapshot log, last entry log.*/
+message LogInfoProto {
+ TermIndexProto lastSnapshot = 1;
+ TermIndexProto applied = 2;
+ TermIndexProto committed = 3;
+ TermIndexProto lastEntry = 4;
+}
Review Comment:
sure! updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to reply of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #1047:
URL: https://github.com/apache/ratis/pull/1047#issuecomment-1979653550
> @DaveTeng0 , Let's add the peer info (i.e. applyIndex etc.) to `GroupInfoReply`. Then, we won't need two different info methods.
>
> Sorry that I said `GroupInfoRequest` was sent to the leader. Indeed, it was sent to a particular server specified in the parameter in getGroupManagementApi
>
> https://github.com/apache/ratis/blob/1bd451046b2d33fe063712d7e811262bef580a4e/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java#L63
>
> .)
Hey @szetszwo I merged the peer information (applied index, snapshot index, current term) into response of GroupInfoCommand. Please help to take another look, and feel free to let me know if there is any question, thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1513824568
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -418,7 +425,7 @@ message RaftClientReplyProto {
}
uint64 logIndex = 14; // When the request is a write request and the reply is success, the log index of the transaction
- repeated CommitInfoProto commitInfos = 15;
+ repeated PeerInfoProto peerInfos = 15;
Review Comment:
sure, updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516661910
##########
ratis-common/src/main/java/org/apache/ratis/util/ProtoUtils.java:
##########
@@ -225,10 +226,20 @@ static String toString(CommitInfoProto proto) {
return RaftPeerId.valueOf(proto.getServer().getId()) + ":c" + proto.getCommitIndex();
}
+ static String toString(LogInfoProto proto) {
+ return "lastSnapshot: " + proto.getLastSnapshot()
+ + ", applied: " + proto.getApplied()
+ + ", committed: " + proto.getCommitted();
+ }
+
static String toString(Collection<CommitInfoProto> protos) {
return protos.stream().map(ProtoUtils::toString).collect(Collectors.toList()).toString();
}
+ static String toStringLogInfo(Collection<LogInfoProto> protos) {
+ return protos.stream().map(ProtoUtils::toString).collect(Collectors.toList()).toString();
+ }
+
Review Comment:
These two `toString` methods are not used. Let's remove them?
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
Review Comment:
Use `getStateMachine().getLastAppliedTermIndex()`.
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,15 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+/** Add new LogInfoProto for RATIS-2030, allow GroupInfoCommand to show each server's last committed log,
+ last applied log, last snapshot log, last entry log.*/
+message LogInfoProto {
+ TermIndexProto lastSnapshot = 1;
+ TermIndexProto applied = 2;
+ TermIndexProto committed = 3;
+ TermIndexProto lastEntry = 4;
+}
Review Comment:
Since `LogInfoProto` is only used in `GroupInfoReplyProto`. Let move it to next to `GroupInfoReplyProto`.
##########
ratis-common/src/main/java/org/apache/ratis/protocol/GroupInfoReply.java:
##########
@@ -33,25 +34,27 @@ public class GroupInfoReply extends RaftClientReply {
private final RoleInfoProto roleInfoProto;
private final boolean isRaftStorageHealthy;
private final RaftConfigurationProto conf;
+ private LogInfoProto logInfoProto;
Review Comment:
add `final`.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(TermIndex.valueOf(currentTerm, getRaftLog().getLastCommittedIndex()).toProto());
+ logInfoBuilder.setLastEntry(getRaftLog().getLastEntryTermIndex().toProto());
+ if (getStateMachine().getLatestSnapshot() != null) {
+ logInfoBuilder.setLastSnapshot(
+ TermIndex.valueOf(currentTerm,
+ getStateMachine().getLatestSnapshot().getIndex())
+ .toProto());
+ }
Review Comment:
Don't call the same method (`getLatestSnapshot()` in this case) multiple times unless it is returning a constant (i.e. calling `getStateMachine()` multiple times is okay):
1. They may return different values.
2. It may be expensive.
Also, use `snapshot.getTermIndex()`:
After all the changes, it should look like:
```java
LogInfoProto getLogInfo(){
final RaftLog log = getRaftLog();
final LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder()
.setApplied(getStateMachine().getLastAppliedTermIndex().toProto())
.setCommitted(log.getTermIndex(log.getLastCommittedIndex()).toProto())
.setLastEntry(log.getLastEntryTermIndex().toProto());
final SnapshotInfo snapshot = getStateMachine().getLatestSnapshot();
if (snapshot != null) {
logInfoBuilder.setLastSnapshot(snapshot.getTermIndex().toProto());
}
return logInfoBuilder.build();
}
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(TermIndex.valueOf(currentTerm, getRaftLog().getLastCommittedIndex()).toProto());
Review Comment:
We need to get the term from the log since the entry can be committed in an earlier term.
```java
.setCommitted(log.getTermIndex(log.getLastCommittedIndex()).toProto())
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516901662
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(TermIndex.valueOf(currentTerm, getRaftLog().getLastCommittedIndex()).toProto());
+ logInfoBuilder.setLastEntry(getRaftLog().getLastEntryTermIndex().toProto());
+ if (getStateMachine().getLatestSnapshot() != null) {
+ logInfoBuilder.setLastSnapshot(
+ TermIndex.valueOf(currentTerm,
+ getStateMachine().getLatestSnapshot().getIndex())
+ .toProto());
+ }
Review Comment:
updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516509383
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,9 +643,24 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
}
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(getRaftLog().getLastEntryTermIndex().toProto());
Review Comment:
fixed!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516901142
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,7 +643,22 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
+ }
+
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(TermIndex.valueOf(currentTerm, getRaftLog().getLastCommittedIndex()).toProto());
+ logInfoBuilder.setLastEntry(getRaftLog().getLastEntryTermIndex().toProto());
+ if (getStateMachine().getLatestSnapshot() != null) {
+ logInfoBuilder.setLastSnapshot(
+ TermIndex.valueOf(currentTerm,
+ getStateMachine().getLatestSnapshot().getIndex())
+ .toProto());
+ }
Review Comment:
Thanks Nicholas for the pointer!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1509626229
##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer/InfoCommand.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.shell.cli.sh.peer;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.OptionGroup;
+import org.apache.commons.cli.Options;
+import org.apache.ratis.client.RaftClient;
+import org.apache.ratis.protocol.PeerInfoReply;
+import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.shell.cli.RaftUtils;
+import org.apache.ratis.shell.cli.sh.command.AbstractRatisCommand;
+import org.apache.ratis.shell.cli.sh.command.Context;
+
+import java.io.IOException;
+
+
+public class InfoCommand extends AbstractRatisCommand{
Review Comment:
yeah, at first I also think it should be like that, but then I look at all the other ratis command names under package https://github.com/apache/ratis/tree/master/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer are:
- [AddCommand](https://github.com/apache/ratis/blob/master/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer/AddCommand.java)
- [RemoveCommand.java](https://github.com/apache/ratis/blob/master/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer/RemoveCommand.java)
- [SetPriorityCommand.java](https://github.com/apache/ratis/blob/master/ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer/SetPriorityCommand.java)
, so I kind of just follow that naming convention..
but you're also right since I did see other command like for 'Group', they have 'group' as prefix in class name.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1515134904
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,12 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+message LogInfoProto {
+ TermIndexProto lastSnapshot = 1;
+ TermIndexProto applied = 2;
+ TermIndexProto committed = 3;
Review Comment:
Please add also `TermIndexProto lastEntry` (which is the last entry written to the log).
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,9 +643,24 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
}
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(getRaftLog().getLastEntryTermIndex().toProto());
Review Comment:
This is the last entry, not the committed entry. We should use `RaftLog.getLastCommittedIndex()`.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1513824332
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -554,6 +561,6 @@ message GroupInfoReplyProto {
RaftGroupProto group = 2;
RoleInfoProto role = 3;
bool isRaftStorageHealthy = 4;
- repeated CommitInfoProto commitInfos = 5;
+ repeated PeerInfoProto peerInfos = 5;
Review Comment:
sure, updated!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516493103
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -642,9 +643,24 @@ GroupInfoReply getGroupInfo(GroupInfoRequest request) {
final RaftConfigurationProto conf =
LogProtoUtils.toRaftConfigurationProtoBuilder(getRaftConf()).build();
return new GroupInfoReply(request, getCommitInfos(), getGroup(), getRoleInfoProto(),
- dir.isHealthy(), conf);
+ dir.isHealthy(), conf, getLogInfo());
}
+ LogInfoProto getLogInfo(){
+ long currentTerm = getInfo().getCurrentTerm();
+ LogInfoProto.Builder logInfoBuilder = LogInfoProto.newBuilder();
+ logInfoBuilder.setApplied(TermIndex.valueOf(currentTerm, getInfo().getLastAppliedIndex()).toProto());
+ logInfoBuilder.setCommitted(getRaftLog().getLastEntryTermIndex().toProto());
Review Comment:
yeah that makes sense! Thanks Nicholas for catching that!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add peer info to response of GroupInfoCommand CLI [ratis]
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1516491961
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -175,6 +175,12 @@ message CommitInfoProto {
uint64 commitIndex = 2;
}
+message LogInfoProto {
+ TermIndexProto lastSnapshot = 1;
+ TermIndexProto applied = 2;
+ TermIndexProto committed = 3;
Review Comment:
yeah! i missed it, adding! Thanks Nicholas!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Ratis-2031. Add framework of PeerInfoCommand CLI [ratis]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #1047:
URL: https://github.com/apache/ratis/pull/1047#discussion_r1508413645
##########
ratis-common/src/main/java/org/apache/ratis/protocol/PeerInfoReply.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.protocol;
+
+import org.apache.ratis.proto.RaftProtos;
+
+import java.util.Collection;
+import java.util.List;
+
+public class PeerInfoReply extends RaftClientReply {
+
+ private RaftGroup group;
+ private RaftProtos.RoleInfoProto roleInfoProto;
+ private long currentTerm;
+ private long lastCommitIndex;
+ private long lastAppliedIndex;
+ private List<Long> followerNextIdx;
+ private long lastSnapshotIndex;
+
+ @SuppressWarnings("parameternumber")
+ public PeerInfoReply(RaftClientRequest request,
Review Comment:
ideally this class should have a build style helper method.
##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/peer/InfoCommand.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.shell.cli.sh.peer;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.OptionGroup;
+import org.apache.commons.cli.Options;
+import org.apache.ratis.client.RaftClient;
+import org.apache.ratis.protocol.PeerInfoReply;
+import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.shell.cli.RaftUtils;
+import org.apache.ratis.shell.cli.sh.command.AbstractRatisCommand;
+import org.apache.ratis.shell.cli.sh.command.Context;
+
+import java.io.IOException;
+
+
+public class InfoCommand extends AbstractRatisCommand{
Review Comment:
should it be called PeerInfoCommand to be consistent with other commands (GroupInfoCommand)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org