You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2021/08/09 23:30:01 UTC

[GitHub] [ratis] hanishakoneru opened a new pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

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


   ## What changes were proposed in this pull request?
   
   When a peer is bootstrapping and joins the Ratis ring via SetConf request, the Leader adds the peer to the sender list and sends it either the append log entries or the snapshot or a notification to install snapshot via State Machine. The leader sends the snapshot only when some or all of the ratis logs have been purged. 
   
   The snapshot is maintained by the State Machine and as such could contain information not present in the Ratis logs (for example Ozone's OzoneManager StateMachine maintains the snapshot as a DB file and it could contain information which cannot be constructed just using the Ratis logs - HDDS-5338). 
   
   To avoid this scenario, this Jira proposes that whenever a new peer is bootstrapping, the leader should send an install snapshot request at least once.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/RATIS-1390
   
   ## How was this patch tested?
   
   Will add a unit test in next iteration.
   


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



[GitHub] [ratis] hanishakoneru commented on a change in pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #489:
URL: https://github.com/apache/ratis/pull/489#discussion_r691671305



##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/leader/FollowerInfo.java
##########
@@ -52,6 +52,14 @@
   /** Set follower's snapshotIndex. */
   void setSnapshotIndex(long newSnapshotIndex);
 
+  /** Acknowledge that Follower attempted to install a snapshot. It does not guarantee that the installation was
+   * successful. This helps to determine whether Follower can come out of bootstrap process. */
+  void ackInstallSnapshotAttempt();

Review comment:
       Done.




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



[GitHub] [ratis] hanishakoneru commented on a change in pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #489:
URL: https://github.com/apache/ratis/pull/489#discussion_r691671372



##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -148,6 +148,7 @@ enum InstallSnapshotResult {
   IN_PROGRESS = 2;
   ALREADY_INSTALLED = 3;
   CONF_MISMATCH = 4;
+  NULL_SNAPSHOT = 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



[GitHub] [ratis] hanishakoneru commented on pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on pull request #489:
URL: https://github.com/apache/ratis/pull/489#issuecomment-901496503


   Thanks @szetszwo for the review. I have addressed your comments and added 2 unit tests. 


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



[GitHub] [ratis] hanishakoneru commented on pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on pull request #489:
URL: https://github.com/apache/ratis/pull/489#issuecomment-902809698


   The failing tests look unrelated and pass locally. I have retriggered CI.


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



[GitHub] [ratis] szetszwo merged pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

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


   


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



[GitHub] [ratis] szetszwo commented on a change in pull request #489: RATIS-1390. Bootstrapping Peer should always try to install a snapshot the first time.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #489:
URL: https://github.com/apache/ratis/pull/489#discussion_r689215340



##########
File path: ratis-server-api/src/main/java/org/apache/ratis/server/leader/FollowerInfo.java
##########
@@ -52,6 +52,14 @@
   /** Set follower's snapshotIndex. */
   void setSnapshotIndex(long newSnapshotIndex);
 
+  /** Acknowledge that Follower attempted to install a snapshot. It does not guarantee that the installation was
+   * successful. This helps to determine whether Follower can come out of bootstrap process. */
+  void ackInstallSnapshotAttempt();

Review comment:
       Let's use a name similar to hasAttemptedToInstallSnapshot().  How about "setAttemptedToInstallSnapshot"?

##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -148,6 +148,7 @@ enum InstallSnapshotResult {
   IN_PROGRESS = 2;
   ALREADY_INSTALLED = 3;
   CONF_MISMATCH = 4;
+  NULL_SNAPSHOT = 5;

Review comment:
       How about calling it as SNAPSHOT_UNAVALIABLE?
   




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