[GitHub] [incubator-ratis] hanishakoneru commented on a change in pull request #253: RATIS-1128. Update Configuration on InstallSnapshot

[GitBox <gi...@apache.org>]

hanishakoneru commented on a change in pull request #253:
URL: https://github.com/apache/incubator-ratis/pull/253#discussion_r516988281



##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/ServerProtoUtils.java
##########
@@ -384,22 +385,29 @@ static InstallSnapshotRequestProto toInstallSnapshotRequestProto(
             .addAllFileChunks(chunks)
             .setTotalSize(totalSize)
             .setDone(done);
+    // Set term to DEFAULT_TERM as this term is not going to used by installSnapshot to update the RaftConfiguration
+    final LogEntryProto confLogEntryProto = toLogEntryProto(raftConfiguration, DEFAULT_TERM,
+        raftConfiguration.getLogEntryIndex());

Review comment:
       The term of this LogEntryProto is not used to update the in memory RaftConfiguration (or ConfigurationManager). But it is updated in the Raft Meta conf file (raft-meta.conf).
   @szetszwo , @lokeshj1703, would it be better to send the correct term corresponding to the last conf entry while sending the installSnapshot?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org