You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by "Eli Reisman (JIRA)" <ji...@apache.org> on 2012/06/22 02:46:42 UTC
[jira] [Created] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Eli Reisman created GIRAPH-213:
----------------------------------
Summary: NettyClient.stop() could deadlock according to netty.io docs
Key: GIRAPH-213
URL: https://issues.apache.org/jira/browse/GIRAPH-213
Project: Giraph
Issue Type: Improvement
Reporter: Eli Reisman
Assignee: Eli Reisman
Priority: Trivial
NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403416#comment-13403416 ]
Jakob Homan commented on GIRAPH-213:
------------------------------------
ok, that sounds reasonable.
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eli Reisman updated GIRAPH-213:
-------------------------------
Attachment: GIRAPH-213-1.patch
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403699#comment-13403699 ]
Eli Reisman commented on GIRAPH-213:
------------------------------------
Just rolled back the patches added today since the build is havign trouble with BspTestCase and tried this patch again. It compiles and tests with full 'mvn verify' so I don't think its the problem. A lot of stuff got committed today.
one issue is some ofthe patches might (probably!) use input or output formats that my other patch consolidated under the lib/ directory, this could break those patches. Don't know what the protocol is here to fix this. could be something else too. I might try to layer on a few more of the ones commmited today to see where it dies.
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13400083#comment-13400083 ]
Eli Reisman commented on GIRAPH-213:
------------------------------------
By way of explanation, the patch is a bit goofy looking, but using the ChannelFuture for completion is apparently the netty.io recommended way to go. The future event will (apparently) be triggered regardless of the success or failure of any given close operation, so as soon as everyone is accounted for, the ClientBootstrap's thread pools (etc) are guaranteed to be released (i.e. zero chance of bootstrap not releasing due to failed Channel closures, at least as it is explained on netty.io)
Not an important improvement, just more housekeeping.
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402729#comment-13402729 ]
Eli Reisman commented on GIRAPH-213:
------------------------------------
No, according to netty.io the method used in the NettyClient.stop() is not safe and can deadlock. They recommend never using it. Even though the future method looks funky, a read of the docs indicates the futures get their event calls either way also. So as long as we use the Atomic counter to make sure we don't release resources before every Future gets its event call, we are safe. I ran this patch with jobs and also with full 'mvn verify' tests and it seems to work fine, no problems. Again, its not about the open/closed status of the connections, its just about when to release the resources, so instant return from the call doesn't hurt us. Apparently the existing call looks reasonable (barrier until resources can be released) it is deprecated.
The NettyServer does it too, but at least it sets a timeout per-call. Problem is, this holds up the return of the call while NOT guaranteeing the connections are actually closed in time for the resource release, which is bad. There is a separate ChannelGroupFuture I could use to fix that one too, but I thought I would start with this one as it had no timeout and deadlock is a bigger risk (I assume?) than releasing resources before the Channels are guaranteed to be closed, though neither is good.
Let me know what you decide.
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402737#comment-13402737 ]
Eli Reisman commented on GIRAPH-213:
------------------------------------
To summarize: even when you need a synchronous barrier event like this, the netty.io folks recommend the Future method with synch channels like this. I know it looks odd. Seen stuff like this in "Concurrency in Practice" too. The current way is deprecated. Big "BAD Don't do this" on the site :)
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403426#comment-13403426 ]
Hudson commented on GIRAPH-213:
-------------------------------
Integrated in Giraph-trunk-Commit #128 (See [https://builds.apache.org/job/Giraph-trunk-Commit/128/])
GIRAPH-213. NettyClient.stop() could deadlock according to netty.io docs. Contributed by Eli Reisman. (Revision 1355135)
Result = FAILURE
jghoman : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1355135
Files :
* /giraph/trunk/CHANGELOG
* /giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403704#comment-13403704 ]
Eli Reisman commented on GIRAPH-213:
------------------------------------
The JIRA to consolidate all IO under lib/ is not yet committed, so that wasn't it. I rolled back to 219, patched in this + 127 and they compile and test fine again. Something that went in today is doing it. it is one of the bsp tests that fails.
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-213) NettyClient.stop() could deadlock
according to netty.io docs
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13402483#comment-13402483 ]
Jakob Homan commented on GIRAPH-213:
------------------------------------
NettyClient.stop() itself should be synchronous, correct? It looks like this moves all the closing connections to asynch. Do we need to wait for them to return before returning from stop?
> NettyClient.stop() could deadlock according to netty.io docs
> ------------------------------------------------------------
>
> Key: GIRAPH-213
> URL: https://issues.apache.org/jira/browse/GIRAPH-213
> Project: Giraph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: Eli Reisman
> Assignee: Eli Reisman
> Priority: Trivial
> Fix For: 0.2.0
>
> Attachments: GIRAPH-213-1.patch
>
>
> NettyClient.stop() does not follow netty.io best practices and could potentially deadlock?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira