mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 55462: Validate resource reservation against allocated role.
Date Mon, 13 Feb 2017 23:34:35 GMT


> On Feb. 11, 2017, 12:01 a.m., Benjamin Mahler wrote:
> > src/master/validation.cpp, lines 1612-1629
> > <https://reviews.apache.org/r/55462/diff/4/?file=1629533#file1629533line1612>
> >
> >     Looks like this needs a rebase against master? This block was removed and the
roles are no longer an option.
> 
> Benjamin Bannier wrote:
>     No, this patch was created against a recent `HEAD`. The modified patch you committed
as part of r/55461 introduced a loop-local `set<string> frameworkRoles` inside the loop
over resources. That is nice as it avoids keeping two optional, related values in scope (`frameworkInfo`
and `frameworkRoles`). This comes at the cost of recalculating this framework invariant for
each loop iteration; I here pulled it out of the loop to not recalculate it over and over
again. Do you believe that's a bad idea?

Ok that makes sense. And what about the validation that is in this block? Based on the discussion
from the last review these were removed, are you suggesting to add them back in?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55462/#review165204
-----------------------------------------------------------


On Feb. 13, 2017, 3:38 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55462/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2017, 3:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Bugs: MESOS-6730
>     https://issues.apache.org/jira/browse/MESOS-6730
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change introduces validation of the 'AllocationInfo' of resources
> used in reservations.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 2bfca95713319a195e43e990cbc2dc5570b89c4e 
>   src/tests/master_validation_tests.cpp fd1f4a6cf0661351e265e50da1bd6ea04ed13d26 
> 
> Diff: https://reviews.apache.org/r/55462/diff/
> 
> 
> Testing
> -------
> 
> N/A yet.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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