mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 71020: Added overcommit and hierarchical inclusion check for `UPDATE_QUOTA`.
Date Mon, 08 Jul 2019 21:55:56 GMT


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > I assume we'll track the missing pieces with additional tickets or leave the existing
ticket open until we have them?

Yes, let's keep the ticket open.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/71020/diff/1/?file=2153866#file2153866line94>
> >
> >     This seems like a transitive property that doesn't need to be spelled out?
> >     
> >     sum of children's guarantees <=
> >     parent's guarantee <=
> >     parent's limit
> >     
> >     It seems somewhat unnatural to directly check:
> >     
> >     sum of children's guarantees <=
> >     parent's limit
> >     
> >     rather than check:
> >     
> >     sum of children's guarantees <=
> >     parent's limit
> >     
> >     &&
> >     
> >     guarantee <= limit

Good point! Updated the comment, commit description and related tickets.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 467-515 (patched)
> > <https://reviews.apache.org/r/71020/diff/1/?file=2153866#file2153866line467>
> >
> >     Hm.. is this code copied from the original quota handler? Is your plan to have
this be the single path and the old api goes through this?
> >     
> >     if so, can you clarify with a TODO that this is duplicated from the original
quota handler and the plan is to have that go through this path?

The logic here is a bit different (supposedly more performant). Added a TODO to pull this
out as a standalone function to be shared by the old and the new.


> On July 7, 2019, 3:50 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 485 (patched)
> > <https://reviews.apache.org/r/71020/diff/1/?file=2153866#file2153866line485>
> >
> >     BadRequest vs Conflict below seems a bit inconsitent? They're both violations
of the constraints?

Changed to return `BadRequest` in both cases.


- Meng


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


On July 7, 2019, 3:12 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71020/
> -----------------------------------------------------------
> 
> (Updated July 7, 2019, 3:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9812
>     https://issues.apache.org/jira/browse/MESOS-9812
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The overcommit check validates that the total quota guarantees in
> the cluster is contained by the cluster capacity.
> 
> The hierarchical inclusion check validates that the sum of
> children's  guarantees is contained by the parent guarantee.
> 
> Further validation is needed for:
> 
> - Check a role's limit is less than its current consumption.
> - Check a role's limit is less than its parent's limit.
> - Check the sum of children's guarantees are less than its
> parent's limit.
> 
> 
> Diffs
> -----
> 
>   src/master/quota_handler.cpp f9c743170461d8d83180db20c917d3842241d4df 
> 
> 
> Diff: https://reviews.apache.org/r/71020/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> tests in r/71022/
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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