> 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 > >
