> On March 17, 2017, 1:41 a.m., Benjamin Mahler wrote: > > Thinking a little more about this, it seems odd that the sorter is storing > > weight information for clients that are unknown to it. Why? It seems > > cleaner if the sorter only understands weights for its clients, rather than > > potential future clients. > > > > One way to resolve this would be to have roles with weights (but no > > allocations or frameworks subscribed to them) be present as inactive > > clients in the sorters, and using the existing interface. That seems to be > > in line with the approach in the next patch? > > Neil Conway wrote: > _Something_ needs to store weights for potential future clients. Can you > elaborate why you think it is cleaner for this knowledge to live in the > allocator versus in the sorter? Weights are actually a property of the > sorter; other than passing the appropriate weight configuration to the > sorters, the allocator doesn't actually care about weights. > > The next patch does not represent weights on internal nodes by using > inactive clients. Inactive clients still correspond to _sorter clients_; you > can set a weight on an arbitrary role path, which might not be associated > with any client (see the `TODO` around renaming the `Client` struct to > reflect this). Once you introduce hierarchical roles, the set of names which > have a weight defined and the set of client names might have very little in > common.
> Can you elaborate why you think it is cleaner for this knowledge to live in > the allocator versus in the sorter? I think the confusing aspect of this is that the sorter is "anticipating" future clients that may never arrive and have no effect on the sorter unless at some point they "arrive", which means we're storing unnecessary information as far as the sorter's responsibilities are concerned. As another point of confusion here, it seems that we can only "anticipate" some of the time: (1) If a role has a non-default weight, we can anticipate that it is a potential client. (2) If the role does not have a default weight, we can't anticipate it's potential as a client (i.e. all valid, authorized roles are potential clients). (3) We can't anticipate framework ids as clients. Something needs to store the configuration and I think it seems a mesos component that understands roles is more suited to that. > Once you introduce hierarchical roles, the set of names which have a weight > defined and the set of client names might have very little in common. It sounds like there are three states: (`potential client`, `inactive client`, `active client`)? Is it possible to avoid the potential client complication by treating potential clients the same as inactive clients in the hierarchical sorter changes? Why do we need to call out potential clients distinctly from inactive clients when the hierarchy is introduced? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57527/#review169247 ----------------------------------------------------------- On March 17, 2017, 1:12 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57527/ > ----------------------------------------------------------- > > (Updated March 17, 2017, 1:12 a.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park. > > > Repository: mesos > > > Description > ------- > > Previously, DRFSorter only kept track of weights for clients currently > in the sorter; the allocator supplied the current weight when adding a > client to the sorter. > > This commit changes the sorter to keep track of all weights, not just > those for the active clients in the sorter. The allocator can now just > pass along role weights to the role sorters, rather than needing to > track them itself. > > This commit changes the sorter API. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > f84b0574ce9a392c9528c87b04b01dbb2053cff7 > src/master/allocator/mesos/hierarchical.cpp > 8d54a8cca1bb478f4437f68c5e14f66a9f9bb9e9 > src/master/allocator/sorter/drf/sorter.hpp > 76329220e1115c1de7810fb69b943c78c078be59 > src/master/allocator/sorter/drf/sorter.cpp > ed54680cecb637931fc344fbcf8fd3b14cc24295 > src/master/allocator/sorter/sorter.hpp > b3029fcf7342406955760da53f1ae736769f308c > src/tests/sorter_tests.cpp ec0636beb936d46a253d19322f2157abe95156b6 > > > Diff: https://reviews.apache.org/r/57527/diff/3/ > > > Testing > ------- > > `make check` > > > Thanks, > > Neil Conway > >
