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 71029: Added Role::quotaConsumption to the master.
Date Tue, 09 Jul 2019 00:10:55 GMT

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


Fix it, then Ship it!





src/master/master.hpp
Lines 2730 (patched)
<https://reviews.apache.org/r/71029/#comment303668>

    `totalUsedResources` here also includes the executor resources which are not managed by
the allocator. Do we want to include that in the consumption?
    
    If no, then the consumption is not going to be consistent with framework stats. But if
yes, since the consumption will be compared with quota (both from the user perspective as
well as internally in the future), that means when operator configures the quota, they need
to take executor resources into account. I am not sure if that's good user experience.
    
    For simplicity, I am OK with leaving it as is now. But let's document the rationale and/or
any todos.
    
    Also please update the description for this.



src/master/master.hpp
Lines 2743-2752 (patched)
<https://reviews.apache.org/r/71029/#comment303671>

    how about total_reservaiton - used_reservation:
    
    ```
    ResourceQuantities unallocatedReservation;
    
     foreachvalue (Slave* slave, master->slaves.registered) {
       ResourceQuantities totalReservation =
         ResourceQuantities::fromScalarResources(
           slave->totalResources.filter(reservedToRoleSubtree).scalars());
           
       ResourceQuantities usedReservation =
         ResourceQuantities::fromScalarResources(
           slave->usedResources.filter(reservedToRoleSubtree).scalars());
       
       unallocatedReservation += totalReservation - usedReservation;
     }
    
    ```
    
    Should also be more performant to use ResourceQuantities and no need to clear the allocation
info.



src/master/master.hpp
Lines 2758 (patched)
<https://reviews.apache.org/r/71029/#comment303672>

    Nits, how about avoiding the addition and the two variables by just having a `consumption`:
    
    ```
    // Quota consumption = allocation + unallocated reservation
    
    ResourceQuantities consumption;
    
    // Add allocation
    
    ....
    
    // Add unallocated reservation
    ...
    
    return consumption;
    
    ```


- Meng Zhu


On July 8, 2019, 11:07 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71029/
> -----------------------------------------------------------
> 
> (Updated July 8, 2019, 11:07 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9871
>     https://issues.apache.org/jira/browse/MESOS-9871
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Quota consumption is computed as follows:
> 
>   Allocation + Unallocated Reservation ==
>   Reservations + Unreserved Allocation
> 
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
> 
> 
> Diffs
> -----
> 
>   src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
> 
> 
> Diff: https://reviews.apache.org/r/71029/diff/1/
> 
> 
> Testing
> -------
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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