You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Maja Kabiljo <ma...@fb.com> on 2012/11/30 00:59:18 UTC
Review Request: GIRAPH-437: Missing progress calls when stopping Netty server
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/
-----------------------------------------------------------
Review request for giraph.
Description
-------
At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
This addresses bug GIRAPH-437.
https://issues.apache.org/jira/browse/GIRAPH-437
Diffs
-----
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
Diff: https://reviews.apache.org/r/8286/diff/
Testing
-------
mvn verify
Thanks,
Maja Kabiljo
Re: Review Request: GIRAPH-437: Missing progress calls when stopping Netty
server
Posted by Maja Kabiljo <ma...@fb.com>.
> On Nov. 30, 2012, 1:15 a.m., Avery Ching wrote:
> > +1, looks good to me. I like the Waitable interface.
Thanks for the review, Avery!
- Maja
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/#review13885
-----------------------------------------------------------
On Nov. 29, 2012, 11:59 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8286/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 11:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
>
> Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
>
>
> This addresses bug GIRAPH-437.
> https://issues.apache.org/jira/browse/GIRAPH-437
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
>
> Diff: https://reviews.apache.org/r/8286/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-437: Missing progress calls when stopping Netty
server
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/#review13885
-----------------------------------------------------------
Ship it!
+1, looks good to me. I like the Waitable interface.
- Avery Ching
On Nov. 29, 2012, 11:59 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8286/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 11:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
>
> Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
>
>
> This addresses bug GIRAPH-437.
> https://issues.apache.org/jira/browse/GIRAPH-437
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
>
> Diff: https://reviews.apache.org/r/8286/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-437: Missing progress calls when stopping Netty
server
Posted by Maja Kabiljo <ma...@fb.com>.
> On Nov. 30, 2012, 1:21 a.m., Nitay Joffe wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java, line 155
> > <https://reviews.apache.org/r/8286/diff/1/?file=231725#file231725line155>
> >
> > Why not just java Future? http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/Future.html
It has some methods we don't need and doesn't have the timeout result. So we'll have to extend it anyway. It's also nicer this way for WaitableWithoutResult.
- Maja
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/#review13886
-----------------------------------------------------------
On Nov. 29, 2012, 11:59 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8286/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 11:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
>
> Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
>
>
> This addresses bug GIRAPH-437.
> https://issues.apache.org/jira/browse/GIRAPH-437
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
>
> Diff: https://reviews.apache.org/r/8286/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-437: Missing progress calls when stopping Netty
server
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 30, 2012, 1:21 a.m., Nitay Joffe wrote:
> > http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java, line 155
> > <https://reviews.apache.org/r/8286/diff/1/?file=231725#file231725line155>
> >
> > Why not just java Future? http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/Future.html
>
> Maja Kabiljo wrote:
> It has some methods we don't need and doesn't have the timeout result. So we'll have to extend it anyway. It's also nicer this way for WaitableWithoutResult.
Gotcha, okay SG :)
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/#review13886
-----------------------------------------------------------
On Nov. 29, 2012, 11:59 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8286/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 11:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
>
> Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
>
>
> This addresses bug GIRAPH-437.
> https://issues.apache.org/jira/browse/GIRAPH-437
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
>
> Diff: https://reviews.apache.org/r/8286/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>
Re: Review Request: GIRAPH-437: Missing progress calls when stopping Netty
server
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8286/#review13886
-----------------------------------------------------------
http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java
<https://reviews.apache.org/r/8286/#comment29686>
Why not just java Future? http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/Future.html
- Nitay Joffe
On Nov. 29, 2012, 11:59 p.m., Maja Kabiljo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8286/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2012, 11:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> At the end of a long running job I got an exception about not reporting progress. The last log line was: "stop: Halting netty server", so I suspect it's because awaitUninterruptibly() call there.
>
> Changing awaitUninterruptibly to periodical calls of await. Added a log line to the end of NettyServer.stop so if it happens again we can be sure where the problem is. I also refactored ProgressableUtils so new cases where we need to wait for something will be easier to write.
>
>
> This addresses bug GIRAPH-437.
> https://issues.apache.org/jira/browse/GIRAPH-437
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/main/java/org/apache/giraph/utils/ProgressableUtils.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/ConnectionTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestFailureTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/RequestTest.java 1415035
> http://svn.apache.org/repos/asf/giraph/trunk/giraph/src/test/java/org/apache/giraph/comm/SaslConnectionTest.java 1415035
>
> Diff: https://reviews.apache.org/r/8286/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Maja Kabiljo
>
>