mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 35411: Send oversubscribable resources during (re-)registration.
Date Mon, 15 Jun 2015 21:30:32 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message