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

ASF GitHub Bot commented on ZOOKEEPER-2549:
-------------------------------------------

Github user yufeldman commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/99#discussion_r90553827
  
    --- Diff: src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java ---
    @@ -165,31 +163,35 @@ public void process(WatchedEvent event) {
         @Override
         public void sendResponse(ReplyHeader h, Record r, String tag)
                 throws IOException {
    -        if (!channel.isOpen()) {
    -            return;
    -        }
    -        ByteArrayOutputStream baos = new ByteArrayOutputStream();
    -        // Make space for length
    -        BinaryOutputArchive bos = BinaryOutputArchive.getArchive(baos);
             try {
    -            baos.write(fourBytes);
    -            bos.writeRecord(h, "header");
    -            if (r != null) {
    -                bos.writeRecord(r, tag);
    +            if (!channel.isOpen()) {
    +                return;
                 }
    -            baos.close();
    -        } catch (IOException e) {
    -            LOG.error("Error serializing response");
    -        }
    -        byte b[] = baos.toByteArray();
    -        ByteBuffer bb = ByteBuffer.wrap(b);
    -        bb.putInt(b.length - 4).rewind();
    -        sendBuffer(bb);
    -        if (h.getXid() > 0) {
    -            // zks cannot be null otherwise we would not have gotten here!
    -            if 
(!zkServer.shouldThrottle(outstandingCount.decrementAndGet())) {
    -                enableRecv();
    +            ByteArrayOutputStream baos = new ByteArrayOutputStream();
    +            // Make space for length
    +            BinaryOutputArchive bos = BinaryOutputArchive.getArchive(baos);
    +            try {
    +                baos.write(fourBytes);
    +                bos.writeRecord(h, "header");
    +                if (r != null) {
    +                    bos.writeRecord(r, tag);
    +                }
    +                baos.close();
    +            } catch (IOException e) {
    --- End diff --
    
    I did not modify this code - it was like that before, but potentially - yes 
it makes sense to rethrow
    I would say there are multiple places I came across where exceptions are 
swallowed 


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

Reply via email to