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

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

Thanks for the update.  Some comments:
- RpcTimeout.onTimeout should be synchronized.
- RpcTimeout.callTimeout should be final.
- In RpcTimeout.onTimeout, the errorMsg type should be Supplier<String> so that 
the string won't be constructed if there is no error.
-* Let's also print the stack trace since the throwable is unexpected.
{code}
        LOG.error(errorMsg.get(), t);
{code}
- In GRpcLogAppender.onSuccess, request can be null due to timeout earlier so 
that the if-statement should be 
{code}
      if (request == null
          || request.getServerRequest().getCallId() != 
reply.getServerReply().getCallId()) {
        // If reply comes after timeout, the reply is ignored.
        LOG.warn("Ignoring reply: " + reply);
        return;
      }
{code}
-* The same comment also applies to onInconsistency.



> Support timeout for append entry requests(GrpcLogAppender)
> ----------------------------------------------------------
>
>                 Key: RATIS-217
>                 URL: https://issues.apache.org/jira/browse/RATIS-217
>             Project: Ratis
>          Issue Type: New Feature
>            Reporter: Lokesh Jain
>            Assignee: Lokesh Jain
>            Priority: Major
>         Attachments: RATIS-217.002.patch
>
>
> This jira aims to add timeout for append entry requests sent by 
> GrpcLogAppender.



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

Reply via email to