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 2022/02/22 13:00:12 UTC

[GitHub] [ratis] guohao-rosicky opened a new pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

guohao-rosicky opened a new pull request #606:
URL: https://github.com/apache/ratis/pull/606


   ## What changes were proposed in this pull request?
   
   Fix NettyServerStreamRpc.Proxies#getProxy NPE
   
   ```
   2022-02-22 17:58:44,541 [fc12e60f-0cae-475e-8233-5b08287c70cf-NettyServerStreamRpc-workerGroup--thread1] WARN org.apache.ratis.netty.server.DataStreamManagement: Failed to process DataStreamRequestByteBuf:clientId=client-61AA87234F6E,type=STREAM_HEADER,id=23008,offset=0,length=323
   java.util.concurrent.CompletionException: java.lang.NullPointerException: fc12e60f-0cae-475e-8233-5b08287c70cf-NettyServerStreamRpc: Server b0403548-9f78-4fef-bca3-0121bf708629 not found: peers=[]
           at org.apache.ratis.netty.server.DataStreamManagement.newStreamInfo(DataStreamManagement.java:251)
           at org.apache.ratis.netty.server.DataStreamManagement.lambda$readImpl$4(DataStreamManagement.java:411)
           at org.apache.ratis.util.MemoizedSupplier.get(MemoizedSupplier.java:62)
           at org.apache.ratis.netty.server.DataStreamManagement.lambda$readImpl$5(DataStreamManagement.java:412)
           at java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1660)
           at org.apache.ratis.netty.server.DataStreamManagement$StreamMap.computeIfAbsent(DataStreamManagement.java:187)
           at org.apache.ratis.netty.server.DataStreamManagement.readImpl(DataStreamManagement.java:412)
           at org.apache.ratis.netty.server.DataStreamManagement.read(DataStreamManagement.java:387)
           at org.apache.ratis.netty.server.NettyServerStreamRpc$1.channelRead(NettyServerStreamRpc.java:199)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
           at org.apache.ratis.thirdparty.io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
           at org.apache.ratis.thirdparty.io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
           at org.apache.ratis.thirdparty.io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
           at org.apache.ratis.thirdparty.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
           at org.apache.ratis.thirdparty.io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
           at org.apache.ratis.thirdparty.io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:795)
           at org.apache.ratis.thirdparty.io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:480)
           at org.apache.ratis.thirdparty.io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:378)
           at org.apache.ratis.thirdparty.io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
           at org.apache.ratis.thirdparty.io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
           at java.lang.Thread.run(Thread.java:748)
   Caused by: java.lang.NullPointerException: fc12e60f-0cae-475e-8233-5b08287c70cf-NettyServerStreamRpc: Server b0403548-9f78-4fef-bca3-0121bf708629 not found: peers=[]
           at java.util.Objects.requireNonNull(Objects.java:290)
           at org.apache.ratis.util.PeerProxyMap.getProxy(PeerProxyMap.java:110)
           at org.apache.ratis.netty.server.NettyServerStreamRpc$Proxies.getDataStreamOutput(NettyServerStreamRpc.java:100)
           at org.apache.ratis.netty.server.NettyServerStreamRpc$Proxies.getDataStreamOutput(NettyServerStreamRpc.java:88)
           at org.apache.ratis.netty.server.DataStreamManagement$StreamInfo.<init>(DataStreamManagement.java:126)
           at org.apache.ratis.netty.server.DataStreamManagement.newStreamInfo(DataStreamManagement.java:249)
           ... 26 more
   ```
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/RATIS-1529


-- 
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] guohao-rosicky commented on pull request #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #606:
URL: https://github.com/apache/ratis/pull/606#issuecomment-1048832769


   It has been changed to Catch IOE. @szetszwo 


-- 
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] guohao-rosicky commented on pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #606:
URL: https://github.com/apache/ratis/pull/606#issuecomment-1047773813


   @szetszwo What do you think of this change?
   
   Update the proxyMap when an exception occurs.


-- 
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 #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

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



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,12 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));

Review comment:
       Please revert the formatting change.  Thanks.

##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,12 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, true);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);

Review comment:
       Please revert the formatting change.  Thanks.




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r812511327



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       @szetszwo  I agree, I will submit another PR later to solve the NPE problem, this PR can be changed to catch IOException




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r815254797



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,12 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, true);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);

Review comment:
       @szetszwo  Modified, Please take a look. Thanks.




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r811970125



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       I fixed it. Take a look. @szetszwo 
   This problem I suspect is NettyServerStreamRpc#newClient when the peer is null.




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r812511327



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       I agree, I will submit another PR later to solve the NPE problem, this PR can be changed to catch IOException




-- 
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 #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

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



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       @guohao-rosicky , I mean we should not expect there is a NPE in runtime.  So, even catching Exception is incorrect.  We should fix the root cause of it.




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r812941126



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       NPE is a problem when initializing Client connections. I will add assertions or retry in another PR.
   @szetszwo 




-- 
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 #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

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



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       @guohao-rosicky , this is not about the title of the JIRA.  Please see how could we fix the NPE.  We really should not catch Exception in NettyServerStreamRpc.  It hides a bug instead of fixing it.




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r812011898



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       I agree with that. This PR will also solve other problems. I can open another JIRA and continue to follow the PROBLEMS of NPE.
   
   What do you think? @szetszwo 




-- 
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] guohao-rosicky commented on pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #606:
URL: https://github.com/apache/ratis/pull/606#issuecomment-1047858348


   I agree with that. This PR will also solve other problems. I can open another JIRA and continue to follow the PROBLEMS of NPE.
   
   What do you think?


-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r812026981



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       I changed this PR to handle when there is an exception in NettyServerStreamRpc#getDataStreamOutput
   
   create new jira: NettyServerStreamRpc.Proxies#getProxy NPE
   
   https://issues.apache.org/jira/browse/RATIS-1531




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r811970125



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       I fixed it. @szetszwo Please take a look. 
   This problem I suspect is NettyServerStreamRpc#newClient when the peer is null.




-- 
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] guohao-rosicky removed a comment on pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky removed a comment on pull request #606:
URL: https://github.com/apache/ratis/pull/606#issuecomment-1047858348


   I agree with that. This PR will also solve other problems. I can open another JIRA and continue to follow the PROBLEMS of NPE.
   
   What do you think?


-- 
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 #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

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



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       @guohao-rosicky , we should NEVER catch NullPointerException in production code since it indicates a bug.  Could you see what is the cause of it?




-- 
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] guohao-rosicky commented on a change in pull request #606: RATIS-1529. Fix NettyServerStreamRpc.Proxies#getProxy NPE

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #606:
URL: https://github.com/apache/ratis/pull/606#discussion_r811958666



##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/server/NettyServerStreamRpc.java
##########
@@ -97,9 +97,17 @@ private void getDataStreamOutput(RaftClientRequest request, Set<RaftPeer> peers,
         throws IOException {
       for (RaftPeer peer : peers) {
         try {
-          outs.add((DataStreamOutputRpc) map.getProxy(peer.getId()).stream(request));
+          DataStreamClient client = map.getProxy(peer.getId());
+          outs.add((DataStreamOutputRpc) client.stream(request));
         } catch (IOException e) {
-          throw new IOException(map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+          map.handleException(peer.getId(), e, false);
+          throw new IOException(
+              map.getName() + ": Failed to getDataStreamOutput for " + peer, e);
+        } catch (NullPointerException e) {

Review comment:
       @szetszwo I changed it to catch all exceptions and then trigger map.handleException




-- 
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 #606: RATIS-1529. Handle when there is an exception in NettyServerStreamRpc# getDataStreamOutput

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


   


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