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 70276: Included RP resources / static reservations in quota capacity check.
Date Mon, 25 Mar 2019 00:06:47 GMT

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


Fix it, then Ship it!





src/master/master.hpp
Lines 1195-1199 (original), 1195-1199 (patched)
<https://reviews.apache.org/r/70276/#comment300074>

    Why cut the comments here? I think we should at least mention what we are checking i.e.
include a formula like that in the old code.
    
    In addition, since we remove the static reservation check, it would be nice if we can
include a comment here regarding reservations (both static and dynamic) for people who wonders
about the old behavior.



src/master/quota_handler.cpp
Lines 222-224 (original), 217-219 (patched)
<https://reviews.apache.org/r/70276/#comment300076>

    Early termination is still beneficial. We could also do the subtraction optimization I
mentioned in the previous review.
    
    Also, we are mixing the behavior change and some refactoring here. For clarity, it will
be great if we can separate them into different patches.



src/master/quota_handler.cpp
Line 224 (original), 219 (patched)
<https://reviews.apache.org/r/70276/#comment300075>

    Excluding revocable resources is another change of behavior here. Let's mention that in
the description and comment.



src/master/quota_handler.cpp
Line 617 (original), 602 (patched)
<https://reviews.apache.org/r/70276/#comment300077>

    We could just pass the quantities here?


- Meng Zhu


On March 22, 2019, 8:54 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70276/
> -----------------------------------------------------------
> 
> (Updated March 22, 2019, 8:54 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Bugs: MESOS-7883
>     https://issues.apache.org/jira/browse/MESOS-7883
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Static reservations and resource provider resources both count towards
> quota consumption and should therefore be included in the quota
> capacity check.
> 
> Note also that this renames the function to be an "overcommit" check
> rather than making any suggestions of being a "satisfiability" check.
> The latter is far more complex to implement properly, and the current
> quota API design includes a simple overcommit check rather than any
> attempt as satisfiability checking.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 77be494d1ca2d3cd79ab033e41a5ff66c0225f79 
>   src/master/quota_handler.cpp 2dcfdcaf836f21d7d39b5ef5c36de0db25ca7517 
> 
> 
> Diff: https://reviews.apache.org/r/70276/diff/1/
> 
> 
> Testing
> -------
> 
> Added a test in the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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