----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72508/#review220765 -----------------------------------------------------------
Ship it! Looks good, feel free to split out the ScalarResourceTotals into a patch in front, to make the fix more minimal and clear. src/master/allocator/mesos/hierarchical.hpp Lines 115 (patched) <https://reviews.apache.org/r/72508/#comment309445> Hm.. "subset of scalar resources" makes me think of: some_subset(scalar resources) rather than what I think you intended: scalars(resources) Maybe just say tracking scalar totals across agents? Ideally spelling out if this is forgiving with the inputs or crashes if passing non scalars (had to read the code to see): ``` // Helper for tracking cross-agent scalar resource totals // (since directly summing scalar Resources across agents // is very expensive). // // This will implicitly filter out non-scalars from the // function inputs, the caller does not need to call // .scalars() beforehand. ``` src/master/allocator/mesos/hierarchical.hpp Lines 124 (patched) <https://reviews.apache.org/r/72508/#comment309447> `; }` src/master/allocator/mesos/hierarchical.hpp Lines 129 (patched) <https://reviews.apache.org/r/72508/#comment309446> maybe: ``` scalars scalarsTotal ``` seeing total next to scalar might lead one to think that total is not involving only scalars src/master/allocator/mesos/hierarchical.hpp Lines 155-161 (patched) <https://reviews.apache.org/r/72508/#comment309450> How about: `quotaOfferedPlusConsumed` `quotaOfferedOrConsumed` `quotaConsumedOrOffered` `quotaConsumedPlusOffered` The way I would want to think about this is that it's quota consumption with offered included vs quota consumption without factoring in what's offered. - Benjamin Mahler On May 13, 2020, 7:43 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72508/ > ----------------------------------------------------------- > > (Updated May 13, 2020, 7:43 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10128 > https://issues.apache.org/jira/browse/MESOS-10128 > > > Repository: mesos > > > Description > ------- > > Before this patch, the roles tree was tracking total resources > offered/allocated to a role as a single `Resources` objects. > In the case when each agent has a limited number of unique resources > (for example, a single persistent voulme), this resulted in poor > asymptotic complexity of allocation versus the number of agents > (O(N^2)) that was clearly observable in > `HierarchicalAllocations_BENCHMARK_Test.PersistentVolumes`. > In addition, the role tree code was violating the convention that > `Resources` belonging to different agents should never be added. > > This patch implements per-agent tracking of `Resources` in the roles > tree, thus improving the performance of allocation (and getting rid of > the potentially problematic O(N^2) asymptotic) in the case of many > agents with a limited number of unique resources each. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 6454cdaa19f776365df34ecf83114f0d6fa20f27 > src/master/allocator/mesos/hierarchical.cpp > 5fe9ffcb518b8427d663ddae43e550795d290e3c > > > Diff: https://reviews.apache.org/r/72508/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
