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 70716: Decoupled quota limits from quota guarantees in the allocator.
Date Tue, 28 May 2019 19:24:23 GMT

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


Ship it!





src/master/allocator/mesos/hierarchical.hpp
Lines 121-124 (original), 121-126 (patched)
<https://reviews.apache.org/r/70716/#comment302281>

    Could probably just declare these two together with one comment, that would be a bit more
succinct / readable IMO



src/master/allocator/mesos/hierarchical.cpp
Lines 1446-1450 (patched)
<https://reviews.apache.org/r/70716/#comment302282>

    We could contain this logic a bit more without having the 'limits' variable in the top
level scope?
    
    ```
    roles[role].quotaLimits = [&]() {
      protobuf::Map<string, Value::Scalar> limits;
      foreach (const Resource& r, quota.info.guarantee()) {
        limits[r.name()] = r.scalar();
      }
      return ResourceLimits(limits);
    }();
    ```



src/master/allocator/mesos/hierarchical.cpp
Line 2559 (original), 2570-2571 (patched)
<https://reviews.apache.org/r/70716/#comment302283>

    How about:
    
    ```
      return roles.contains(role) &&
        (!roles.at(role).quotaGuarantees.empty() ||
         !roles.at(role).quotaLimits.empty());
    ```


- Benjamin Mahler


On May 24, 2019, noon, Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70716/
> -----------------------------------------------------------
> 
> (Updated May 24, 2019, noon)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, resource quota acts as both guarantees and limits.
> This patch adds a field `quotaLimits` in the role struct.
> This paves the way for enforcing guarantees and limits
> separately in the allocator.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 71a9656fb934bf9ac58e3165254ea49cb09efa8b

>   src/master/allocator/mesos/hierarchical.cpp 40c8363afddccdd5275ca06318a8cc2cc6fa21af

> 
> 
> Diff: https://reviews.apache.org/r/70716/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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