[
https://issues.apache.org/jira/browse/ZOOKEEPER-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15718613#comment-15718613
]
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_r90762715
--- 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 --
Agree on re-throw the exception here. We could just remove the try - catch
block here given there is a new try - catch block added in this PR that wraps
the entire `sendResponse.`
We can start triaging other places where the exceptions are swallowed but
should re-throw after this PR merging in.
> 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)
