You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2015/04/22 23:40:20 UTC
Re: Review Request 32843: Updated KILL to include SlaveID.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32843/
-----------------------------------------------------------
(Updated April 22, 2015, 9:40 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
rebased. NNFR.
Bugs: MESOS-1127
https://issues.apache.org/jira/browse/MESOS-1127
Repository: mesos
Description
-------
Having SlaveID will help us do better reconciliation when the task is not found. Also, updated master to handle Call::Kill.
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15
src/master/master.cpp f3462d15005e24ab28e8265484b0d3810f21bd47
src/scheduler/scheduler.cpp 584b042e32865fdf875bf41ebcfb7f9c327d882a
src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657
Diff: https://reviews.apache.org/r/32843/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 32843: Updated KILL to include SlaveID.
Posted by Vinod Kone <vi...@gmail.com>.
> On April 22, 2015, 10:20 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 2731-2732
> > <https://reviews.apache.org/r/32843/diff/4/?file=939952#file939952line2731>
> >
> > You may want to avoid taking a const reference to the temporary here?
fixed.
> On April 22, 2015, 10:20 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2772
> > <https://reviews.apache.org/r/32843/diff/4/?file=939952#file939952line2772>
> >
> > Will this be a BadRequest so schedulers can detect the issue? Maybe a TODO?
done.
> On April 22, 2015, 10:20 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 2773
> > <https://reviews.apache.org/r/32843/diff/4/?file=939952#file939952line2773>
> >
> > Whoops, remove the semi-colon?
done.
> On April 22, 2015, 10:20 p.m., Ben Mahler wrote:
> > src/tests/scheduler_tests.cpp, line 417
> > <https://reviews.apache.org/r/32843/diff/4/?file=939954#file939954line417>
> >
> > Did you want this extra newline?
nope. fixed.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32843/#review81222
-----------------------------------------------------------
On April 22, 2015, 9:40 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32843/
> -----------------------------------------------------------
>
> (Updated April 22, 2015, 9:40 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-1127
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Having SlaveID will help us do better reconciliation when the task is not found. Also, updated master to handle Call::Kill.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
> src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15
> src/master/master.cpp f3462d15005e24ab28e8265484b0d3810f21bd47
> src/scheduler/scheduler.cpp 584b042e32865fdf875bf41ebcfb7f9c327d882a
> src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657
>
> Diff: https://reviews.apache.org/r/32843/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 32843: Updated KILL to include SlaveID.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32843/#review81222
-----------------------------------------------------------
Ship it!
src/master/master.cpp
<https://reviews.apache.org/r/32843/#comment131473>
Does this variable buy us much vs `kill.task_id()`? For example, we don't do this for framework->id(), I'm guessing you did it to be similar to slaveId?
src/master/master.cpp
<https://reviews.apache.org/r/32843/#comment131471>
You may want to avoid taking a const reference to the temporary here?
src/master/master.cpp
<https://reviews.apache.org/r/32843/#comment131475>
Will this be a BadRequest so schedulers can detect the issue? Maybe a TODO?
src/master/master.cpp
<https://reviews.apache.org/r/32843/#comment131474>
Whoops, remove the semi-colon?
src/tests/scheduler_tests.cpp
<https://reviews.apache.org/r/32843/#comment131476>
Did you want this extra newline?
- Ben Mahler
On April 22, 2015, 9:40 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32843/
> -----------------------------------------------------------
>
> (Updated April 22, 2015, 9:40 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-1127
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Having SlaveID will help us do better reconciliation when the task is not found. Also, updated master to handle Call::Kill.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
> src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15
> src/master/master.cpp f3462d15005e24ab28e8265484b0d3810f21bd47
> src/scheduler/scheduler.cpp 584b042e32865fdf875bf41ebcfb7f9c327d882a
> src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657
>
> Diff: https://reviews.apache.org/r/32843/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 32843: Updated KILL to include SlaveID.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32843/
-----------------------------------------------------------
(Updated April 23, 2015, 2:51 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Benm's. NNFR.
Bugs: MESOS-1127
https://issues.apache.org/jira/browse/MESOS-1127
Repository: mesos
Description
-------
Having SlaveID will help us do better reconciliation when the task is not found. Also, updated master to handle Call::Kill.
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15
src/master/master.cpp f3462d15005e24ab28e8265484b0d3810f21bd47
src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657
Diff: https://reviews.apache.org/r/32843/diff/
Testing
-------
make check
Thanks,
Vinod Kone