[jira] [Comment Edited] (RATIS-607) StatusRuntimeException in GrpcServerProtocol service as these are not closed

["Tsz Wo Nicholas Sze (JIRA)" <ji...@apache.org>]

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

Tsz Wo Nicholas Sze edited comment on RATIS-607 at 7/2/19 3:06 AM:
-------------------------------------------------------------------

[~msingh], thanks for fixing this!  Some comments on the patch.
- In GrpcServerProtocolService, when the StreamObserver is completed/has error, we should remove it from the list.
- The generic code should be changed to handle all rpc implementaions but not sepecific to gRPC.
-* In RaftServerRpc, rename closeStreamObservers to close.
-* In LogUtils, just print the cause anyway.
{code}
        log.warn(message.get() + ": " + t + ", cause: " + t.getCause());
{code}
-* Let's move the GrpcLogAppender log to LogAppender.
{code}
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LogAppender.java
@@ -90,6 +90,7 @@ public class LogAppender {
         LOG.error(this + " unexpected exception", e);
         lifeCycle.transition(EXCEPTION);
       } finally {
+        LOG.info("{}: Closing {}", getFollower().getName(), getClass().getSimpleName());
         if (!lifeCycle.compareAndTransition(CLOSING, CLOSED)) {
           lifeCycle.transitionIfNotEqual(EXCEPTION);
         }
{code}
- The log message changes have overlap with RATIS-605.  Let's wait for RATIS-605 so that we could use RaftGroupMemberId, i.e. do not change RaftServerImpl and FollowerInfo here?
- Please add some tests.

See if you want to do the following JIRA here.  Or we may do it later.
- AppendRequestStreamObserver and InstallSnapshotRequestStreamObserver share quite a few common code, let's refactor them to extend the a common base class



was (Author: szetszwo):
[~msingh], thanks for fixing this!  Some comments on the patch.
- In GrpcServerProtocolService, when the StreamObserver is completed/has error, we should remove it from the list.
- The generic code should be changed to handle all rpc implementaions but not sepecific to gRPC.
-* In RaftServerRpc, rename closeStreamObservers to close.
-* In LogUtils, just print the cause anyway.
{code}
        log.warn(message.get() + ": " + t + ", cause:" + t.getCause());
{code}
-* Let's move the GrpcLogAppender log to LogAppender.
{code}
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LogAppender.java
@@ -90,6 +90,7 @@ public class LogAppender {
         LOG.error(this + " unexpected exception", e);
         lifeCycle.transition(EXCEPTION);
       } finally {
+        LOG.info("{}: Closing {}", getFollower().getName(), getClass().getSimpleName());
         if (!lifeCycle.compareAndTransition(CLOSING, CLOSED)) {
           lifeCycle.transitionIfNotEqual(EXCEPTION);
         }
{code}
- The log message changes have overlap with RATIS-605.  Let's wait for RATIS-605 so that we could use RaftGroupMemberId, i.e. do not change RaftServerImpl and FollowerInfo here?
- Please add some tests.

See if you want to do the following JIRA here.  Or we may do it later.
- AppendRequestStreamObserver and InstallSnapshotRequestStreamObserver share quite a few common code, let's refactor them to extend the a common base class


> StatusRuntimeException in GrpcServerProtocol service as these are not closed
> ----------------------------------------------------------------------------
>
>                 Key: RATIS-607
>                 URL: https://issues.apache.org/jira/browse/RATIS-607
>             Project: Ratis
>          Issue Type: Bug
>          Components: server
>    Affects Versions: 0.3.0
>            Reporter: Mukul Kumar Singh
>            Assignee: Mukul Kumar Singh
>            Priority: Major
>         Attachments: RATIS-607.001.patch
>
>
> a) When a node shutdowns or transitions from a follower to candidate, it does not shuts down the streamobservers to its leader.
> b) similarly when a leader transitions to a follower, it does not close the appenders to the followers cleanly.



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