[jira] [Commented] (RATIS-514) Check if leader and follower have same configuration for installSnapshot

[Tsz Wo Nicholas Sze (JIRA)]

    [ 
https://issues.apache.org/jira/browse/RATIS-514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16809973#comment-16809973
 ] 

Tsz Wo Nicholas Sze commented on RATIS-514:
-------------------------------------------

Thanks [~hanishakoneru].  The patch looks good.  Some minor comments:
- Let's change the error messages as below:
-* GrpcLogAppender
{code}
        case CONF_MISMATCH:
          LOG.error("{}: Configuration Mismatch ({}): Leader {} has it set to 
{} but follower {} has it set to {}",
              server.getId(), 
RaftServerConfigKeys.Log.Appender.INSTALL_SNAPSHOT_ENABLED_KEY,
              server.getId(), installSnapshotEnabled, getFollowerId(), 
!installSnapshotEnabled);
{code}
-* RaftServerImpl
{code}
    LOG.error("{}: Configuration Mismatch ({}): Leader {} has it set to {} but 
follower {} has it set to {}",
        getId(), RaftServerConfigKeys.Log.Appender.INSTALL_SNAPSHOT_ENABLED_KEY,
        leaderId, request.hasSnapshotChunk(), getId(), installSnapshotEnabled);
{code}
- Pass Builder instead of proto for intermediate variable so that it can avoid 
unnecessary serialization.
{code}
  static InstallSnapshotRequestProto toInstallSnapshotRequestProto(
      RaftPeerId requestorId, RaftPeerId replyId, RaftGroupId groupId, String 
requestId, int requestIndex,
      long term, TermIndex lastTermIndex, List<FileChunkProto> chunks, long 
totalSize, boolean done) {
    final InstallSnapshotRequestProto.SnapshotChunkProto.Builder 
snapshotChunkBuilder =
        InstallSnapshotRequestProto.SnapshotChunkProto.newBuilder()
            .setRequestId(requestId)
            .setRequestIndex(requestIndex)
            .setTermIndex(toTermIndexProto(lastTermIndex))
            .addAllFileChunks(chunks)
            .setTotalSize(totalSize)
            .setDone(done);
    return InstallSnapshotRequestProto.newBuilder()
        .setServerRequest(toRaftRpcRequestProtoBuilder(requestorId, replyId, 
groupId))
        // .setRaftConfiguration()  TODO: save and pass RaftConfiguration
        .setLeaderTerm(term)
        .setSnapshotChunk(snapshotChunkBuilder)
        .build();
  }

  static InstallSnapshotRequestProto toInstallSnapshotRequestProto(
      RaftPeerId requestorId, RaftPeerId replyId, RaftGroupId groupId,
      long leaderTerm, TermIndex firstAvailable) {
    final InstallSnapshotRequestProto.NotificationProto.Builder 
notificationBuilder =
        InstallSnapshotRequestProto.NotificationProto.newBuilder()
            .setFirstAvailableTermIndex(toTermIndexProto(firstAvailable));
    return InstallSnapshotRequestProto.newBuilder()
        .setServerRequest(toRaftRpcRequestProtoBuilder(requestorId, replyId, 
groupId))
        // .setRaftConfiguration()  TODO: save and pass RaftConfiguration
        .setLeaderTerm(leaderTerm)
        .setNotification(notificationBuilder)
        .build();
  }
{code}


> Check if leader and follower have same configuration for installSnapshot
> ------------------------------------------------------------------------
>
>                 Key: RATIS-514
>                 URL: https://issues.apache.org/jira/browse/RATIS-514
>             Project: Ratis
>          Issue Type: Improvement
>          Components: server
>            Reporter: Hanisha Koneru
>            Assignee: Hanisha Koneru
>            Priority: Major
>         Attachments: RATIS-514.001.patch
>
>
> It is possible that the installSnapshot setting is different between leader 
> and follower. In such cases, we should log an error.
> Also, before processing the request, we should check whether this setting is 
> same between the leader and follower.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)