mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Klaus Ma" <klaus1982...@gmail.com>
Subject Re: Review Request 39285: Added Quota Request Validation.
Date Mon, 09 Nov 2015 12:52:31 GMT


> On Nov. 7, 2015, 12:19 a.m., Klaus Ma wrote:
> > src/master/quota_handler.cpp, line 140
> > <https://reviews.apache.org/r/39285/diff/7/?file=1118136#file1118136line140>
> >
> >     Suggest to move it into the loop; if any role is not known by master, we did
not need to continue to check others.
> 
> Alexander Rukletsov wrote:
>     I think the flow is more readable how it's now: in the loop we reconstruct the "reference"
role, afterwards we check whether it is known to the master. Also, my gut feeling is that
typos in roles will not be that frequent, so checking it once instead of per resource makes
sense to me.
> 
> Joerg Schad wrote:
>     There should just be a single role per request, why should I check that in the loop?

`::protobuf::parse()` will be heavy, we can end loop early if the role is not known by master.
But as @AlexR said, typos in role is rare case, it also make sense to me to keep current behavior.


> On Nov. 7, 2015, 12:19 a.m., Klaus Ma wrote:
> > src/master/quota_handler.cpp, line 180
> > <https://reviews.apache.org/r/39285/diff/7/?file=1118136#file1118136line180>
> >
> >     Should we move it into `validateQuotaRequest`? If any role is exist in master,
we did not need to continue to check others. And in QuotaHandler, we had the pointer to master
`Master* master`.
> 
> Alexander Rukletsov wrote:
>     I'd keep it here, because it's related to how we currently process the request, rather
than whether the request is valid.
> 
> Joerg Schad wrote:
>     There was an earlier comment from Joris mentioning that this isn't really part of
validatating the request (as it also involves state of the master).

Agree :).


- Klaus


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


On Nov. 9, 2015, 8:51 p.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39285/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3199
>     https://issues.apache.org/jira/browse/MESOS-3199
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Quota Request Validation.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp b76d30197b3decda0a742e03ce01a17a64b633ac 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39285/diff/
> 
> 
> Testing
> -------
> 
> make test
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>


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