> On June 20, 2018, 2:20 p.m., Benjamin Bannier wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Line 921 (original) > > <https://reviews.apache.org/r/67669/diff/1/?file=2042688#file2042688line924> > > > > While this might not be required by the allocator, generally an > > operation changing the resource quantity is not supported by the `Sorter` > > interface. > > > > We already rely on the invariant being checked here in above calls to > > `Sorter::update` which explicit disallow such updates. In order to support > > operations changing the quantity of the stripped resources we need to do > > additional work.
I added a unit test and updated the description of `Sorter::update`, but I'm hesitating to add some more concrete checks. We discussed about checking the resources names don't change, but changing names are essentially removing the old name and adding a new one, so it seems not consistent to have such checks. Another concern I have is that we now rely on the caller to update the allocation and the total resources synchronously, but I cannot find a good way to validate that. Suggestions? - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67669/#review205079 ----------------------------------------------------------- On June 20, 2018, 5:34 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67669/ > ----------------------------------------------------------- > > (Updated June 20, 2018, 5:34 a.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Meng > Zhu. > > > Bugs: MESOS-8995 > https://issues.apache.org/jira/browse/MESOS-8995 > > > Repository: mesos > > > Description > ------- > > `HierarchicalAllocatorProcess::updateAllocation` checks that the given > resource conversions should not change the allocation quantities of the > specified framework. Although this is true for speculative operations, > this might not hold for arbitrary resource conversions in general. > > Since the allocator does not rely on this invariant, this patch relaxes > the invariant to make resource conversions more future-proof. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c > > > Diff: https://reviews.apache.org/r/67669/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
