You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by Terence Yim <ch...@gmail.com> on 2014/03/06 22:57:59 UTC

Review Request 18864: Review for TWILL-52

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18864/
-----------------------------------------------------------

Review request for Twill.


Repository: twill


Description
-------

Fix for issue TWILL-52


Diffs
-----

  twill-core/src/main/java/org/apache/twill/internal/AbstractZKServiceController.java a132128 
  twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java 5d2f33c 
  twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java 7592176 
  twill-core/src/test/java/org/apache/twill/internal/ControllerTest.java 048e489 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java 8e0c6be 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java 9dff24d 
  twill-yarn/src/test/java/org/apache/twill/yarn/SessionExpireTestRun.java PRE-CREATION 
  twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestSuite.java bed613a 
  twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestUtils.java 079db56 

Diff: https://reviews.apache.org/r/18864/diff/


Testing
-------

Added new test case SessionExpireTestRun with all existing test cases passed on all profiles.


Thanks,

Terence Yim


Re: Review Request 18864: Review for TWILL-52

Posted by Andreas Neumann <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18864/#review36971
-----------------------------------------------------------

Ship it!


Ship It!

- Andreas Neumann


On March 12, 2014, 1:14 a.m., Terence Yim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18864/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 1:14 a.m.)
> 
> 
> Review request for Twill.
> 
> 
> Repository: twill
> 
> 
> Description
> -------
> 
> Fix for issue TWILL-52
> 
> 
> Diffs
> -----
> 
>   twill-core/src/main/java/org/apache/twill/internal/AbstractZKServiceController.java a132128 
>   twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java 5d2f33c 
>   twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java 7592176 
>   twill-core/src/test/java/org/apache/twill/internal/ControllerTest.java 048e489 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java 8e0c6be 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java 9dff24d 
>   twill-yarn/src/test/java/org/apache/twill/yarn/SessionExpireTestRun.java PRE-CREATION 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestSuite.java bed613a 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestUtils.java 079db56 
> 
> Diff: https://reviews.apache.org/r/18864/diff/
> 
> 
> Testing
> -------
> 
> Added new test case SessionExpireTestRun with all existing test cases passed on all profiles.
> 
> 
> Thanks,
> 
> Terence Yim
> 
>


Re: Review Request 18864: Review for TWILL-52

Posted by Terence Yim <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18864/
-----------------------------------------------------------

(Updated March 12, 2014, 1:14 a.m.)


Review request for Twill.


Repository: twill


Description
-------

Fix for issue TWILL-52


Diffs (updated)
-----

  twill-core/src/main/java/org/apache/twill/internal/AbstractZKServiceController.java a132128 
  twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java 5d2f33c 
  twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java 7592176 
  twill-core/src/test/java/org/apache/twill/internal/ControllerTest.java 048e489 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java 8e0c6be 
  twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java 9dff24d 
  twill-yarn/src/test/java/org/apache/twill/yarn/SessionExpireTestRun.java PRE-CREATION 
  twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestSuite.java bed613a 
  twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestUtils.java 079db56 

Diff: https://reviews.apache.org/r/18864/diff/


Testing
-------

Added new test case SessionExpireTestRun with all existing test cases passed on all profiles.


Thanks,

Terence Yim


Re: Review Request 18864: Review for TWILL-52

Posted by Terence Yim <ch...@gmail.com>.

> On March 7, 2014, 8:22 p.m., Andreas Neumann wrote:
> > twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java, line 157
> > <https://reviews.apache.org/r/18864/diff/1/?file=512992#file512992line157>
> >
> >     Does this happen often? Perhaps info will create very verbose logging here?

It should be rare. It only happens when ZK session expired, which could be due to network partition / client heartbeat failure (e.g. long GC pause).


> On March 7, 2014, 8:22 p.m., Andreas Neumann wrote:
> > twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java, line 210
> > <https://reviews.apache.org/r/18864/diff/1/?file=512994#file512994line210>
> >
> >     Can you explain somewhere what it means when this happens? How can it get into a state where the zk node is gone?

It is the ephemeral node created by the AM. The node will be gone if AM finished and disconnected from ZK or ZK session expiration happened (which should be rare).


- Terence


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18864/#review36572
-----------------------------------------------------------


On March 6, 2014, 9:57 p.m., Terence Yim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18864/
> -----------------------------------------------------------
> 
> (Updated March 6, 2014, 9:57 p.m.)
> 
> 
> Review request for Twill.
> 
> 
> Repository: twill
> 
> 
> Description
> -------
> 
> Fix for issue TWILL-52
> 
> 
> Diffs
> -----
> 
>   twill-core/src/main/java/org/apache/twill/internal/AbstractZKServiceController.java a132128 
>   twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java 5d2f33c 
>   twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java 7592176 
>   twill-core/src/test/java/org/apache/twill/internal/ControllerTest.java 048e489 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java 8e0c6be 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java 9dff24d 
>   twill-yarn/src/test/java/org/apache/twill/yarn/SessionExpireTestRun.java PRE-CREATION 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestSuite.java bed613a 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestUtils.java 079db56 
> 
> Diff: https://reviews.apache.org/r/18864/diff/
> 
> 
> Testing
> -------
> 
> Added new test case SessionExpireTestRun with all existing test cases passed on all profiles.
> 
> 
> Thanks,
> 
> Terence Yim
> 
>


Re: Review Request 18864: Review for TWILL-52

Posted by Andreas Neumann <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18864/#review36572
-----------------------------------------------------------



twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java
<https://reviews.apache.org/r/18864/#comment67575>

    Does this happen often? Perhaps info will create very verbose logging here?



twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java
<https://reviews.apache.org/r/18864/#comment67574>

    should this be "submitted"?



twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java
<https://reviews.apache.org/r/18864/#comment67576>

    Can you explain somewhere what it means when this happens? How can it get into a state where the zk node is gone? 


- Andreas Neumann


On March 6, 2014, 9:57 p.m., Terence Yim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18864/
> -----------------------------------------------------------
> 
> (Updated March 6, 2014, 9:57 p.m.)
> 
> 
> Review request for Twill.
> 
> 
> Repository: twill
> 
> 
> Description
> -------
> 
> Fix for issue TWILL-52
> 
> 
> Diffs
> -----
> 
>   twill-core/src/main/java/org/apache/twill/internal/AbstractZKServiceController.java a132128 
>   twill-core/src/main/java/org/apache/twill/internal/TwillContainerLauncher.java 5d2f33c 
>   twill-core/src/main/java/org/apache/twill/internal/ZKServiceDecorator.java 7592176 
>   twill-core/src/test/java/org/apache/twill/internal/ControllerTest.java 048e489 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java 8e0c6be 
>   twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java 9dff24d 
>   twill-yarn/src/test/java/org/apache/twill/yarn/SessionExpireTestRun.java PRE-CREATION 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestSuite.java bed613a 
>   twill-yarn/src/test/java/org/apache/twill/yarn/YarnTestUtils.java 079db56 
> 
> Diff: https://reviews.apache.org/r/18864/diff/
> 
> 
> Testing
> -------
> 
> Added new test case SessionExpireTestRun with all existing test cases passed on all profiles.
> 
> 
> Thanks,
> 
> Terence Yim
> 
>