> On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1016-1031 > > <https://reviews.apache.org/r/41075/diff/6/?file=1163905#file1163905line1016> > > > > If the allocator's roleSorter doesn't know about the role (i.e. no > > frameworks are registered with that role), do we still want to make an > > explicit `allocate()` call on a SetQuota request for that role? Will > > setting quota on an unregistered role have any impact on the fair shares of > > other frameworks? > > Neil Conway wrote: > It is possible for `roleSorter` to not know about the role but for > `quotaRoleSorter` to know about it -- i.e., if the role has a configured > quota but no active frameworks. In general, I can imagine circumstances in > which we would still want to call `allocate()` here (e.g., if we end up > revoking/rescinding offers to non-quota frameworks to preserve resources > needed by a quota'd role). Perhaps AlexR can comment?
Once quota is set for a role, the next `allocate()` call will lay away resources. I think Adam means that we do not necessarily need to allocate straight away because there are no consumers (no active frameworks in the quota'ed role) of these resources. Hence though quota on an unregistered role (I assume it means a role without any frameworks) does impact other frameworks, we should not necessarily rush with `allocate()`. However, does it make sense to introduce a condition here? What is the advantage? One extra `allocate()` call should not be a big deal, since set quota operation won't happen that often. So even Adam has the point, I would leave the code as is for readability. > On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > src/master/http.cpp, lines 1601-1602 > > <https://reviews.apache.org/r/41075/diff/6/?file=1163906#file1163906line1601> > > > > Why do you list unregistered roles with quota configured, if you don't > > model/display their quota? > > If you think quota (and roles configured for quota) should show up in > > `/roles`, file a JIRA and we'll do it all right in one pass. > > Neil Conway wrote: > This was per discussion with AlexR. The idea is that, previously, > "/roles" showed all the "potentially interesting" roles: by definition, it > will include any role that has a non-default weight or non-default quota. > With implicit roles, we want to show the same set of roles that have > configured properties. > > Whether we show a role with configured quota is separate from whether we > show quota information about any of the roles. For the latter, not sure if we > want that information in "/roles", quota-related endpoints, or both. In any > case, seems fine to defer this part of it. That's correct. Let me repeat my argument one more time: once a quota becomes "whitelisted" or, I prefer, "visible", we should show it in "/roles" to make operator's life easier. If an operator sets a quota for a role, they should see this role in "/roles", otherwise it becomes very tedious to track all "whitelisted" roles and potential typos. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110449 ----------------------------------------------------------- On Dec. 16, 2015, 4:19 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41075/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2015, 4:19 a.m.) > > > Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg > Mann, and Yongqiao Wang. > > > Bugs: MESOS-4085 > https://issues.apache.org/jira/browse/MESOS-4085 > > > Repository: mesos > > > Description > ------- > > Changed the behavior of the master when the `--roles` flag is NOT > specified. Previously, this would allow only the `*` role to be used. Now, > omitting `--roles` means that any role can be used. This is called "implicit > roles". Configuring which principals can perform operations as which roles > should be done using ACLs in the authorization system. > > Note that this changes the behavior of the system when `--roles` is not > specified. This is likely acceptable: if the operator didn't specify `--roles` > in prior versions of Mesos, they were likely not using roles or authorization > at > that time. > > Another minor behavioral change is that the "/roles" endpoint will now only > return results for currently "active" roles (those with one or more registered > frameworks). > > The `--roles` flag is now considered deprecated and will be removed in a > future > version of Mesos. > > > Diffs > ----- > > include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 > include/mesos/master/allocator.proto > 702f56f56c3b1331613cecf26522986f6b572f8c > src/master/allocator/mesos/allocator.hpp > 97ee80726ad155917811265a983258b0165d3451 > src/master/allocator/mesos/hierarchical.hpp > 99c742906874c30c39c159e58a65277ade3c07fd > src/master/allocator/mesos/hierarchical.cpp > 5da825a1d578a9ee40b4985378fddb3c5fb3b416 > src/master/http.cpp 37fbcb93074fe189133161afaa28046dc2ad0731 > src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 > src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 > src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272 > src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 > src/tests/hierarchical_allocator_tests.cpp > e239b4746494fcc2b362a83afb634a2ce5e25f9b > src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 > > Diff: https://reviews.apache.org/r/41075/diff/ > > > Testing > ------- > > "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the > more likely role-related tests. > > TODOs: > > * Add tests for allocation behavior for weights + implicit roles > * More tests for quota + implicit roles? > > Notes: > > * There's two places where we use manual `new`/`delete` where a `unique_ptr` > would probably be nicer. I'm inclined to leave this as-is for now though > (making use of unique_ptr is a broader issue). > > > Thanks, > > Neil Conway > >
