You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Charles Reiss <wo...@gmail.com> on 2012/11/19 22:14:33 UTC
Review Request: Call Scheduler::executorLost on executor exit.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Description
-------
Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
This addresses bug MESOS-313.
https://issues.apache.org/jira/browse/MESOS-313
Diffs
-----
src/master/master.cpp 87fad0e
src/sched/sched.cpp a1247f9
src/tests/gc_tests.cpp 6b3ba02
src/tests/master_tests.cpp 948ab5d
Diff: https://reviews.apache.org/r/8128/diff/
Testing
-------
make check
Thanks,
Charles Reiss
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review14318
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Nov. 19, 2012, 11:05 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 11:05 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review13595
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Nov. 19, 2012, 11:05 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 11:05 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Charles Reiss <wo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/
-----------------------------------------------------------
(Updated Nov. 19, 2012, 11:05 p.m.)
Review request for mesos and Benjamin Hindman.
Description
-------
Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
This addresses bug MESOS-313.
https://issues.apache.org/jira/browse/MESOS-313
Diffs (updated)
-----
src/master/master.cpp 87fad0e
src/sched/sched.cpp a1247f9
src/tests/gc_tests.cpp 6b3ba02
src/tests/master_tests.cpp 948ab5d
Diff: https://reviews.apache.org/r/8128/diff/
Testing
-------
make check
Thanks,
Charles Reiss
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Charles Reiss <wo...@gmail.com>.
> On Nov. 19, 2012, 10:31 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 1148
> > <https://reviews.apache.org/r/8128/diff/1-2/?file=221867#file221867line1148>
> >
> > please include the hostname
> > << " (" << slave->info.hostname() << ")"
Can't; the master doesn't know it in this case.
> On Nov. 19, 2012, 10:31 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 1151
> > <https://reviews.apache.org/r/8128/diff/1-2/?file=221867#file221867line1151>
> >
> > Not entirely yours, but would you mind re-organizing the code inside this function?
> > As is, its hard to reason about the flow.
> >
> > How about..
> >
> > Framework* framework = getFramework(frameworkId);
> > if (framework != NULL) {
> > // Send ExitedExecutorMessage to framework
> >
> > // framework->removeExecutor(slaveId, executorId);
> > } else {
> > LOG(WARNING) <<
> > }
> >
> > Slave* slave = getSlave(slaveId);
> >
> > if (slave == NULL) {
> > LOG(WARNING) <<
> > return;
> > }
> >
> > if (slave->hasExecutor()) {
> > // Blah
> > } else {
> > LOG(WARNING) <<
> > }
> >
> >
> >
Done.
- Charles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review13591
-----------------------------------------------------------
On Nov. 19, 2012, 11:05 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 11:05 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review13591
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/8128/#comment29164>
please include the hostname
<< " (" << slave->info.hostname() << ")"
src/master/master.cpp
<https://reviews.apache.org/r/8128/#comment29165>
Not entirely yours, but would you mind re-organizing the code inside this function?
As is, its hard to reason about the flow.
How about..
Framework* framework = getFramework(frameworkId);
if (framework != NULL) {
// Send ExitedExecutorMessage to framework
// framework->removeExecutor(slaveId, executorId);
} else {
LOG(WARNING) <<
}
Slave* slave = getSlave(slaveId);
if (slave == NULL) {
LOG(WARNING) <<
return;
}
if (slave->hasExecutor()) {
// Blah
} else {
LOG(WARNING) <<
}
- Vinod Kone
On Nov. 19, 2012, 10:09 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 10:09 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Charles Reiss <wo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/
-----------------------------------------------------------
(Updated Nov. 19, 2012, 10:09 p.m.)
Review request for mesos and Benjamin Hindman.
Description
-------
Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
This addresses bug MESOS-313.
https://issues.apache.org/jira/browse/MESOS-313
Diffs (updated)
-----
src/master/master.cpp 87fad0e
src/sched/sched.cpp a1247f9
src/tests/gc_tests.cpp 6b3ba02
src/tests/master_tests.cpp 948ab5d
Diff: https://reviews.apache.org/r/8128/diff/
Testing
-------
make check
Thanks,
Charles Reiss
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Charles Reiss <wo...@gmail.com>.
> On Nov. 19, 2012, 9:53 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 1145-1150
> > <https://reviews.apache.org/r/8128/diff/1/?file=221867#file221867line1145>
> >
> > Why not always forward this to the scheduler, irrespective of whether the master knows about slaveId/ executorId/frameworkId?
Master needs to know about the frameworkId to know the scheduler PID to forward to, but, removed requirement to know slave.
> On Nov. 19, 2012, 9:53 p.m., Vinod Kone wrote:
> > src/sched/sched.cpp, line 406
> > <https://reviews.apache.org/r/8128/diff/1/?file=221868#file221868line406>
> >
> > s/on/on slave/
Done.
- Charles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review13588
-----------------------------------------------------------
On Nov. 19, 2012, 10:09 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 10:09 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>
Re: Review Request: Call Scheduler::executorLost on executor exit.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8128/#review13588
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/8128/#comment29161>
Why not always forward this to the scheduler, irrespective of whether the master knows about slaveId/ executorId/frameworkId?
src/sched/sched.cpp
<https://reviews.apache.org/r/8128/#comment29159>
s/on/on slave/
src/sched/sched.cpp
<https://reviews.apache.org/r/8128/#comment29160>
we are trying to use a new formatting for function calls.
if it fits on the 2nd line.
scheduler->executorLost(
driver, executorId, slaveId, static_cast<int>(status);
if it doesn't
scheduler->executorLost(
driver,
executorId,
slaveId,
static_cast<int>(status))
- Vinod Kone
On Nov. 19, 2012, 9:14 p.m., Charles Reiss wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8128/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2012, 9:14 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> Forward ExitedExecutorMessages from the master to framework schedulers, and call Scheduler::executorLost when they are received.
>
>
> This addresses bug MESOS-313.
> https://issues.apache.org/jira/browse/MESOS-313
>
>
> Diffs
> -----
>
> src/master/master.cpp 87fad0e
> src/sched/sched.cpp a1247f9
> src/tests/gc_tests.cpp 6b3ba02
> src/tests/master_tests.cpp 948ab5d
>
> Diff: https://reviews.apache.org/r/8128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Charles Reiss
>
>