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 71029: Added Role::consumedQuota to the master.
Date Wed, 10 Jul 2019 01:19:41 GMT


> On July 9, 2019, 12:10 a.m., Meng Zhu wrote:
> > src/master/master.hpp
> > Lines 2730 (patched)
> > <https://reviews.apache.org/r/71029/diff/1/?file=2153963#file2153963line2730>
> >
> >     `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.

> totalUsedResources here also includes the executor resources which are not managed by
the allocator. Do we want to include that in the consumption?

Per offline discussion, this was the command executor case (where the agent generates the
ExecutorInfo and adds resources to it). The master doesn't store command executors, so it
doesn't have the same overcommit visible.


> On July 9, 2019, 12:10 a.m., Meng Zhu wrote:
> > src/master/master.hpp
> > Lines 2743-2752 (patched)
> > <https://reviews.apache.org/r/71029/diff/1/?file=2153963#file2153963line2743>
> >
> >     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.

Great suggestion, thanks! This is much better without the Resources arithmetic

(note that the `slave->usedResources` and needs to be looped over)


> On July 9, 2019, 12:10 a.m., Meng Zhu wrote:
> > src/master/master.hpp
> > Lines 2758 (patched)
> > <https://reviews.apache.org/r/71029/diff/1/?file=2153963#file2153963line2758>
> >
> >     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;
> >     
> >     ```

Hm.. showing the computation of 'allocation' and 'unallocated reservation' by having them
in variables and then showing the final addition (which is the same as the comment's addition)
seems easier to follow?


- Benjamin


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


On July 10, 2019, 1:18 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71029/
> -----------------------------------------------------------
> 
> (Updated July 10, 2019, 1:18 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 consumed quota will be problematic.
> 
> 
> Diffs
> -----
> 
>   src/master/framework.cpp c35de2574ef683a9f397a957422fdbf8fa751313 
>   src/master/master.hpp 23dafe746b6f9b3d70ad7220f54c4d49068b8af8 
> 
> 
> Diff: https://reviews.apache.org/r/71029/diff/2/
> 
> 
> Testing
> -------
> 
> Added a test in a subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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