> 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 > >
