Re: Review Request 35411: Send oversubscribable resources during (re-)registration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/#review87959 --- Ship it! src/slave/slave.cpp https://reviews.apache.org/r/35411/#comment140371 s/oversubscribable/oversubscribed/ src/slave/slave.cpp https://reviews.apache.org/r/35411/#comment140372 s/oversubscribable/oversubscribed/ - Vinod Kone On June 15, 2015, 5:59 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/ --- (Updated June 15, 2015, 5:59 p.m.) Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2866 https://issues.apache.org/jira/browse/MESOS-2866 Repository: mesos Description --- Send oversubscribable resources during (re-)registration. Diffs - src/slave/slave.hpp 0df1b55791963fb4159b7ea5318d09dde4f7d8c7 src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4 src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/35411/diff/ Testing --- Thanks, Ben Mahler
Re: Review Request 35411: Send oversubscribable resources during (re-)registration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/#review87944 --- Ship it! Ship It! - Jie Yu On June 13, 2015, 2:04 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/ --- (Updated June 13, 2015, 2:04 a.m.) Review request for mesos, Jie Yu and Vinod Kone. Bugs: MESOS-2866 https://issues.apache.org/jira/browse/MESOS-2866 Repository: mesos Description --- Send oversubscribable resources during (re-)registration. Diffs - src/slave/slave.hpp 0df1b55791963fb4159b7ea5318d09dde4f7d8c7 src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4 src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/35411/diff/ Testing --- Thanks, Ben Mahler
Re: Review Request 35411: Send oversubscribable resources during (re-)registration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/#review87968 --- Just wanted to tag on this review and follow the update changes - LGTM :) src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35411/#comment140373 Which kind of update message? :) s/update/'SlaveUpdate'/ ? src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35411/#comment140375 You have the PIDs of the master and slave - would it make sense to be explicit in the pattern matching? src/tests/oversubscription_tests.cpp https://reviews.apache.org/r/35411/#comment140374 Should we const 'resources'? - Niklas Nielsen On June 15, 2015, 10:59 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/ --- (Updated June 15, 2015, 10:59 a.m.) Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2866 https://issues.apache.org/jira/browse/MESOS-2866 Repository: mesos Description --- Send oversubscribable resources during (re-)registration. Diffs - src/slave/slave.hpp 0df1b55791963fb4159b7ea5318d09dde4f7d8c7 src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4 src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/35411/diff/ Testing --- Thanks, Ben Mahler
Re: Review Request 35411: Send oversubscribable resources during (re-)registration.
On June 15, 2015, 8:21 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 498 https://reviews.apache.org/r/35411/diff/1/?file=983985#file983985line498 Which kind of update message? :) s/update/'SlaveUpdate'/ ? Changed it to forwards the estimation to match the other test comments, thanks! On June 15, 2015, 8:21 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 528 https://reviews.apache.org/r/35411/diff/1/?file=983985#file983985line528 You have the PIDs of the master and slave - would it make sense to be explicit in the pattern matching? Since none of the tests in this file use the PID matching, I followed suit for consistency. It would be nice to use them in general, but there are cases where we don't have both PIDs yet (e.g. need to set up the expectation before starting the slave). On June 15, 2015, 8:21 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, line 538 https://reviews.apache.org/r/35411/diff/1/?file=983985#file983985line538 Should we const 'resources'? Hm.. seems inconsistent with the rest of this file, but also, do we want to proliferate 'const' everywhere? There are a ton of scope variables in this file that can be const but are not, so I'm not concerned about it. I haven't really noticed a lot of benefit from having scope variables as 'const' where not necessary, have you? Would love to see some examples as the code is getting a bit inconsistent with 'const' usage. We have a ton of variables that can be const, but adding const to all of these might be too verbose IMO :( - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/#review87968 --- On June 15, 2015, 5:59 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35411/ --- (Updated June 15, 2015, 5:59 p.m.) Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone. Bugs: MESOS-2866 https://issues.apache.org/jira/browse/MESOS-2866 Repository: mesos Description --- Send oversubscribable resources during (re-)registration. Diffs - src/slave/slave.hpp 0df1b55791963fb4159b7ea5318d09dde4f7d8c7 src/slave/slave.cpp b523c2fce50e56f4f94d55a9488f49c53452e4d4 src/tests/oversubscription_tests.cpp e8ae053dd9cd712e49bd2830e414b7a3d99c20ca Diff: https://reviews.apache.org/r/35411/diff/ Testing --- Thanks, Ben Mahler
