ibessonov commented on a change in pull request #218:
URL: https://github.com/apache/ignite-3/pull/218#discussion_r672230214
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -296,15 +304,16 @@ public Status addLearners(final String groupId, final
Configuration conf, final
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final AddLearnersRequest.Builder rb = AddLearnersRequest.newBuilder()
//
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ AddLearnersRequest rb = cliOptions.getRaftMessagesFactory()
+ .addLearnersRequest()
+ .groupId(groupId) //
Review comment:
```suggestion
.groupId(groupId)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1510,20 +1513,26 @@ private void readFollower(final ReadIndexRequest
request, final RpcResponseClosu
return;
}
// send request to leader.
- final ReadIndexRequest newRequest = ReadIndexRequest.newBuilder() //
- .mergeFrom(request) //
- .setPeerId(this.leaderId.toString()) //
+ final ReadIndexRequest newRequest =
raftOptions.getRaftMessagesFactory()
+ .readIndexRequest() //
+ .groupId(request.groupId())
+ .serverId(request.serverId())
+ .peerId(request.peerId())
+ .entriesList(request.entriesList())
+ .peerId(this.leaderId.toString()) //
.build();
this.rpcClientService.readIndex(this.leaderId.getEndpoint(),
newRequest, -1, closure);
}
- private void readLeader(final ReadIndexRequest request, final
ReadIndexResponse.Builder respBuilder,
- final RpcResponseClosure<ReadIndexResponse> closure) {
+ private void readLeader(ReadIndexRequest request,
RpcResponseClosure<ReadIndexResponse> closure) {
+ ReadIndexResponseBuilder respBuilder =
raftOptions.getRaftMessagesFactory().readIndexResponse();
+
final int quorum = getQuorum();
if (quorum <= 1) {
// Only one peer, fast path.
- respBuilder.setSuccess(true) //
- .setIndex(this.ballotBox.getLastCommittedIndex());
+ respBuilder
+ .success(true) //
Review comment:
```suggestion
.success(true)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -422,15 +433,15 @@ public Status transferLeader(final String groupId, final
Configuration conf, fin
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final TransferLeaderRequest.Builder rb =
TransferLeaderRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- if (!peer.isEmpty()) {
- rb.setPeerId(peer.toString());
- }
+ TransferLeaderRequest rb = cliOptions.getRaftMessagesFactory()
+ .transferLeaderRequest()
+ .groupId(groupId) //
Review comment:
```suggestion
.groupId(groupId)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1136,14 +1138,15 @@ private void electSelf() {
continue;
}
final OnRequestVoteRpcDone done = new
OnRequestVoteRpcDone(peer, this.currTerm, this);
- done.request = RequestVoteRequest.newBuilder() //
- .setPreVote(false) // It's not a pre-vote request.
- .setGroupId(this.groupId) //
- .setServerId(this.serverId.toString()) //
- .setPeerId(peer.toString()) //
- .setTerm(this.currTerm) //
- .setLastLogIndex(lastLogId.getIndex()) //
- .setLastLogTerm(lastLogId.getTerm()) //
+ done.request = raftOptions.getRaftMessagesFactory()
+ .requestVoteRequest()
+ .preVote(false) // It's not a pre-vote request.
+ .groupId(this.groupId) //
+ .serverId(this.serverId.toString()) //
+ .peerId(peer.toString()) //
+ .term(this.currTerm) //
+ .lastLogIndex(lastLogId.getIndex()) //
+ .lastLogTerm(lastLogId.getTerm()) //
Review comment:
```suggestion
.groupId(this.groupId)
.serverId(this.serverId.toString())
.peerId(peer.toString())
.term(this.currTerm)
.lastLogIndex(lastLogId.getIndex())
.lastLogTerm(lastLogId.getTerm())
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1872,46 +1883,48 @@ public Message handleAppendEntriesRequest(final
AppendEntriesRequest request, fi
boolean doUnlock = true;
final long startMs = Utils.monotonicMs();
this.writeLock.lock();
- final int entriesCount = request.getEntriesCount();
+ final int entriesCount = Utils.size(request.entriesList());
try {
if (!this.state.isActive()) {
LOG.warn("Node {} is not in active state, currTerm={}.",
getNodeId(), this.currTerm);
return RaftRpcFactory.DEFAULT //
- .newResponse(AppendEntriesResponse.getDefaultInstance(),
RaftError.EINVAL,
+ .newResponse(raftOptions.getRaftMessagesFactory(),
RaftError.EINVAL,
"Node %s is not in active state, state %s.",
getNodeId(), this.state.name());
}
final PeerId serverId = new PeerId();
- if (!serverId.parse(request.getServerId())) {
+ if (!serverId.parse(request.serverId())) {
LOG.warn("Node {} received AppendEntriesRequest from {}
serverId bad format.", getNodeId(),
- request.getServerId());
+ request.serverId());
return RaftRpcFactory.DEFAULT //
- .newResponse(AppendEntriesResponse.getDefaultInstance(),
RaftError.EINVAL,
- "Parse serverId failed: %s.", request.getServerId());
+ .newResponse(raftOptions.getRaftMessagesFactory(),
RaftError.EINVAL,
+ "Parse serverId failed: %s.", request.serverId());
}
// Check stale term
- if (request.getTerm() < this.currTerm) {
+ if (request.term() < this.currTerm) {
LOG.warn("Node {} ignore stale AppendEntriesRequest from {},
term={}, currTerm={}.", getNodeId(),
- request.getServerId(), request.getTerm(), this.currTerm);
- return AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setTerm(this.currTerm) //
+ request.serverId(), request.term(), this.currTerm);
+ return raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse()
+ .success(false) //
+ .term(this.currTerm) //
Review comment:
```suggestion
.success(false)
.term(this.currTerm)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -3231,38 +3252,41 @@ public Message handleTimeoutNowRequest(final
TimeoutNowRequest request, final Rp
boolean doUnlock = true;
this.writeLock.lock();
try {
- if (request.getTerm() != this.currTerm) {
+ if (request.term() != this.currTerm) {
final long savedCurrTerm = this.currTerm;
- if (request.getTerm() > this.currTerm) {
- stepDown(request.getTerm(), false, new
Status(RaftError.EHIGHERTERMREQUEST,
+ if (request.term() > this.currTerm) {
+ stepDown(request.term(), false, new
Status(RaftError.EHIGHERTERMREQUEST,
"Raft node receives higher term request"));
}
LOG.info("Node {} received TimeoutNowRequest from {} while
currTerm={} didn't match requestTerm={}.",
- getNodeId(), request.getPeerId(), savedCurrTerm,
request.getTerm());
- return TimeoutNowResponse.newBuilder() //
- .setTerm(this.currTerm) //
- .setSuccess(false) //
+ getNodeId(), request.peerId(), savedCurrTerm,
request.term());
+ return raftOptions.getRaftMessagesFactory()
+ .timeoutNowResponse()
+ .term(this.currTerm) //
+ .success(false) //
.build();
}
if (this.state != State.STATE_FOLLOWER) {
LOG.info("Node {} received TimeoutNowRequest from {}, while
state={}, term={}.", getNodeId(),
- request.getServerId(), this.state, this.currTerm);
- return TimeoutNowResponse.newBuilder() //
- .setTerm(this.currTerm) //
- .setSuccess(false) //
+ request.serverId(), this.state, this.currTerm);
+ return raftOptions.getRaftMessagesFactory()
+ .timeoutNowResponse()
+ .term(this.currTerm) //
+ .success(false) //
Review comment:
```suggestion
.term(this.currTerm)
.success(false)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -214,25 +222,25 @@ public Status changePeers(final String groupId, final
Configuration conf, final
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final ChangePeersRequest.Builder rb = ChangePeersRequest.newBuilder()
//
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : newPeers) {
- rb.addNewPeers(peer.toString());
- }
+ ChangePeersRequest rb = cliOptions.getRaftMessagesFactory()
+ .changePeersRequest()
+ .groupId(groupId)
+ .leaderId(leaderId.toString())
+
.newPeersList(newPeers.getPeers().stream().map(Object::toString).collect(toList()))
+ .build();
try {
- final Message result =
this.cliClientService.changePeers(leaderId.getEndpoint(), rb.build(),
null).get();
+ final Message result =
this.cliClientService.changePeers(leaderId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.changePeers(leaderId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -261,15 +269,15 @@ public Status resetPeer(final String groupId, final
PeerId peerId, final Configu
return new Status(-1, "Fail to init channel to %s", peerId);
}
- final ResetPeerRequest.Builder rb = ResetPeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setPeerId(peerId.toString());
- for (final PeerId peer : newPeers) {
- rb.addNewPeers(peer.toString());
- }
+ ResetPeerRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
ResetPeerRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -359,15 +368,16 @@ public Status removeLearners(final String groupId, final
Configuration conf, fin
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final RemoveLearnersRequest.Builder rb =
RemoveLearnersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ RemoveLearnersRequest rb = cliOptions.getRaftMessagesFactory()
+ .removeLearnersRequest()
+ .groupId(groupId) //
+ .leaderId(leaderId.toString())
+
.learnersList(learners.stream().map(Object::toString).collect(toList()))
+ .build();
try {
- final Message result =
this.cliClientService.removeLearners(leaderId.getEndpoint(), rb.build(),
null).get();
+ final Message result =
this.cliClientService.removeLearners(leaderId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.removeLearners(leaderId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -636,19 +651,21 @@ private PeerId findTargetPeer(final PeerId self, final
String groupId, final Con
throw new IllegalStateException("Fail to init channel to leader "
+ leaderId);
}
- final GetPeersRequest.Builder rb = GetPeersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setOnlyAlive(onlyGetAlive);
+ GetPeersRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
GetPeersRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/ChangePeersRequestProcessor.java
##########
@@ -69,14 +72,12 @@ protected Message processRequest0(final CliRequestContext
ctx, final ChangePeers
done.run(status);
}
else {
- ChangePeersResponse.Builder rb =
ChangePeersResponse.newBuilder();
- for (final PeerId peer : oldConf) {
- rb.addOldPeers(peer.toString());
- }
- for (final PeerId peer : conf) {
- rb.addNewPeers(peer.toString());
- }
- done.sendResponse(rb.build());
+ ChangePeersResponse rb = msgFactory().changePeersResponse()
Review comment:
```suggestion
ChangePeersResponse res = msgFactory().changePeersResponse()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/AddPeerRequestProcessor.java
##########
@@ -59,25 +61,33 @@ protected Message processRequest0(final CliRequestContext
ctx, final AddPeerRequ
done.run(status);
}
else {
- final AddPeerResponse.Builder rb =
AddPeerResponse.newBuilder();
+ List<String> oldPeersList = new ArrayList<>();
+ List<String> newPeersList = new ArrayList<>();
+
boolean alreadyExists = false;
for (final PeerId oldPeer : oldPeers) {
- rb.addOldPeers(oldPeer.toString());
- rb.addNewPeers(oldPeer.toString());
+ oldPeersList.add(oldPeer.toString());
+ newPeersList.add(oldPeer.toString());
if (oldPeer.equals(addingPeer)) {
alreadyExists = true;
}
}
if (!alreadyExists) {
- rb.addNewPeers(addingPeerIdStr);
+ newPeersList.add(addingPeerIdStr);
}
- done.sendResponse(rb.build());
+
+ AddPeerResponse rb = msgFactory().addPeerResponse()
Review comment:
```suggestion
AddPeerResponse res = msgFactory().addPeerResponse()
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorGroupTest.java
##########
@@ -279,15 +279,16 @@ private void mockSendEmptyEntries() {
}
private RpcRequests.AppendEntriesRequest
createEmptyEntriesRequestToPeer(final PeerId peerId) {
- return RpcRequests.AppendEntriesRequest.newBuilder() //
- .setGroupId("test") //
- .setServerId(new PeerId("localhost", 8081).toString()) //
- .setPeerId(peerId.toString()) //
- .setTerm(1) //
- .setPrevLogIndex(10) //
- .setPrevLogTerm(1) //
- .setCommittedIndex(0) //
- .setData(ByteString.EMPTY) //
+ return raftOptions.getRaftMessagesFactory()
+ .appendEntriesRequest()
+ .groupId("test") //
+ .serverId(new PeerId("localhost", 8081).toString()) //
+ .peerId(peerId.toString()) //
+ .term(1) //
+ .prevLogIndex(10) //
+ .prevLogTerm(1) //
+ .committedIndex(0) //
+ .data(ByteString.EMPTY) //
Review comment:
```suggestion
.groupId("test")
.serverId(new PeerId("localhost", 8081).toString())
.peerId(peerId.toString())
.term(1)
.prevLogIndex(10)
.prevLogTerm(1)
.committedIndex(0)
.data(ByteString.EMPTY)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -195,10 +198,11 @@ private Replicator testRpcReturnedError() {
final Replicator r = getReplicator();
assertNull(r.getBlockTimer());
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(12) //
- .setTerm(2) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(12) //
+ .term(2) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(false)
.lastLogIndex(12)
.term(2)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -216,10 +220,11 @@ public void testOnRpcReturnedRpcContinuousError() throws
Exception {
assertNotNull(timer);
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(12) //
- .setTerm(2) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse()
+ .success(false) //
+ .lastLogIndex(12) //
+ .term(2) //
Review comment:
```suggestion
.success(false)
.lastLogIndex(12)
.term(2)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1510,20 +1513,26 @@ private void readFollower(final ReadIndexRequest
request, final RpcResponseClosu
return;
}
// send request to leader.
- final ReadIndexRequest newRequest = ReadIndexRequest.newBuilder() //
- .mergeFrom(request) //
- .setPeerId(this.leaderId.toString()) //
+ final ReadIndexRequest newRequest =
raftOptions.getRaftMessagesFactory()
+ .readIndexRequest() //
Review comment:
```suggestion
.readIndexRequest()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1510,20 +1513,26 @@ private void readFollower(final ReadIndexRequest
request, final RpcResponseClosu
return;
}
// send request to leader.
- final ReadIndexRequest newRequest = ReadIndexRequest.newBuilder() //
- .mergeFrom(request) //
- .setPeerId(this.leaderId.toString()) //
+ final ReadIndexRequest newRequest =
raftOptions.getRaftMessagesFactory()
+ .readIndexRequest() //
+ .groupId(request.groupId())
+ .serverId(request.serverId())
+ .peerId(request.peerId())
+ .entriesList(request.entriesList())
+ .peerId(this.leaderId.toString()) //
Review comment:
```suggestion
.peerId(this.leaderId.toString())
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1920,47 +1933,49 @@ public Message handleAppendEntriesRequest(final
AppendEntriesRequest request, fi
if (entriesCount > 0 && this.snapshotExecutor != null &&
this.snapshotExecutor.isInstallingSnapshot()) {
LOG.warn("Node {} received AppendEntriesRequest while
installing snapshot.", getNodeId());
return RaftRpcFactory.DEFAULT //
- .newResponse(AppendEntriesResponse.getDefaultInstance(),
RaftError.EBUSY,
+ .newResponse(raftOptions.getRaftMessagesFactory(),
RaftError.EBUSY,
"Node %s:%s is installing snapshot.", this.groupId,
this.serverId);
}
- final long prevLogIndex = request.getPrevLogIndex();
- final long prevLogTerm = request.getPrevLogTerm();
+ final long prevLogIndex = request.prevLogIndex();
+ final long prevLogTerm = request.prevLogTerm();
final long localPrevLogTerm =
this.logManager.getTerm(prevLogIndex);
if (localPrevLogTerm != prevLogTerm) {
final long lastLogIndex = this.logManager.getLastLogIndex();
LOG.warn("Node {} reject term_unmatched AppendEntriesRequest
from {}, term={}, prevLogIndex={}, " +
"prevLogTerm={}, localPrevLogTerm={}, lastLogIndex={},
entriesSize={}.",
- getNodeId(), request.getServerId(), request.getTerm(),
prevLogIndex, prevLogTerm, localPrevLogTerm,
+ getNodeId(), request.serverId(), request.term(),
prevLogIndex, prevLogTerm, localPrevLogTerm,
lastLogIndex, entriesCount);
- return AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setTerm(this.currTerm) //
- .setLastLogIndex(lastLogIndex) //
+ return raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse()
+ .success(false) //
+ .term(this.currTerm) //
+ .lastLogIndex(lastLogIndex) //
Review comment:
```suggestion
.success(false)
.term(this.currTerm)
.lastLogIndex(lastLogIndex)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -243,10 +248,11 @@ public void testOnRpcReturnedRpcContinuousError() throws
Exception {
public void testOnRpcReturnedTermMismatch() {
final Replicator r = getReplicator();
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(12) //
- .setTerm(2) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(12) //
+ .term(2) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(false)
.lastLogIndex(12)
.term(2)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -2672,14 +2692,15 @@ private void preVote() {
continue;
}
final OnPreVoteRpcDone done = new OnPreVoteRpcDone(peer,
this.currTerm);
- done.request = RequestVoteRequest.newBuilder() //
- .setPreVote(true) // it's a pre-vote request.
- .setGroupId(this.groupId) //
- .setServerId(this.serverId.toString()) //
- .setPeerId(peer.toString()) //
- .setTerm(this.currTerm + 1) // next term
- .setLastLogIndex(lastLogId.getIndex()) //
- .setLastLogTerm(lastLogId.getTerm()) //
+ done.request = raftOptions.getRaftMessagesFactory()
+ .requestVoteRequest()
+ .preVote(true) // it's a pre-vote request.
+ .groupId(this.groupId) //
+ .serverId(this.serverId.toString()) //
+ .peerId(peer.toString()) //
Review comment:
```suggestion
.groupId(this.groupId)
.serverId(this.serverId.toString())
.peerId(peer.toString())
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/FSMCallerImpl.java
##########
@@ -647,18 +649,20 @@ private void doSnapshotLoad(final LoadSnapshotClosure
done) {
setError(e);
return;
}
- if (meta.getOldPeersCount() == 0) {
+ if (meta.oldPeersList() == null) {
// Joint stage is not supposed to be noticeable by end users.
final Configuration conf = new Configuration();
- for (int i = 0, size = meta.getPeersCount(); i < size; i++) {
- final PeerId peer = new PeerId();
- Requires.requireTrue(peer.parse(meta.getPeers(i)), "Parse peer
failed");
- conf.addPeer(peer);
+ if (meta.peersList() != null) {
Review comment:
I think that this value can never be null
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/RpcResponseFactory.java
##########
@@ -32,45 +33,44 @@
/**
* Creates a RPC response from status, return OK response when status is
null.
*
- * @param parent parent message
+ * @param msgFactory Raft message factory
* @param st status with response
* @return a response instance
*/
- default Message newResponse(final Message parent, final Status st) {
- if (st == null) {
- return newResponse(parent, 0, "OK");
- }
- return newResponse(parent, st.getCode(), st.getErrorMsg());
+ default Message newResponse(RaftMessagesFactory msgFactory, Status st) {
+ if (st == null)
+ return newResponse(msgFactory, 0, "OK");
+
+ return newResponse(msgFactory, st.getCode(), st.getErrorMsg());
}
/**
* Creates an error response with parameters.
*
- * @param parent parent message
+ * @param msgFactory Raft message factory
* @param error error with raft info
* @param fmt message with format string
* @param args arguments referenced by the format specifiers in the format
string
* @return a response instance
*/
- default Message newResponse(final Message parent, final RaftError error,
final String fmt, final Object... args) {
- return newResponse(parent, error.getNumber(), fmt, args);
+ default Message newResponse(RaftMessagesFactory msgFactory, RaftError
error, String fmt, Object... args) {
+ return newResponse(msgFactory, error.getNumber(), fmt, args);
}
/**
* Creates an error response with parameters.
*
- * @param parent parent message
+ * @param msgFactory Raft message factory
* @param code error code with raft info
* @param fmt message with format string
* @param args arguments referenced by the format specifiers in the format
string
* @return a response instance
*/
- default Message newResponse(final Message parent, final int code, final
String fmt, final Object... args) {
- final RpcRequests.ErrorResponse.Builder eBuilder =
RpcRequests.ErrorResponse.newBuilder();
- eBuilder.setErrorCode(code);
- if (fmt != null) {
- eBuilder.setErrorMsg(String.format(fmt, args));
- }
+ default Message newResponse(RaftMessagesFactory msgFactory, int code,
String fmt, Object... args) {
Review comment:
Any idea of why we ignore parent message?
##########
File path:
modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -73,6 +73,18 @@ public TypeSpec generateBuilderInterface(MessageClass
message) {
})
.collect(Collectors.toList());
+ // generate a getter for each getter in the original interface
Review comment:
```suggestion
// Generate a getter for each getter in the original interface.
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -105,23 +108,26 @@ public Status addPeer(final String groupId, final
Configuration conf, final Peer
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final AddPeerRequest.Builder rb = AddPeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setPeerId(peer.toString());
+
+ AddPeerRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
AddPeerRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1674,19 +1683,20 @@ public Message handlePreVoteRequest(final
RequestVoteRequest request) {
doUnlock = true;
this.writeLock.lock();
- final LogId requestLastLogId = new
LogId(request.getLastLogIndex(), request.getLastLogTerm());
+ final LogId requestLastLogId = new
LogId(request.lastLogIndex(), request.lastLogTerm());
granted = requestLastLogId.compareTo(lastLogId) >= 0;
LOG.info(
"Node {} received PreVoteRequest from {}, term={},
currTerm={}, granted={}, requestLastLogId={}, lastLogId={}.",
- getNodeId(), request.getServerId(), request.getTerm(),
this.currTerm, granted, requestLastLogId,
+ getNodeId(), request.serverId(), request.term(),
this.currTerm, granted, requestLastLogId,
lastLogId);
}
while (false);
- return RequestVoteResponse.newBuilder() //
- .setTerm(this.currTerm) //
- .setGranted(granted) //
+ return raftOptions.getRaftMessagesFactory()
+ .requestVoteResponse()
+ .term(this.currTerm) //
+ .granted(granted) //
Review comment:
```suggestion
.term(this.currTerm)
.granted(granted)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -2672,14 +2692,15 @@ private void preVote() {
continue;
}
final OnPreVoteRpcDone done = new OnPreVoteRpcDone(peer,
this.currTerm);
- done.request = RequestVoteRequest.newBuilder() //
- .setPreVote(true) // it's a pre-vote request.
- .setGroupId(this.groupId) //
- .setServerId(this.serverId.toString()) //
- .setPeerId(peer.toString()) //
- .setTerm(this.currTerm + 1) // next term
- .setLastLogIndex(lastLogId.getIndex()) //
- .setLastLogTerm(lastLogId.getTerm()) //
+ done.request = raftOptions.getRaftMessagesFactory()
+ .requestVoteRequest()
+ .preVote(true) // it's a pre-vote request.
+ .groupId(this.groupId) //
+ .serverId(this.serverId.toString()) //
+ .peerId(peer.toString()) //
+ .term(this.currTerm + 1) // next term
+ .lastLogIndex(lastLogId.getIndex()) //
+ .lastLogTerm(lastLogId.getTerm()) //
Review comment:
```suggestion
.lastLogIndex(lastLogId.getIndex())
.lastLogTerm(lastLogId.getTerm())
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -3231,38 +3252,41 @@ public Message handleTimeoutNowRequest(final
TimeoutNowRequest request, final Rp
boolean doUnlock = true;
this.writeLock.lock();
try {
- if (request.getTerm() != this.currTerm) {
+ if (request.term() != this.currTerm) {
final long savedCurrTerm = this.currTerm;
- if (request.getTerm() > this.currTerm) {
- stepDown(request.getTerm(), false, new
Status(RaftError.EHIGHERTERMREQUEST,
+ if (request.term() > this.currTerm) {
+ stepDown(request.term(), false, new
Status(RaftError.EHIGHERTERMREQUEST,
"Raft node receives higher term request"));
}
LOG.info("Node {} received TimeoutNowRequest from {} while
currTerm={} didn't match requestTerm={}.",
- getNodeId(), request.getPeerId(), savedCurrTerm,
request.getTerm());
- return TimeoutNowResponse.newBuilder() //
- .setTerm(this.currTerm) //
- .setSuccess(false) //
+ getNodeId(), request.peerId(), savedCurrTerm,
request.term());
+ return raftOptions.getRaftMessagesFactory()
+ .timeoutNowResponse()
+ .term(this.currTerm) //
+ .success(false) //
Review comment:
```suggestion
.term(this.currTerm)
.success(false)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -105,23 +108,26 @@ public Status addPeer(final String groupId, final
Configuration conf, final Peer
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final AddPeerRequest.Builder rb = AddPeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setPeerId(peer.toString());
+
+ AddPeerRequest rb = cliOptions.getRaftMessagesFactory()
+ .addPeerRequest()
+ .groupId(groupId)
+ .leaderId(leaderId.toString())
+ .peerId(peer.toString())
+ .build();
try {
- final Message result =
this.cliClientService.addPeer(leaderId.getEndpoint(), rb.build(), null).get();
+ final Message result =
this.cliClientService.addPeer(leaderId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.addPeer(leaderId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1920,47 +1933,49 @@ public Message handleAppendEntriesRequest(final
AppendEntriesRequest request, fi
if (entriesCount > 0 && this.snapshotExecutor != null &&
this.snapshotExecutor.isInstallingSnapshot()) {
LOG.warn("Node {} received AppendEntriesRequest while
installing snapshot.", getNodeId());
return RaftRpcFactory.DEFAULT //
- .newResponse(AppendEntriesResponse.getDefaultInstance(),
RaftError.EBUSY,
+ .newResponse(raftOptions.getRaftMessagesFactory(),
RaftError.EBUSY,
"Node %s:%s is installing snapshot.", this.groupId,
this.serverId);
}
- final long prevLogIndex = request.getPrevLogIndex();
- final long prevLogTerm = request.getPrevLogTerm();
+ final long prevLogIndex = request.prevLogIndex();
+ final long prevLogTerm = request.prevLogTerm();
final long localPrevLogTerm =
this.logManager.getTerm(prevLogIndex);
if (localPrevLogTerm != prevLogTerm) {
final long lastLogIndex = this.logManager.getLastLogIndex();
LOG.warn("Node {} reject term_unmatched AppendEntriesRequest
from {}, term={}, prevLogIndex={}, " +
"prevLogTerm={}, localPrevLogTerm={}, lastLogIndex={},
entriesSize={}.",
- getNodeId(), request.getServerId(), request.getTerm(),
prevLogIndex, prevLogTerm, localPrevLogTerm,
+ getNodeId(), request.serverId(), request.term(),
prevLogIndex, prevLogTerm, localPrevLogTerm,
lastLogIndex, entriesCount);
- return AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setTerm(this.currTerm) //
- .setLastLogIndex(lastLogIndex) //
+ return raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse()
+ .success(false) //
+ .term(this.currTerm) //
+ .lastLogIndex(lastLogIndex) //
.build();
}
if (entriesCount == 0) {
// heartbeat or probe request
- final AppendEntriesResponse.Builder respBuilder =
AppendEntriesResponse.newBuilder() //
- .setSuccess(true) //
- .setTerm(this.currTerm) //
- .setLastLogIndex(this.logManager.getLastLogIndex());
+ final AppendEntriesResponseBuilder respBuilder =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse()
+ .success(true) //
+ .term(this.currTerm) //
Review comment:
```suggestion
.success(true)
.term(this.currTerm)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -214,25 +222,25 @@ public Status changePeers(final String groupId, final
Configuration conf, final
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final ChangePeersRequest.Builder rb = ChangePeersRequest.newBuilder()
//
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : newPeers) {
- rb.addNewPeers(peer.toString());
- }
+ ChangePeersRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
ChangePeersRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -162,23 +168,25 @@ public Status removePeer(final String groupId, final
Configuration conf, final P
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final RemovePeerRequest.Builder rb = RemovePeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setPeerId(peer.toString());
+ RemovePeerRequest rb = cliOptions.getRaftMessagesFactory()
+ .removePeerRequest()
+ .groupId(groupId)
+ .leaderId(leaderId.toString())
+ .peerId(peer.toString())
+ .build();
try {
- final Message result =
this.cliClientService.removePeer(leaderId.getEndpoint(), rb.build(),
null).get();
+ final Message result =
this.cliClientService.removePeer(leaderId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.removePeer(leaderId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -162,23 +168,25 @@ public Status removePeer(final String groupId, final
Configuration conf, final P
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final RemovePeerRequest.Builder rb = RemovePeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setPeerId(peer.toString());
+ RemovePeerRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
RemovePeerRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
##########
@@ -1770,26 +1780,27 @@ public Message handleRequestVoteRequest(final
RequestVoteRequest request) {
doUnlock = true;
this.writeLock.lock();
// vote need ABA check after unlock&writeLock
- if (request.getTerm() != this.currTerm) {
+ if (request.term() != this.currTerm) {
LOG.warn("Node {} raise term {} when get lastLogId.",
getNodeId(), this.currTerm);
break;
}
- final boolean logIsOk = new LogId(request.getLastLogIndex(),
request.getLastLogTerm())
+ final boolean logIsOk = new LogId(request.lastLogIndex(),
request.lastLogTerm())
.compareTo(lastLogId) >= 0;
if (logIsOk && (this.votedId == null ||
this.votedId.isEmpty())) {
- stepDown(request.getTerm(), false, new
Status(RaftError.EVOTEFORCANDIDATE,
+ stepDown(request.term(), false, new
Status(RaftError.EVOTEFORCANDIDATE,
"Raft node votes for some candidate, step down to
restart election_timer."));
this.votedId = candidateId.copy();
this.metaStorage.setVotedFor(candidateId);
}
}
while (false);
- return RequestVoteResponse.newBuilder() //
- .setTerm(this.currTerm) //
- .setGranted(request.getTerm() == this.currTerm &&
candidateId.equals(this.votedId)) //
+ return raftOptions.getRaftMessagesFactory()
+ .requestVoteResponse()
+ .term(this.currTerm) //
+ .granted(request.term() == this.currTerm &&
candidateId.equals(this.votedId)) //
Review comment:
```suggestion
.term(this.currTerm)
.granted(request.term() == this.currTerm &&
candidateId.equals(this.votedId))
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -261,15 +269,15 @@ public Status resetPeer(final String groupId, final
PeerId peerId, final Configu
return new Status(-1, "Fail to init channel to %s", peerId);
}
- final ResetPeerRequest.Builder rb = ResetPeerRequest.newBuilder() //
- .setGroupId(groupId) //
- .setPeerId(peerId.toString());
- for (final PeerId peer : newPeers) {
- rb.addNewPeers(peer.toString());
- }
+ ResetPeerRequest rb = cliOptions.getRaftMessagesFactory()
+ .resetPeerRequest()
+ .groupId(groupId)
+ .peerId(peerId.toString())
+
.newPeersList(newPeers.getPeers().stream().map(Object::toString).collect(toList()))
+ .build();
try {
- final Message result =
this.cliClientService.resetPeer(peerId.getEndpoint(), rb.build(), null).get();
+ final Message result =
this.cliClientService.resetPeer(peerId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.resetPeer(peerId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -389,15 +399,16 @@ public Status resetLearners(final String groupId, final
Configuration conf, fina
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final ResetLearnersRequest.Builder rb =
ResetLearnersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ ResetLearnersRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
ResetLearnersRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -447,12 +458,14 @@ public Status snapshot(final String groupId, final PeerId
peer) {
return new Status(-1, "Fail to init channel to %s", peer);
}
- final SnapshotRequest.Builder rb = SnapshotRequest.newBuilder() //
- .setGroupId(groupId) //
- .setPeerId(peer.toString());
+ SnapshotRequest rb = cliOptions.getRaftMessagesFactory()
+ .snapshotRequest()
+ .groupId(groupId)
+ .peerId(peer.toString())
+ .build();
try {
- final Message result =
this.cliClientService.snapshot(peer.getEndpoint(), rb.build(), null).get();
+ final Message result =
this.cliClientService.snapshot(peer.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.snapshot(peer.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -359,15 +368,16 @@ public Status removeLearners(final String groupId, final
Configuration conf, fin
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final RemoveLearnersRequest.Builder rb =
RemoveLearnersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ RemoveLearnersRequest rb = cliOptions.getRaftMessagesFactory()
+ .removeLearnersRequest()
+ .groupId(groupId) //
Review comment:
```suggestion
.groupId(groupId)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -636,19 +651,21 @@ private PeerId findTargetPeer(final PeerId self, final
String groupId, final Con
throw new IllegalStateException("Fail to init channel to leader "
+ leaderId);
}
- final GetPeersRequest.Builder rb = GetPeersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString()) //
- .setOnlyAlive(onlyGetAlive);
+ GetPeersRequest rb = cliOptions.getRaftMessagesFactory()
+ .getPeersRequest()
+ .groupId(groupId)
+ .leaderId(leaderId.toString())
+ .onlyAlive(onlyGetAlive)
+ .build();
try {
- final Message result =
this.cliClientService.getPeers(leaderId.getEndpoint(), rb.build(), null).get(
+ final Message result =
this.cliClientService.getPeers(leaderId.getEndpoint(), rb, null).get(
Review comment:
```suggestion
final Message result =
this.cliClientService.getPeers(leaderId.getEndpoint(), req, null).get(
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -389,15 +399,16 @@ public Status resetLearners(final String groupId, final
Configuration conf, fina
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final ResetLearnersRequest.Builder rb =
ResetLearnersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ ResetLearnersRequest rb = cliOptions.getRaftMessagesFactory()
+ .resetLearnersRequest()
+ .groupId(groupId)
+ .leaderId(leaderId.toString())
+
.learnersList(learners.stream().map(Object::toString).collect(toList()))
+ .build();
try {
- final Message result =
this.cliClientService.resetLearners(leaderId.getEndpoint(), rb.build(),
null).get();
+ final Message result =
this.cliClientService.resetLearners(leaderId.getEndpoint(), rb, null).get();
Review comment:
```suggestion
final Message result =
this.cliClientService.resetLearners(leaderId.getEndpoint(), req, null).get();
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -447,12 +458,14 @@ public Status snapshot(final String groupId, final PeerId
peer) {
return new Status(-1, "Fail to init channel to %s", peer);
}
- final SnapshotRequest.Builder rb = SnapshotRequest.newBuilder() //
- .setGroupId(groupId) //
- .setPeerId(peer.toString());
+ SnapshotRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
SnapshotRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/CliServiceImpl.java
##########
@@ -359,15 +368,16 @@ public Status removeLearners(final String groupId, final
Configuration conf, fin
if (!this.cliClientService.connect(leaderId.getEndpoint())) {
return new Status(-1, "Fail to init channel to leader %s",
leaderId);
}
- final RemoveLearnersRequest.Builder rb =
RemoveLearnersRequest.newBuilder() //
- .setGroupId(groupId) //
- .setLeaderId(leaderId.toString());
- for (final PeerId peer : learners) {
- rb.addLearners(peer.toString());
- }
+
+ RemoveLearnersRequest rb = cliOptions.getRaftMessagesFactory()
Review comment:
```suggestion
RemoveLearnersRequest req = cliOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/ReadOnlyServiceImpl.java
##########
@@ -200,21 +201,26 @@ private void notifyFail(final Status status) {
}
private void executeReadIndexEvents(final List<ReadIndexEvent> events) {
- if (events.isEmpty()) {
+ if (events.isEmpty())
return;
- }
- final ReadIndexRequest.Builder rb = ReadIndexRequest.newBuilder() //
- .setGroupId(this.node.getGroupId()) //
- .setServerId(this.node.getServerId().toString());
- final List<ReadIndexState> states = new ArrayList<>(events.size());
+ ReadIndexRequestBuilder rb = raftOptions.getRaftMessagesFactory()
+ .readIndexRequest()
+ .groupId(this.node.getGroupId()) //
Review comment:
```suggestion
.groupId(this.node.getGroupId())
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/Replicator.java
##########
@@ -1640,11 +1645,14 @@ private void sendTimeoutNow(final boolean unlockId,
final boolean stopAfterFinis
}
private void sendTimeoutNow(final boolean unlockId, final boolean
stopAfterFinish, final int timeoutMs) {
- final TimeoutNowRequest.Builder rb = TimeoutNowRequest.newBuilder();
- rb.setTerm(this.options.getTerm());
- rb.setGroupId(this.options.getGroupId());
- rb.setServerId(this.options.getServerId().toString());
- rb.setPeerId(this.options.getPeerId().toString());
+ TimeoutNowRequest rb = raftOptions.getRaftMessagesFactory()
Review comment:
I'd renamed this one as well but github doesn't allow me to do this for
all usages.
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/AddLearnersRequestProcessor.java
##########
@@ -69,20 +70,26 @@ protected Message processRequest0(final CliRequestContext
ctx, final AddLearners
done.run(status);
}
else {
- final LearnersOpResponse.Builder rb =
LearnersOpResponse.newBuilder();
+ List<String> oldLearnersList = new ArrayList<>();
+ List<String> newLearnersList = new ArrayList<>();
for (final PeerId peer : oldLearners) {
- rb.addOldLearners(peer.toString());
- rb.addNewLearners(peer.toString());
+ oldLearnersList.add(peer.toString());
+ newLearnersList.add(peer.toString());
}
for (final PeerId peer : addingLearners) {
if (!oldLearners.contains(peer)) {
- rb.addNewLearners(peer.toString());
+ newLearnersList.add(peer.toString());
}
}
- done.sendResponse(rb.build());
+ LearnersOpResponse rb = msgFactory().learnersOpResponse()
Review comment:
```suggestion
LearnersOpResponse res = msgFactory().learnersOpResponse()
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/storage/snapshot/remote/RemoteFileCopier.java
##########
@@ -151,9 +151,10 @@ public void close() throws IOException {
}
private CopySession newCopySession(final String source) {
- final GetFileRequest.Builder reqBuilder = GetFileRequest.newBuilder()
//
- .setFilename(source) //
- .setReaderId(this.readId);
+ final GetFileRequestBuilder reqBuilder =
raftOptions.getRaftMessagesFactory()
+ .getFileRequest()
+ .filename(source) //
Review comment:
```suggestion
.filename(source)
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/CliRequests.java
##########
@@ -19,443 +19,145 @@
package org.apache.ignite.raft.jraft.rpc;
+import java.util.List;
+import org.apache.ignite.raft.jraft.RaftMessageGroup;
+import org.apache.ignite.network.annotations.Transferable;
+
public final class CliRequests {
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.ADD_PEER_REQUEST, autoSerializable =
false)
public interface AddPeerRequest extends Message {
- String getGroupId();
-
- String getLeaderId();
-
- String getPeerId();
-
- interface Builder {
- Builder setGroupId(String groupId);
+ String groupId();
- Builder setLeaderId(String leaderId);
+ String leaderId();
- Builder setPeerId(String peerId);
-
- AddPeerRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createAddPeerRequest();
- }
+ String peerId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.ADD_PEER_RESPONSE, autoSerializable =
false)
public interface AddPeerResponse extends Message {
- static Message getDefaultInstance() {
- return null;
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createAddPeerResponse();
- }
-
- java.util.List<String> getOldPeersList();
-
- int getOldPeersCount();
-
- String getOldPeers(int index);
-
- java.util.List<String> getNewPeersList();
-
- int getNewPeersCount();
-
- String getNewPeers(int index);
-
- public interface Builder {
- Builder addOldPeers(String oldPeersId);
-
- Builder addNewPeers(String newPeersId);
+ List<String> oldPeersList();
- AddPeerResponse build();
- }
+ List<String> newPeersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.REMOVE_PEER_REQUEST, autoSerializable =
false)
public interface RemovePeerRequest extends Message {
- String getGroupId();
+ String groupId();
- String getLeaderId();
+ String leaderId();
- String getPeerId();
-
- interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder setPeerId(String peerId);
-
- RemovePeerRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createRemovePeerRequest();
- }
+ String peerId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.REMOVE_PEER_RESPONSE, autoSerializable =
false)
public interface RemovePeerResponse extends Message {
- static Message getDefaultInstance() {
- return null;
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createRemovePeerResponse();
- }
-
- java.util.List<String> getOldPeersList();
-
- int getOldPeersCount();
+ List<String> oldPeersList();
- String getOldPeers(int index);
-
- java.util.List<String> getNewPeersList();
-
- int getNewPeersCount();
-
- String getNewPeers(int index);
-
- public interface Builder {
- Builder addOldPeers(String oldPeerId);
-
- Builder addNewPeers(String newPeerId);
-
- RemovePeerResponse build();
- }
+ List<String> newPeersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.CHANGE_PEERS_REQUEST, autoSerializable =
false)
public interface ChangePeersRequest extends Message {
- String getGroupId();
-
- String getLeaderId();
+ String groupId();
- java.util.List<String> getNewPeersList();
+ String leaderId();
- int getNewPeersCount();
-
- String getNewPeers(int index);
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder addNewPeers(String peerId);
-
- ChangePeersRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createChangePeerRequest();
- }
+ List<String> newPeersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.CHANGE_PEERS_RESPONSE, autoSerializable
= false)
public interface ChangePeersResponse extends Message {
+ List<String> oldPeersList();
- static Message getDefaultInstance() {
- return null;
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createChangePeerResponse();
- }
-
- java.util.List<String> getOldPeersList();
-
- int getOldPeersCount();
-
- String getOldPeers(int index);
-
- java.util.List<String> getNewPeersList();
-
- int getNewPeersCount();
-
- String getNewPeers(int index);
-
- public interface Builder {
- Builder addOldPeers(String oldPeerId);
-
- Builder addNewPeers(String newPeerId);
-
- ChangePeersResponse build();
- }
+ List<String> newPeersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.SNAPSHOT_REQUEST, autoSerializable =
false)
public interface SnapshotRequest extends Message {
- String getGroupId();
-
- String getPeerId();
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setPeerId(String peerId);
+ String groupId();
- SnapshotRequest build();
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createSnapshotRequest();
- }
+ String peerId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.RESET_PEER_REQUEST, autoSerializable =
false)
public interface ResetPeerRequest extends Message {
- String getGroupId();
-
- String getPeerId();
-
- java.util.List<String> getOldPeersList();
-
- int getOldPeersCount();
-
- String getOldPeers(int index);
-
- java.util.List<String> getNewPeersList();
-
- int getNewPeersCount();
-
- String getNewPeers(int index);
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setPeerId(String peerId);
+ String groupId();
- Builder addNewPeers(String peerId);
+ String peerId();
- ResetPeerRequest build();
- }
+ List<String> oldPeersList();
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createResetPeerRequest();
- }
+ List<String> newPeersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.TRANSFER_LEADER_REQUEST,
autoSerializable = false)
public interface TransferLeaderRequest extends Message {
- String getGroupId();
+ String groupId();
- String getLeaderId();
+ String leaderId();
- String getPeerId();
-
- boolean hasPeerId();
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder setPeerId(String peerId);
-
- TransferLeaderRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createTransferLeaderRequest();
- }
+ String peerId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.GET_LEADER_REQUEST, autoSerializable =
false)
public interface GetLeaderRequest extends Message {
- String getGroupId();
-
- String getPeerId();
-
- boolean hasPeerId();
+ String groupId();
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setPeerId(String peerId);
-
- GetLeaderRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createGetLeaderRequest();
- }
+ String peerId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.GET_LEADER_RESPONSE, autoSerializable =
false)
public interface GetLeaderResponse extends Message {
- static Message getDefaultInstance() {
- return null;
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createGetLeaderResponse();
- }
-
- String getLeaderId();
-
- public interface Builder {
- GetLeaderResponse build();
-
- Builder setLeaderId(String leaderId);
- }
+ String leaderId();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.GET_PEERS_REQUEST, autoSerializable =
false)
public interface GetPeersRequest extends Message {
- String getGroupId();
+ String groupId();
- String getLeaderId();
+ String leaderId();
- boolean getOnlyAlive();
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder setOnlyAlive(boolean onlyGetAlive);
-
- GetPeersRequest build();
- }
-
- public static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createGetPeersRequest();
- }
+ boolean onlyAlive();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.GET_PEERS_RESPONSE, autoSerializable =
false)
public interface GetPeersResponse extends Message {
- static Message getDefaultInstance() {
- return null;
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createGetPeersResponse();
- }
-
- java.util.List<String> getPeersList();
-
- int getPeersCount();
+ List<String> peersList();
- String getPeers(int index);
-
- java.util.List<String> getLearnersList();
-
- int getLearnersCount();
-
- String getLearners(int index);
-
- public interface Builder {
- Builder addPeers(String peerId);
-
- Builder addLearners(String learnerId);
-
- GetPeersResponse build();
- }
+ List<String> learnersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.ADD_LEARNERS_REQUEST, autoSerializable =
false)
public interface AddLearnersRequest extends Message {
- String getGroupId();
-
- String getLeaderId();
+ String groupId();
- java.util.List<String> getLearnersList();
+ String leaderId();
- int getLearnersCount();
-
- String getLearners(int index);
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder addLearners(String learnerId);
-
- AddLearnersRequest build();
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createAddLearnersRequest();
- }
+ List<String> learnersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.REMOVE_LEARNERS_REQUEST,
autoSerializable = false)
public interface RemoveLearnersRequest extends Message {
- String getGroupId();
-
- String getLeaderId();
-
- java.util.List<String> getLearnersList();
+ String groupId();
- int getLearnersCount();
+ String leaderId();
- String getLearners(int index);
-
- public interface Builder {
- Builder setGroupId(String groupId);
-
- Builder setLeaderId(String leaderId);
-
- Builder addLearners(String leaderId);
-
- RemoveLearnersRequest build();
- }
-
- static Builder newBuilder() {
- return MessageBuilderFactory.DEFAULT.createRemoveLearnersRequest();
- }
+ List<String> learnersList();
}
+ @Transferable(value =
RaftMessageGroup.RpcClientMessageGroup.RESET_LEARNERS_REQUEST, autoSerializable
= false)
public interface ResetLearnersRequest extends Message {
- String getGroupId();
-
- String getLeaderId();
-
- java.util.List<String> getLearnersList();
+ String groupId();
- /**
- * <code>repeated string learners = 3;</code>
- */
- int getLearnersCount();
+ String leaderId();
- /**
- * <code>repeated string learners = 3;</code>
Review comment:
Do you know what it is?
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -134,16 +137,16 @@ private void mockSendEmptyEntries(final boolean
isHeartbeat) {
}
private RpcRequests.AppendEntriesRequest createEmptyEntriesRequest(final
boolean isHeartbeat) {
- RpcRequests.AppendEntriesRequest.Builder rb =
RpcRequests.AppendEntriesRequest.newBuilder() //
- .setGroupId("test") //
- .setServerId(new PeerId("localhost", 8082).toString()) //
- .setPeerId(this.peerId.toString()) //
- .setTerm(1) //
- .setPrevLogIndex(10) //
- .setPrevLogTerm(1) //
- .setCommittedIndex(0);
+ AppendEntriesRequestBuilder rb =
raftOptions.getRaftMessagesFactory().appendEntriesRequest()
+ .groupId("test") //
+ .serverId(new PeerId("localhost", 8082).toString()) //
+ .peerId(this.peerId.toString()) //
+ .term(1) //
+ .prevLogIndex(10) //
+ .prevLogTerm(1) //
+ .committedIndex(0);
Review comment:
```suggestion
.groupId("test")
.serverId(new PeerId("localhost", 8082).toString())
.peerId(this.peerId.toString())
.term(1)
.prevLogIndex(10)
.prevLogTerm(1)
.committedIndex(0);
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/AddLearnersRequestProcessor.java
##########
@@ -69,20 +70,26 @@ protected Message processRequest0(final CliRequestContext
ctx, final AddLearners
done.run(status);
}
else {
- final LearnersOpResponse.Builder rb =
LearnersOpResponse.newBuilder();
+ List<String> oldLearnersList = new ArrayList<>();
+ List<String> newLearnersList = new ArrayList<>();
for (final PeerId peer : oldLearners) {
- rb.addOldLearners(peer.toString());
- rb.addNewLearners(peer.toString());
+ oldLearnersList.add(peer.toString());
+ newLearnersList.add(peer.toString());
}
for (final PeerId peer : addingLearners) {
if (!oldLearners.contains(peer)) {
- rb.addNewLearners(peer.toString());
+ newLearnersList.add(peer.toString());
}
}
- done.sendResponse(rb.build());
+ LearnersOpResponse rb = msgFactory().learnersOpResponse()
+ .oldLearnersList(oldLearnersList)
+ .newLearnersList(newLearnersList)
+ .build();
+
+ done.sendResponse(rb);
Review comment:
```suggestion
done.sendResponse(res);
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/AddPeerRequestProcessor.java
##########
@@ -59,25 +61,33 @@ protected Message processRequest0(final CliRequestContext
ctx, final AddPeerRequ
done.run(status);
}
else {
- final AddPeerResponse.Builder rb =
AddPeerResponse.newBuilder();
+ List<String> oldPeersList = new ArrayList<>();
+ List<String> newPeersList = new ArrayList<>();
+
boolean alreadyExists = false;
for (final PeerId oldPeer : oldPeers) {
- rb.addOldPeers(oldPeer.toString());
- rb.addNewPeers(oldPeer.toString());
+ oldPeersList.add(oldPeer.toString());
+ newPeersList.add(oldPeer.toString());
if (oldPeer.equals(addingPeer)) {
alreadyExists = true;
}
}
if (!alreadyExists) {
- rb.addNewPeers(addingPeerIdStr);
+ newPeersList.add(addingPeerIdStr);
}
- done.sendResponse(rb.build());
+
+ AddPeerResponse rb = msgFactory().addPeerResponse()
+ .newPeersList(newPeersList)
+ .oldPeersList(oldPeersList)
+ .build();
+
+ done.sendResponse(rb);
Review comment:
```suggestion
done.sendResponse(res);
```
##########
File path:
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/impl/cli/ChangePeersRequestProcessor.java
##########
@@ -69,14 +72,12 @@ protected Message processRequest0(final CliRequestContext
ctx, final ChangePeers
done.run(status);
}
else {
- ChangePeersResponse.Builder rb =
ChangePeersResponse.newBuilder();
- for (final PeerId peer : oldConf) {
- rb.addOldPeers(peer.toString());
- }
- for (final PeerId peer : conf) {
- rb.addNewPeers(peer.toString());
- }
- done.sendResponse(rb.build());
+ ChangePeersResponse rb = msgFactory().changePeersResponse()
+
.oldPeersList(oldConf.stream().map(Object::toString).collect(toList()))
+
.newPeersList(conf.getPeers().stream().map(Object::toString).collect(toList()))
+ .build();
+
+ done.sendResponse(rb);
Review comment:
```suggestion
done.sendResponse(res);
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -264,25 +270,28 @@ public void testOnRpcReturnedMoreLogs() {
final Replicator r = getReplicator();
assertEquals(11, r.getRealNextIndex());
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(12) //
- .setTerm(1) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(12) //
+ .term(1) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(false)
.lastLogIndex(12)
.term(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -642,25 +669,27 @@ public void testInstallSnapshot() {
Mockito.when(this.snapshotStorage.open()).thenReturn(reader);
final String uri = "remote://localhost:8081/99";
Mockito.when(reader.generateURIForCopy()).thenReturn(uri);
- final RaftOutter.SnapshotMeta meta =
RaftOutter.SnapshotMeta.newBuilder() //
- .setLastIncludedIndex(11) //
- .setLastIncludedTerm(1) //
+ final RaftOutter.SnapshotMeta meta =
raftOptions.getRaftMessagesFactory().snapshotMeta()
+ .lastIncludedIndex(11) //
+ .lastIncludedTerm(1) //
.build();
Mockito.when(reader.load()).thenReturn(meta);
assertEquals(0, r.statInfo.lastLogIncluded);
assertEquals(0, r.statInfo.lastTermIncluded);
- final RpcRequests.InstallSnapshotRequest.Builder rb =
RpcRequests.InstallSnapshotRequest.newBuilder();
- rb.setTerm(this.opts.getTerm());
- rb.setGroupId(this.opts.getGroupId());
- rb.setServerId(this.opts.getServerId().toString());
- rb.setPeerId(this.opts.getPeerId().toString());
- rb.setMeta(meta);
- rb.setUri(uri);
+ final RpcRequests.InstallSnapshotRequest rb =
raftOptions.getRaftMessagesFactory()
Review comment:
```suggestion
final RpcRequests.InstallSnapshotRequest req =
raftOptions.getRaftMessagesFactory()
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -302,25 +311,27 @@ public void testOnRpcReturnedLessLogs() {
final Replicator r = getReplicator();
assertEquals(11, r.getRealNextIndex());
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(8) //
- .setTerm(1) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(8) //
+ .term(1) //
.build();
this.id.unlock();
final Future<Message> rpcInFly = r.getRpcInFly();
assertNotNull(rpcInFly);
Mockito.when(this.logManager.getTerm(8)).thenReturn(1L);
- final RpcRequests.AppendEntriesRequest newReq =
RpcRequests.AppendEntriesRequest.newBuilder() //
- .setGroupId("test") //
- .setServerId(new PeerId("localhost", 8082).toString()) //
- .setPeerId(this.peerId.toString()) //
- .setTerm(1) //
- .setPrevLogIndex(8) //
- .setPrevLogTerm(1) //
- .setData(ByteString.EMPTY) //
- .setCommittedIndex(0) //
+ final RpcRequests.AppendEntriesRequest newReq =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesRequest()
+ .groupId("test") //
+ .serverId(new PeerId("localhost", 8082).toString()) //
+ .peerId(this.peerId.toString()) //
+ .term(1) //
+ .prevLogIndex(8) //
+ .prevLogTerm(1) //
+ .data(ByteString.EMPTY) //
+ .committedIndex(0) //
Review comment:
```suggestion
.appendEntriesRequest()
.groupId("test")
.serverId(new PeerId("localhost", 8082).toString())
.peerId(this.peerId.toString())
.term(1)
.prevLogIndex(8)
.prevLogTerm(1)
.data(ByteString.EMPTY)
.committedIndex(0)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -503,10 +528,11 @@ public void testOnHeartbeatReturnedOK() {
public void testOnHeartbeatReturnedTermMismatch() {
final Replicator r = getReplicator();
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(12) //
- .setTerm(2) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(12) //
+ .term(2) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(false)
.lastLogIndex(12)
.term(2)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -302,25 +311,27 @@ public void testOnRpcReturnedLessLogs() {
final Replicator r = getReplicator();
assertEquals(11, r.getRealNextIndex());
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(false) //
- .setLastLogIndex(8) //
- .setTerm(1) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(false) //
+ .lastLogIndex(8) //
+ .term(1) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(false)
.lastLogIndex(8)
.term(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -416,27 +428,37 @@ public void testContinueSendingEntries() throws Exception
{
final Future<Message> rpcInFly = r.getRpcInFly();
assertNotNull(rpcInFly);
- final RpcRequests.AppendEntriesRequest.Builder rb =
RpcRequests.AppendEntriesRequest.newBuilder() //
- .setGroupId("test") //
- .setServerId(new PeerId("localhost", 8082).toString()) //
- .setPeerId(this.peerId.toString()) //
- .setTerm(1) //
- .setPrevLogIndex(10) //
- .setPrevLogTerm(1) //
- .setCommittedIndex(0);
+ final AppendEntriesRequestBuilder rb =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesRequest()
+ .groupId("test") //
+ .serverId(new PeerId("localhost", 8082).toString()) //
+ .peerId(this.peerId.toString()) //
+ .term(1) //
+ .prevLogIndex(10) //
+ .prevLogTerm(1) //
Review comment:
```suggestion
.groupId("test")
.serverId(new PeerId("localhost", 8082).toString())
.peerId(this.peerId.toString())
.term(1)
.prevLogIndex(10)
.prevLogTerm(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -342,10 +353,11 @@ public void testOnRpcReturnedWaitMoreEntries() throws
Exception {
assertEquals(-1, r.getWaitId());
final RpcRequests.AppendEntriesRequest request =
createEmptyEntriesRequest();
- final RpcRequests.AppendEntriesResponse response =
RpcRequests.AppendEntriesResponse.newBuilder() //
- .setSuccess(true) //
- .setLastLogIndex(10) //
- .setTerm(1) //
+ final RpcRequests.AppendEntriesResponse response =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesResponse() //
+ .success(true) //
+ .lastLogIndex(10) //
+ .term(1) //
Review comment:
```suggestion
.appendEntriesResponse()
.success(true)
.lastLogIndex(10)
.term(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -676,9 +705,10 @@ public void testOnTimeoutNowReturnedTermMismatch() {
final Replicator r = getReplicator();
this.id.unlock();
final RpcRequests.TimeoutNowRequest request =
createTimeoutnowRequest();
- final RpcRequests.TimeoutNowResponse response =
RpcRequests.TimeoutNowResponse.newBuilder() //
- .setSuccess(false) //
- .setTerm(12) //
+ final RpcRequests.TimeoutNowResponse response =
raftOptions.getRaftMessagesFactory()
+ .timeoutNowResponse()
+ .success(false) //
+ .term(12) //
Review comment:
```suggestion
.success(false)
.term(12)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -770,25 +812,34 @@ private void
mockSendEntries(@SuppressWarnings("SameParameterValue") final int n
}
private RpcRequests.AppendEntriesRequest createEntriesRequest(final int n)
{
- final RpcRequests.AppendEntriesRequest.Builder rb =
RpcRequests.AppendEntriesRequest.newBuilder() //
- .setGroupId("test") //
- .setServerId(new PeerId("localhost", 8082).toString()) //
- .setPeerId(this.peerId.toString()) //
- .setTerm(1) //
- .setPrevLogIndex(10) //
- .setPrevLogTerm(1) //
- .setCommittedIndex(0);
+ final AppendEntriesRequestBuilder rb =
raftOptions.getRaftMessagesFactory()
+ .appendEntriesRequest()
+ .groupId("test") //
+ .serverId(new PeerId("localhost", 8082).toString()) //
+ .peerId(this.peerId.toString()) //
+ .term(1) //
+ .prevLogIndex(10) //
+ .prevLogTerm(1) //
Review comment:
```suggestion
.groupId("test")
.serverId(new PeerId("localhost", 8082).toString())
.peerId(this.peerId.toString())
.term(1)
.prevLogIndex(10)
.prevLogTerm(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -642,25 +669,27 @@ public void testInstallSnapshot() {
Mockito.when(this.snapshotStorage.open()).thenReturn(reader);
final String uri = "remote://localhost:8081/99";
Mockito.when(reader.generateURIForCopy()).thenReturn(uri);
- final RaftOutter.SnapshotMeta meta =
RaftOutter.SnapshotMeta.newBuilder() //
- .setLastIncludedIndex(11) //
- .setLastIncludedTerm(1) //
+ final RaftOutter.SnapshotMeta meta =
raftOptions.getRaftMessagesFactory().snapshotMeta()
+ .lastIncludedIndex(11) //
+ .lastIncludedTerm(1) //
.build();
Mockito.when(reader.load()).thenReturn(meta);
assertEquals(0, r.statInfo.lastLogIncluded);
assertEquals(0, r.statInfo.lastTermIncluded);
- final RpcRequests.InstallSnapshotRequest.Builder rb =
RpcRequests.InstallSnapshotRequest.newBuilder();
- rb.setTerm(this.opts.getTerm());
- rb.setGroupId(this.opts.getGroupId());
- rb.setServerId(this.opts.getServerId().toString());
- rb.setPeerId(this.opts.getPeerId().toString());
- rb.setMeta(meta);
- rb.setUri(uri);
+ final RpcRequests.InstallSnapshotRequest rb =
raftOptions.getRaftMessagesFactory()
+ .installSnapshotRequest()
+ .term(this.opts.getTerm())
+ .groupId(this.opts.getGroupId())
+ .serverId(this.opts.getServerId().toString())
+ .peerId(this.opts.getPeerId().toString())
+ .meta(meta)
+ .uri(uri)
+ .build();
Mockito.when(
-
this.rpcService.installSnapshot(eq(this.opts.getPeerId().getEndpoint()),
eq(rb.build()),
+
this.rpcService.installSnapshot(eq(this.opts.getPeerId().getEndpoint()), eq(rb),
Review comment:
```suggestion
this.rpcService.installSnapshot(eq(this.opts.getPeerId().getEndpoint()),
eq(req),
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -642,25 +669,27 @@ public void testInstallSnapshot() {
Mockito.when(this.snapshotStorage.open()).thenReturn(reader);
final String uri = "remote://localhost:8081/99";
Mockito.when(reader.generateURIForCopy()).thenReturn(uri);
- final RaftOutter.SnapshotMeta meta =
RaftOutter.SnapshotMeta.newBuilder() //
- .setLastIncludedIndex(11) //
- .setLastIncludedTerm(1) //
+ final RaftOutter.SnapshotMeta meta =
raftOptions.getRaftMessagesFactory().snapshotMeta()
+ .lastIncludedIndex(11) //
+ .lastIncludedTerm(1) //
Review comment:
```suggestion
.lastIncludedIndex(11)
.lastIncludedTerm(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/rpc/AppendEntriesBenchmark.java
##########
@@ -224,21 +230,21 @@ public void zeroCopy() {
final ByteBuffer buf = ByteBuffer.wrap(bytes);
dataBuffer.add(buf.slice());
}
- rb.setData(RecyclableByteBufferList.concatenate(dataBuffer));
- return rb.build().toByteArray();
+ rb.data(RecyclableByteBufferList.concatenate(dataBuffer));
+ return Marshaller.DEFAULT.marshall(rb.build());
}
finally {
RecycleUtil.recycle(dataBuffer);
}
}
- private static void fillCommonFields(final AppendEntriesRequest.Builder
rb) {
- rb.setTerm(1) //
- .setGroupId("1") //
- .setServerId("test") //
- .setPeerId("127.0.0.1:8080") //
- .setPrevLogIndex(2) //
- .setPrevLogTerm(3) //
- .setCommittedIndex(4);
+ private static void fillCommonFields(final AppendEntriesRequestBuilder rb)
{
+ rb.term(1) //
+ .groupId("1") //
+ .serverId("test") //
+ .peerId("127.0.0.1:8080") //
+ .prevLogIndex(2) //
+ .prevLogTerm(3) //
Review comment:
```suggestion
rb.term(1)
.groupId("1")
.serverId("test")
.peerId("127.0.0.1:8080")
.prevLogIndex(2)
.prevLogTerm(3)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/core/ReplicatorTest.java
##########
@@ -805,17 +856,18 @@ public void testGetNextSendIndex() {
private RpcRequests.InstallSnapshotRequest createInstallSnapshotRequest() {
final String uri = "remote://localhost:8081/99";
- final RaftOutter.SnapshotMeta meta =
RaftOutter.SnapshotMeta.newBuilder() //
- .setLastIncludedIndex(11) //
- .setLastIncludedTerm(1) //
+ final RaftOutter.SnapshotMeta meta =
raftOptions.getRaftMessagesFactory()
+ .snapshotMeta() //
+ .lastIncludedIndex(11) //
+ .lastIncludedTerm(1) //
Review comment:
```suggestion
.snapshotMeta()
.lastIncludedIndex(11)
.lastIncludedTerm(1)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/rpc/AbstractClientServiceTest.java
##########
@@ -161,9 +161,10 @@ public void testInvokeWithDoneOnException() throws
Exception {
public void testInvokeWithDoneOnErrorResponse() throws Exception {
final InvokeContext invokeCtx = new InvokeContext();
final ArgumentCaptor<InvokeCallback> callbackArg =
ArgumentCaptor.forClass(InvokeCallback.class);
- final CliRequests.GetPeersRequest request =
CliRequests.GetPeersRequest.newBuilder() //
- .setGroupId("id") //
- .setLeaderId("127.0.0.1:8001") //
+ final CliRequests.GetPeersRequest request =
rpcOptions.getRaftMessagesFactory()
+ .getPeersRequest()
+ .groupId("id") //
+ .leaderId("127.0.0.1:8001") //
Review comment:
```suggestion
.groupId("id")
.leaderId("127.0.0.1:8001")
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/rpc/TestMessageGroup.java
##########
@@ -6,32 +6,22 @@
* (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
+ * 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.ignite.raft.jraft.rpc.message;
-import org.apache.ignite.raft.jraft.rpc.RpcRequests;
+package org.apache.ignite.raft.jraft.rpc;
-class PingRequestImpl implements RpcRequests.PingRequest,
RpcRequests.PingRequest.Builder {
- private long sendTimestamp;
+import org.apache.ignite.network.annotations.MessageGroup;
- @Override public long getSendTimestamp() {
- return sendTimestamp;
- }
-
- @Override public Builder setSendTimestamp(long timestamp) {
- this.sendTimestamp = timestamp;
-
- return this;
- }
-
- @Override public RpcRequests.PingRequest build() {
- return this;
- }
+/**
+ * Message group for tests.
+ */
+@MessageGroup(groupType = 4, groupName = "TestRaftMessages")
Review comment:
I think we should use more obscure numbers for test groups, what do you
think?
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/storage/FileServiceTest.java
##########
@@ -129,19 +144,20 @@ public void testGetLargeFileData() throws IOException {
final long readerId =
FileService.getInstance().addReader(this.fileReader);
int fileOffset = 0;
while (true) {
- final RpcRequests.GetFileRequest request =
RpcRequests.GetFileRequest.newBuilder() //
- .setCount(4096).setFilename("data") //
- .setOffset(fileOffset) //
- .setReaderId(readerId) //
+ final RpcRequests.GetFileRequest request =
msgFactory.getFileRequest()
+ .count(4096)
+ .filename("data") //
+ .offset(fileOffset) //
+ .readerId(readerId) //
Review comment:
```suggestion
.filename("data")
.offset(fileOffset)
.readerId(readerId)
```
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/rpc/TestMessageGroup.java
##########
@@ -6,32 +6,22 @@
* (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
+ * 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.ignite.raft.jraft.rpc.message;
-import org.apache.ignite.raft.jraft.rpc.RpcRequests;
+package org.apache.ignite.raft.jraft.rpc;
-class PingRequestImpl implements RpcRequests.PingRequest,
RpcRequests.PingRequest.Builder {
- private long sendTimestamp;
+import org.apache.ignite.network.annotations.MessageGroup;
- @Override public long getSendTimestamp() {
- return sendTimestamp;
- }
-
- @Override public Builder setSendTimestamp(long timestamp) {
- this.sendTimestamp = timestamp;
-
- return this;
- }
-
- @Override public RpcRequests.PingRequest build() {
- return this;
- }
+/**
+ * Message group for tests.
+ */
+@MessageGroup(groupType = 4, groupName = "TestRaftMessages")
Review comment:
I'm afraid that we'll have collisions
##########
File path:
modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -73,6 +73,18 @@ public TypeSpec generateBuilderInterface(MessageClass
message) {
})
.collect(Collectors.toList());
+ // generate a getter for each getter in the original interface
Review comment:
I thought that we have a convention in Ignite. Seems like it's violated
in too many places.
##########
File path:
modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -73,6 +73,18 @@ public TypeSpec generateBuilderInterface(MessageClass
message) {
})
.collect(Collectors.toList());
+ // generate a getter for each getter in the original interface
Review comment:
I'm sure we have it for simple comments as well
##########
File path:
modules/raft/src/test/java/org/apache/ignite/raft/jraft/rpc/TestMessageGroup.java
##########
@@ -6,32 +6,22 @@
* (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
+ * 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.ignite.raft.jraft.rpc.message;
-import org.apache.ignite.raft.jraft.rpc.RpcRequests;
+package org.apache.ignite.raft.jraft.rpc;
-class PingRequestImpl implements RpcRequests.PingRequest,
RpcRequests.PingRequest.Builder {
- private long sendTimestamp;
+import org.apache.ignite.network.annotations.MessageGroup;
- @Override public long getSendTimestamp() {
- return sendTimestamp;
- }
-
- @Override public Builder setSendTimestamp(long timestamp) {
- this.sendTimestamp = timestamp;
-
- return this;
- }
-
- @Override public RpcRequests.PingRequest build() {
- return this;
- }
+/**
+ * Message group for tests.
+ */
+@MessageGroup(groupType = 4, groupName = "TestRaftMessages")
Review comment:
Ok
##########
File path:
modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -73,6 +73,18 @@ public TypeSpec generateBuilderInterface(MessageClass
message) {
})
.collect(Collectors.toList());
+ // generate a getter for each getter in the original interface
Review comment:
I assume that jraft code has comments in the notation that you used, so
never mind.
I'll ask the ame question next time in a proper Ignite code
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
