You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2015/06/03 06:28:07 UTC
Re: Review Request 34632: Added QoS Controller in slave
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34632/
-----------------------------------------------------------
(Updated June 2, 2015, 9:28 p.m.)
Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
Repository: mesos
Description
-------
Wired up QoS Controller in slave and necessary test code for the new slave constructor argument.
Diffs (updated)
-----
src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40
src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd
src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d
src/slave/slave.hpp 245ea062a56461d96ee3055be1c93ec508d1bec7
src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05
src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6
src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e
src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538
Diff: https://reviews.apache.org/r/34632/diff/
Testing
-------
make check
Thanks,
Niklas Nielsen
Re: Review Request 34632: Added QoS Controller in slave
Posted by Niklas Nielsen <ni...@qni.dk>.
> On June 3, 2015, 4:26 p.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 344-345
> > <https://reviews.apache.org/r/34632/diff/2/?file=977215#file977215line344>
> >
> > Can you explain why you want to start receiving/acting on corrections even before the slave is recovered? How do you kill a container if the slave is not recovered?
Moved :) Thanks
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34632/#review86501
-----------------------------------------------------------
On June 4, 2015, 10:42 a.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34632/
> -----------------------------------------------------------
>
> (Updated June 4, 2015, 10:42 a.m.)
>
>
> Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Wired up QoS Controller in slave and necessary test code for the new slave constructor argument.
>
>
> Diffs
> -----
>
> src/local/local.cpp 6bf4b77589ec728f479503a5ae49cb2b49fc0f96
> src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd
> src/slave/main.cpp c4d8940d504b819dfcc8a715e08591e8e792c0b6
> src/slave/slave.hpp 37e85af90275dc0c88c12b39a5813e7d71abb1f3
> src/slave/slave.cpp 30e0d8ba2a2b12a0a7f1f7246b65360de30ef4ea
> src/tests/cluster.hpp f5a7c540f0b8c37369c1af85403700a89899592e
> src/tests/mesos.hpp 7e134b7d938e149504a70b77fe617ddc5a192f66
> src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538
>
> Diff: https://reviews.apache.org/r/34632/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 34632: Added QoS Controller in slave
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34632/#review86501
-----------------------------------------------------------
LGTM overall. Just one place (regarding the time when we start to receive/act on corrections) needs more thoughts.
src/slave/main.cpp
<https://reviews.apache.org/r/34632/#comment138545>
Kill this extra line.
src/slave/main.cpp
<https://reviews.apache.org/r/34632/#comment138546>
Kill this extra line.
src/slave/slave.hpp
<https://reviews.apache.org/r/34632/#comment138547>
include order
src/slave/slave.cpp
<https://reviews.apache.org/r/34632/#comment138548>
Reorder
src/slave/slave.cpp
<https://reviews.apache.org/r/34632/#comment138555>
I would kill this line to be consistent with resource estimator above. (or add one line for resource estimator).
src/slave/slave.cpp
<https://reviews.apache.org/r/34632/#comment138560>
Can you explain why you want to start receiving/acting on corrections even before the slave is recovered? How do you kill a container if the slave is not recovered?
src/slave/slave.cpp
<https://reviews.apache.org/r/34632/#comment138554>
Add one more line here.
src/tests/mesos.cpp
<https://reviews.apache.org/r/34632/#comment138561>
const Option<...>...
- Jie Yu
On June 3, 2015, 4:28 a.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34632/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 4:28 a.m.)
>
>
> Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Wired up QoS Controller in slave and necessary test code for the new slave constructor argument.
>
>
> Diffs
> -----
>
> src/local/local.cpp 84f73e21e66e7a654b6b09f6a57a70a1ccc34b40
> src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd
> src/slave/main.cpp e3a45f4dfa0c8cc5d28d795f42bdc8cd14b7b10d
> src/slave/slave.hpp 245ea062a56461d96ee3055be1c93ec508d1bec7
> src/slave/slave.cpp 271cb03770cd08406054dfce35d0821475e49b05
> src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6
> src/tests/mesos.hpp aeeed61ea987963345a8b5c6e09d14e51098bb2e
> src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538
>
> Diff: https://reviews.apache.org/r/34632/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 34632: Added QoS Controller in slave
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34632/#review86677
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On June 4, 2015, 5:42 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34632/
> -----------------------------------------------------------
>
> (Updated June 4, 2015, 5:42 p.m.)
>
>
> Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Wired up QoS Controller in slave and necessary test code for the new slave constructor argument.
>
>
> Diffs
> -----
>
> src/local/local.cpp 6bf4b77589ec728f479503a5ae49cb2b49fc0f96
> src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd
> src/slave/main.cpp c4d8940d504b819dfcc8a715e08591e8e792c0b6
> src/slave/slave.hpp 37e85af90275dc0c88c12b39a5813e7d71abb1f3
> src/slave/slave.cpp 30e0d8ba2a2b12a0a7f1f7246b65360de30ef4ea
> src/tests/cluster.hpp f5a7c540f0b8c37369c1af85403700a89899592e
> src/tests/mesos.hpp 7e134b7d938e149504a70b77fe617ddc5a192f66
> src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538
>
> Diff: https://reviews.apache.org/r/34632/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 34632: Added QoS Controller in slave
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34632/
-----------------------------------------------------------
(Updated June 4, 2015, 10:42 a.m.)
Review request for mesos, Bartek Plotka, Jie Yu, and Vinod Kone.
Repository: mesos
Description
-------
Wired up QoS Controller in slave and necessary test code for the new slave constructor argument.
Diffs (updated)
-----
src/local/local.cpp 6bf4b77589ec728f479503a5ae49cb2b49fc0f96
src/slave/flags.hpp 15dd8387df1344807b54e911a8094a89f4629cbd
src/slave/main.cpp c4d8940d504b819dfcc8a715e08591e8e792c0b6
src/slave/slave.hpp 37e85af90275dc0c88c12b39a5813e7d71abb1f3
src/slave/slave.cpp 30e0d8ba2a2b12a0a7f1f7246b65360de30ef4ea
src/tests/cluster.hpp f5a7c540f0b8c37369c1af85403700a89899592e
src/tests/mesos.hpp 7e134b7d938e149504a70b77fe617ddc5a192f66
src/tests/mesos.cpp d3a8bb70478993883f34804c8e2d7559d16c3538
Diff: https://reviews.apache.org/r/34632/diff/
Testing
-------
make check
Thanks,
Niklas Nielsen