> On June 2, 2017, 4:26 p.m., Neil Conway wrote: > > src/master/allocator/sorter/drf/sorter.hpp > > Lines 347 (patched) > > <https://reviews.apache.org/r/57935/diff/3/?file=1716055#file1716055line347> > > > > Why is `flatten` necessary here? We are already comparing two scalar > > quantities, so is the flatten required? > > Anindya Sinha wrote: > In the case of a `RESERVE` or `UNRESERVE` operation, the `newAllocation` > and `oldAllocation` varies in distribution of resources across roles. > > eg. For `RESERVE`, oldAllocation = `disk(*):100`, and newAllocation = > `disk(*):90; disk(role1):10`. > > So, `createStrippedScalarQuantities()` on these `Resources` shall drop > the `ReservationInfo` but the roles will remain intact. > Without `flatten()`, `oldAllocationQuantity` and `newAllocationQuantity` > will not be the same (due to different roles) and hence `dirty` shall be set. > But I do not think we need to recalculate shares since the total resources > have not changed (only the distribution has changed in terms of roles). That > is the reason for the comparison having the `flatten()`. > > Looking at when `dirty` is true: We update `totals` (which is hashmap > `<ResourceName, ScalarValue>` in `update()`. And when `calculateShare()` is > called, we calculate share based on totals in the Sorter and individual Node. > So I think we should be good to have the `flatten()` in this comparison. > > Let me know if this does not sound ok. > > Neil Conway wrote: > Sounds reasonable.
So, can this be merged now? - Anindya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57935/#review176687 ----------------------------------------------------------- On June 2, 2017, 4:59 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57935/ > ----------------------------------------------------------- > > (Updated June 2, 2017, 4:59 p.m.) > > > Review request for mesos and Neil Conway. > > > Bugs: MESOS-7138 > https://issues.apache.org/jira/browse/MESOS-7138 > > > Repository: mesos > > > Description > ------- > > In `DRFSorter::update`, we should set the `dirty` flag only when the > total scalar quantities have changed in any of the clients in the > hierarchy, and not always. > > > Diffs > ----- > > src/master/allocator/sorter/drf/sorter.hpp > 77e52dec735d276389643f7f356cd763b2f785e9 > src/master/allocator/sorter/drf/sorter.cpp > ecc5586737b6b447c5a1cf1a37037832bcbacd69 > > > Diff: https://reviews.apache.org/r/57935/diff/4/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Anindya Sinha > >
