You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/11/03 22:15:34 UTC

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

hanishakoneru opened a new pull request #253:
URL: https://github.com/apache/incubator-ratis/pull/253


   ## What changes were proposed in this pull request?
   
   If a Follower is lagging behind and needs to install snapshot to catch up, it can miss any configuration changes that were applied on the Leader. When installing snapshot, the configuration change must also be propagated and applied on the follower. 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/RATIS-1128
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   (If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
   


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



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

Posted by 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



[GitHub] [incubator-ratis] szetszwo merged pull request #253: RATIS-1128. Update Configuration on InstallSnapshot

Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #253:
URL: https://github.com/apache/incubator-ratis/pull/253


   


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