Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On May 26, 2015, 8:43 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 26, 2015, 8:43 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Joe Smith]

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

Ship it!


Ship It!

- Joe Smith


On May 26, 2015, 1:43 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 26, 2015, 1:43 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Aurora ReviewBot]

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


Master (6db13ba) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On May 26, 2015, 8:43 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 26, 2015, 8:43 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Brian Wickman]

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

(Updated May 26, 2015, 8:43 p.m.)


Review request for Aurora, Joe Smith and Vinod Kone.


Changes
---

Propagate exceptions raised during stop and log them.


Bugs: AURORA-698
https://issues.apache.org/jira/browse/AURORA-698


Repository: aurora


Description
---

Daemonize all deadline calls in aurora executor.  If we do not daemonize, it's 
possible for the aurora executor to send TASK_KILLED and then block 
indefinitely on shutdown.  This way the aurora executor process will at least 
exit, allow the cgroup to tear down all active processes.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
df0df0cba3269a137a370e2b4096fd61fd3af258 

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


Testing
---

./pants test src/test/python/apache/aurora/executor::


Thanks,

Brian Wickman

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Brian Wickman]

> On May 21, 2015, 5:54 a.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/executor/aurora_executor.py, line 189
> > 
> >
> > The two `deadline` calls in shutdown are run without `propagate=True`. 
> > IIRC we will therefore silently swallow all raised exceptions. Shouldn't we 
> > at least log them?

Good call, added logging.


- Brian


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


On May 26, 2015, 8:43 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 26, 2015, 8:43 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Stephan Erb]

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



src/main/python/apache/aurora/executor/aurora_executor.py


The two `deadline` calls in shutdown are run without `propagate=True`. IIRC 
we will therefore silently swallow all raised exceptions. Shouldn't we at least 
log them?


- Stephan Erb


On May 20, 2015, 8:10 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 20, 2015, 8:10 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 34484: Daemonize all deadline calls in aurora executor.

[Aurora ReviewBot]

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


Master (998993d) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On May 20, 2015, 6:10 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34484/
> ---
> 
> (Updated May 20, 2015, 6:10 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Vinod Kone.
> 
> 
> Bugs: AURORA-698
> https://issues.apache.org/jira/browse/AURORA-698
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Daemonize all deadline calls in aurora executor.  If we do not daemonize, 
> it's possible for the aurora executor to send TASK_KILLED and then block 
> indefinitely on shutdown.  This way the aurora executor process will at least 
> exit, allow the cgroup to tear down all active processes.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> df0df0cba3269a137a370e2b4096fd61fd3af258 
> 
> Diff: https://reviews.apache.org/r/34484/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Review Request 34484: Daemonize all deadline calls in aurora executor.

[Brian Wickman]

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

Review request for Aurora, Joe Smith and Vinod Kone.


Bugs: AURORA-698
https://issues.apache.org/jira/browse/AURORA-698


Repository: aurora


Description
---

Daemonize all deadline calls in aurora executor.  If we do not daemonize, it's 
possible for the aurora executor to send TASK_KILLED and then block 
indefinitely on shutdown.  This way the aurora executor process will at least 
exit, allow the cgroup to tear down all active processes.


Diffs
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
df0df0cba3269a137a370e2b4096fd61fd3af258 

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


Testing
---

./pants test src/test/python/apache/aurora/executor::


Thanks,

Brian Wickman