You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Adam B <ad...@mesosphere.io> on 2016/01/04 12:15:59 UTC
Re: Review Request 40429: Report executor exit to framework
schedulers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40429/#review112541
-----------------------------------------------------------
Ship it!
Looks good, but you really need to set up your EXPECTations before you do the thing (e.g. `containerizer.destroy()`) that would trigger the EXPECTation. Fix these, and I'd feel comfortable committing it.
Any further thoughts from Vinod?
docs/upgrades.md (line 15)
<https://reviews.apache.org/r/40429/#comment173030>
"The `executorLost` callback in the Scheduler interface will now be called whenever the slave detects termination of a custom executor. This callback was never called in previous versions, so please make sure any framework schedulers can now safely handle this callback. Note that this callback may not be reliably delivered."
src/tests/fault_tolerance_tests.cpp (lines 1720 - 1722)
<https://reviews.apache.org/r/40429/#comment173035>
Can avoid these clock adjust if you do another `AWAIT_READY(executorLost);`
src/tests/gc_tests.cpp (lines 551 - 553)
<https://reviews.apache.org/r/40429/#comment173036>
EXPECT(executorLost) should go before the containerizer.destroy() call, to prevent a race. I see this in several places. Fix it everywhere.
src/tests/master_slave_reconciliation_tests.cpp (lines 137 - 143)
<https://reviews.apache.org/r/40429/#comment173037>
EXPECT(executorLost) goes before containerizer.destroy()
- Adam B
On Dec. 24, 2015, 12:29 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2015, 12:29 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Report executor exit to framework schedulers. This is a MVP to start the work of notifying scheduler on scheduler refresh.
>
> Next step would be sending this message reliabily, and/or splitting Event::FAILURE for slave failure and executor termination.
>
>
> Diffs
> -----
>
> CHANGELOG ea8f629f5543a8be61d162779c2b5b12f66cea79
> docs/app-framework-development-guide.md 4a43a93d080bdac37b8aee91748fea7552a1cc67
> docs/upgrades.md aebdc1e1b7a73c93b5a14867214eb852de89ad24
> include/mesos/scheduler.hpp 049c041286f3167e79cc5ea8a9e0bf8d42569832
> src/java/src/org/apache/mesos/Scheduler.java 4f048830a2c47f747033c60730cc770cb2578815
> src/python/interface/src/mesos/interface/__init__.py 4be502fd83029ad5fc798696caf9e27fd95f7482
> src/sched/sched.cpp 44eb4f50e8ed84297268d94a3a0320c843ff6d8c
> src/tests/fault_tolerance_tests.cpp ba657d0e1d8515cffd1b37925bf91a84b2feaef1
> src/tests/gc_tests.cpp f939d27c58177fba052fbcd9d6c9a572d052df52
> src/tests/master_slave_reconciliation_tests.cpp 9afa826006fa7129da1a9c1ac8c389c0e051f717
> src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4
> src/tests/scheduler_event_call_tests.cpp 03f0332ef75bbe7c4947bd6daf55d40384570f18
> src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
>
> Diff: https://reviews.apache.org/r/40429/diff/
>
>
> Testing
> -------
>
> Modified test for SchedulerDriverEventTest.Failure, which verifies that MockScheduler::executorLost is invoked.
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 40429: Report executor exit to framework
schedulers.
Posted by Vinod Kone <vi...@gmail.com>.
> On Jan. 4, 2016, 11:15 a.m., Adam B wrote:
> > Looks good, but you really need to set up your EXPECTations before you do the thing (e.g. `containerizer.destroy()`) that would trigger the EXPECTation. Fix these, and I'd feel comfortable committing it.
> > Any further thoughts from Vinod?
Took a quick glance. Looks ok. I'll let Adam shepherd this to completion.
Was there an email sent to the dev list regarding this upcoming change?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40429/#review112541
-----------------------------------------------------------
On Dec. 24, 2015, 8:29 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2015, 8:29 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Report executor exit to framework schedulers. This is a MVP to start the work of notifying scheduler on scheduler refresh.
>
> Next step would be sending this message reliabily, and/or splitting Event::FAILURE for slave failure and executor termination.
>
>
> Diffs
> -----
>
> CHANGELOG ea8f629f5543a8be61d162779c2b5b12f66cea79
> docs/app-framework-development-guide.md 4a43a93d080bdac37b8aee91748fea7552a1cc67
> docs/upgrades.md aebdc1e1b7a73c93b5a14867214eb852de89ad24
> include/mesos/scheduler.hpp 049c041286f3167e79cc5ea8a9e0bf8d42569832
> src/java/src/org/apache/mesos/Scheduler.java 4f048830a2c47f747033c60730cc770cb2578815
> src/python/interface/src/mesos/interface/__init__.py 4be502fd83029ad5fc798696caf9e27fd95f7482
> src/sched/sched.cpp 44eb4f50e8ed84297268d94a3a0320c843ff6d8c
> src/tests/fault_tolerance_tests.cpp ba657d0e1d8515cffd1b37925bf91a84b2feaef1
> src/tests/gc_tests.cpp f939d27c58177fba052fbcd9d6c9a572d052df52
> src/tests/master_slave_reconciliation_tests.cpp 9afa826006fa7129da1a9c1ac8c389c0e051f717
> src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4
> src/tests/scheduler_event_call_tests.cpp 03f0332ef75bbe7c4947bd6daf55d40384570f18
> src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
>
> Diff: https://reviews.apache.org/r/40429/diff/
>
>
> Testing
> -------
>
> Modified test for SchedulerDriverEventTest.Failure, which verifies that MockScheduler::executorLost is invoked.
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 40429: Report executor exit to framework
schedulers.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 4, 2016, 11:15 a.m., Adam B wrote:
> > Looks good, but you really need to set up your EXPECTations before you do the thing (e.g. `containerizer.destroy()`) that would trigger the EXPECTation. Fix these, and I'd feel comfortable committing it.
> > Any further thoughts from Vinod?
>
> Vinod Kone wrote:
> Took a quick glance. Looks ok. I'll let Adam shepherd this to completion.
>
> Was there an email sent to the dev list regarding this upcoming change?
Hi Vinod, I just sent out the email.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40429/#review112541
-----------------------------------------------------------
On Jan. 5, 2016, 1:14 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 1:14 a.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Report executor exit to framework schedulers. This is a MVP to start the work of notifying scheduler on scheduler refresh.
>
> Next step would be sending this message reliabily, and/or splitting Event::FAILURE for slave failure and executor termination.
>
>
> Diffs
> -----
>
> CHANGELOG 2aa083c85ae2a2f1392121b1d30d71376c01ffd7
> docs/app-framework-development-guide.md 4a43a93d080bdac37b8aee91748fea7552a1cc67
> docs/upgrades.md aebdc1e1b7a73c93b5a14867214eb852de89ad24
> include/mesos/scheduler.hpp 049c041286f3167e79cc5ea8a9e0bf8d42569832
> src/java/src/org/apache/mesos/Scheduler.java 4f048830a2c47f747033c60730cc770cb2578815
> src/python/interface/src/mesos/interface/__init__.py 4be502fd83029ad5fc798696caf9e27fd95f7482
> src/sched/sched.cpp 44eb4f50e8ed84297268d94a3a0320c843ff6d8c
> src/tests/fault_tolerance_tests.cpp ba657d0e1d8515cffd1b37925bf91a84b2feaef1
> src/tests/gc_tests.cpp f939d27c58177fba052fbcd9d6c9a572d052df52
> src/tests/master_slave_reconciliation_tests.cpp 9afa826006fa7129da1a9c1ac8c389c0e051f717
> src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4
> src/tests/scheduler_event_call_tests.cpp 03f0332ef75bbe7c4947bd6daf55d40384570f18
> src/tests/slave_tests.cpp 77750eda99184c0bf4404df8468461e3c7f8cde0
>
> Diff: https://reviews.apache.org/r/40429/diff/
>
>
> Testing
> -------
>
> Modified test for SchedulerDriverEventTest.Failure, which verifies that MockScheduler::executorLost is invoked.
>
>
> Thanks,
>
> Zhitao Li
>
>