[
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718612#comment-15718612
]
ASF GitHub Bot commented on ZOOKEEPER-2549:
-------------------------------------------
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/99#discussion_r90763391
--- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java ---
@@ -716,7 +716,12 @@ public void process(WatchedEvent event) {
// Convert WatchedEvent to a type that can be sent over the wire
WatcherEvent e = event.getWrapper();
- sendResponse(h, e, "notification");
+ try {
+ sendResponse(h, e, "notification");
+ } catch (IOException ex) {
+ LOG.debug("Problem sending to " + getRemoteSocketAddress(),
ex);
+ close();
--- End diff --
>> It was not closing (I think) before as exception was swallowed since
sendResponse in NIOServerCnxn was not throwing IOException
Yes, I think the connection was not closing before in cases of exception
thrown from `NIOServerCnxn.sendResponse` which swallows everything. The change
in this PR changes the behavior by closing the connection in case of exceptions
occur in sendResponse. I am leaning towards the old behavior of NOT closing the
connection, because the connection looks pretty innocent - in fact
`NIOServerCnxn.sendResponse` does not involve any socket IO I believe, it just
queuing stuff to be send over sockets. So if something goes wrong, we just do
our best effort by logging what's wrong - rather than trying mess up with
sockets which seems out of responsibilities of `NIOServerCnxn.sendResponse`.
Similarly since `NIOServerCnxn.sendResponse` does not directly involve sockets,
there should not be any leaks in case sendResponse screw up.
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it
> can stop main ZK requests processing thread
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-2549
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2549
> Project: ZooKeeper
> Issue Type: Bug
> Components: server
> Affects Versions: 3.5.1
> Reporter: Yuliya Feldman
> Assignee: Yuliya Feldman
> Attachments: ZOOKEEPER-2549-2.patch, ZOOKEEPER-2549-3.patch,
> ZOOKEEPER-2549-3.patch, ZOOKEEPER-2549-4.patch, ZOOKEEPER-2549.patch,
> ZOOKEEPER-2549.patch, zookeeper-2549-1.patch
>
>
> As NettyServerCnxn.sendResponse() allows all the exception to bubble up it
> can stop main ZK requests processing thread and make Zookeeper server look
> like it is hanging, while it just can not process any request anymore.
> Idea is to catch all the exceptions in NettyServerCnxn.sendResponse() ,
> convert them to IOException and allow it propagating up
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
