[GitHub] [ratis] hanishakoneru commented on pull request #504: RATIS-1402. do not send extra rpc calls to follower when the follower is still installing a snapshot

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

hanishakoneru commented on pull request #504:
URL: https://github.com/apache/ratis/pull/504#issuecomment-929059212


   > the reason is installedSnapshotIndex.set(reply.getIndex()); is called in a separate thread in the completeFuture when the snapshot is installed successfully. if we do not wait until the installation is completed, then the following code will lost the chance to be called, and inProgressInstallSnapshotRequest will keep not null and thus checkInconsistentAppendEntries will always consider the snapshot installation is in progress.
   
   The code you mentioned above would be called in the next iteration of the installSnapshot notification. You can verify this from the following statement in the follower log:
   ```
   2021-09-10 14:31:35,854 [grpc-default-executor-0] INFO org.apache.ratis.server.RaftServer$Division: b1b06ffa-12b9-4093-a844-58467d6298b9@group-194BB21401F4: InstallSnapshot notification result: SNAPSHOT_INSTALLED, at index: 2
   ```
   If we send IN_PROGRESS notification to the leader, the leader tries again and in a subsequent attempt, it will get the SNAPSHOT_INSTALLED reply.
   
   We need to figure out why the follower thinks install snapshot is in progress even though it is not. That is what is causing the follower to fail append entries request. From the code path you mentioned above, the LOG statement is being printed in the follower log. So the next line in code `inProgressInstallSnapshotRequest.compareAndSet(firstAvailableLogTermIndex, null);` should also have been run. I am not sure what is happening there.
   
   > 1 use join. this is simple and clear. Although join would throw an unchecked exception which might cause the follower to shutdown, a single follower crash will not downgrade the performance of the whole raft group as long as the majority is alive. what is more, the unchecked exception may indicate some serious problems ，such as disk error or network error, so that we have a chance to find and fix the problem and then restart the raft peer.
   
   I don't think this will be so simple. For example, from Ozone perspective, it would not be desirable for an OzoneManager to crash if there was let's say a network blip or something which caused the install snapshot to fail. Shutting down a peer is an extreme step. We should avoid it especially when the peer is not in an inconsistent state and can be recovered.


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