----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57564/#review170491 -----------------------------------------------------------
src/master/allocator/sorter/drf/sorter.hpp Lines 214-216 (original), 215-217 (patched) <https://reviews.apache.org/r/57564/#comment243325> This comment is no longer accurate, right? We used to store a distinct structure purely because of the deactivated clients being absent from the `clients` map. Now that this is no longer the case, it seems like this patch should be removing this additional data structure (which needs to be kept in sync) and just storing `Allocation` within Client. Won't we need to do this anyway for the hierarchy change? Seems clearer to me to make that change in this patch. src/tests/sorter_tests.cpp Lines 243 (patched) <https://reviews.apache.org/r/57564/#comment243324> Thanks for adding these comments! src/tests/sorter_tests.cpp Line 267 (original), 268 (patched) <https://reviews.apache.org/r/57564/#comment243322> Looks like c is supposed to have a count of 5 here? src/tests/sorter_tests.cpp Lines 282 (patched) <https://reviews.apache.org/r/57564/#comment243323> Ditto here? - Benjamin Mahler On March 13, 2017, 6:04 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57564/ > ----------------------------------------------------------- > > (Updated March 13, 2017, 6:04 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > ------- > > DRFSorter previously removed inactive clients from the `clients` > collection, and then re-added clients when they were reactivated. This > resulted in resetting the allocation count for the client, which is > unfortunate. This scheme would also be more difficult to adapt to > hierarchical sorting. > > This commit changes DRFSorter to continue to store inactive clients in > the `clients`; inactive clients are indicated by a new field in the > `Client` struct, and are omitted from the return value of > `DRFSorter::sort`. > > > Diffs > ----- > > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 > > > Diff: https://reviews.apache.org/r/57564/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Neil Conway > >
