Todd Lipcon has posted comments on this change.

Change subject: KUDU-1894 fixed deadlock in client.Connection
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7765/2/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java:

PS2, Line 578: inflightMessages.put
> Did you consider trying something like this? I can prototype and see if it 
I tried using:

    channel.getPipeline().execute(new Runnable() {
        @Override
        public void run() {
          cleanup("connection closed");
        }
      });

to try to defer the cleanup() call to the next loop of the IO thread... but it 
didn't work, because execute() tries to be smart and execute inline if the 
submitting thread is the IO thread itself.

It's funny because it seems there was some patch internal to SslHandler to fix 
https://github.com/netty/netty/issues/989#issuecomment-14854044 which tried to 
do the same thing, and then the commenter at the end points out that it 
shouldn't do anything due to exactly the above smartness.

So, we're back at square one and I guess something like your fix is required. 
Or some other way than the above to defer the cleanup() call out of the 
synchronous path back onto a different thread.


-- 
To view, visit http://gerrit.cloudera.org:8080/7765
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a0edc3e3accbcff60f2cde641ee470312bbd27a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to