Re: Review Request 34632: Added QoS Controller in slave
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
--- 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
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/#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
--- 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
--- 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
Review Request 34632: Added QoS Controller in slave
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34632/ --- Review request for mesos. 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 5c57478fcfdbcbd8ac0e5c3c79809403054e96e6 src/slave/flags.cpp b5e25186dad36bc1306cc6ecb268aba951a18f7e src/slave/main.cpp f762f5b06be74c391cbc336b2da28f8358952ba4 src/slave/slave.hpp 0207eafa914e23e4c72b1e5c4fb43aae6c97049c src/slave/slave.cpp b4d20294330f791e64a597c67b686aed9de84837 src/tests/cluster.hpp 7370c77de68d3a13340a69f6e794d97306d4bbb6 src/tests/mesos.hpp b8f7a2f9236166e42421d926718af8d45e857eba src/tests/mesos.cpp 1d5639c85517229f3396b40f2d8bd421b2ed7325 Diff: https://reviews.apache.org/r/34632/diff/ Testing --- make check Thanks, Niklas Nielsen
