You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anindya Sinha <an...@apple.com> on 2017/05/19 18:26:42 UTC
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated May 19, 2017, 6:26 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Updates based on agreed approach.
Summary (updated)
-----------------
Offers not sent for suppressed roles as indicated in `SUBSCRIBE`.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description (updated)
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 02affe2d6dc76ef91363df04d8d8cbed3beaf34f
src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
Diff: https://reviews.apache.org/r/57817/diff/8/
Changes: https://reviews.apache.org/r/57817/diff/7-8/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Line 309 (original), 309 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689440#file1689440line312>
> >
> > I don't quite follow why you need to have this variable here? Looks like the sorter can be the source of truth of whether a role is activated or not?
`FrameworkInfo::roles` indicates all the roles the framework registers as, and `Call::Subscribe::suppressed_roles` is a subset of roles for which the master would not offer resources. The effective `suppressed_roles` can change (after the framework is (re)registered) with `SUPPRESS` and `REVIVE` calls, which means the roles for which offers are sent to frameworks can change based on roles contained in these messages.
Now, the existing `activateFramework()` (and `deactivateFramework()`) activates (and deactivates) all roles of the framework. I think that should not be the case any longer. We should activate (and deactivate) roles of the framework which are not contained in `suppressed_roles` at any given point of time. To achieve this, we keep track of `HierarchicalAllocatorProcess::Framework::Framework::suppressed_roles` to ensure we keep track of `suppressed_roles` based on `SUPPRESS` and `REVIVE` calls during the life cycle of the framework. This would help us determine if we activate (or deactivate) for a specific role based on whether the role is a `suppressed_roles`.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 348 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689441#file1689441line352>
> >
> > why the if? is the `sorter::activate` not idempotent?
See response to the first comment.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 370 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689441#file1689441line374>
> >
> > ditto.
Same as the response to the first comment.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Lines 2804-2808 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689443#file1689443line2809>
> >
> > why do you need this check? is the below one not sufficient?
Just an optimization to not loop through the roles in `suppressedRoles` incase the sizes of `frameworkRoles` and `suppressedRoles` differ. But I think it should be fine to loop through since we do not anticipate the number of roles to be too many.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review175846
-----------------------------------------------------------
On May 23, 2017, 10:19 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 10:19 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
> src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 1c89a1159d8ac01a40f567beb14ed424ac613912
> src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
> src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
> src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
> src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
> src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
> src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
>
>
> Diff: https://reviews.apache.org/r/57817/diff/9/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Vinod Kone <vi...@gmail.com>.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Line 309 (original), 309 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689440#file1689440line312>
> >
> > I don't quite follow why you need to have this variable here? Looks like the sorter can be the source of truth of whether a role is activated or not?
>
> Anindya Sinha wrote:
> `FrameworkInfo::roles` indicates all the roles the framework registers as, and `Call::Subscribe::suppressed_roles` is a subset of roles for which the master would not offer resources. The effective `suppressed_roles` can change (after the framework is (re)registered) with `SUPPRESS` and `REVIVE` calls, which means the roles for which offers are sent to frameworks can change based on roles contained in these messages.
>
> Now, the existing `activateFramework()` (and `deactivateFramework()`) activates (and deactivates) all roles of the framework. I think that should not be the case any longer. We should activate (and deactivate) roles of the framework which are not contained in `suppressed_roles` at any given point of time. To achieve this, we keep track of `HierarchicalAllocatorProcess::Framework::Framework::suppressed_roles` to ensure we keep track of `suppressed_roles` based on `SUPPRESS` and `REVIVE` calls during the life cycle of the framework. This would help us determine if we activate (or deactivate) for a specific role based on whether the role is a `suppressed_roles`.
I see. Makes sense. Can you add a comment to activateFramework/deactivateFramework about the new semantics for posterity?
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Lines 2804-2808 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689443#file1689443line2809>
> >
> > why do you need this check? is the below one not sufficient?
>
> Anindya Sinha wrote:
> Just an optimization to not loop through the roles in `suppressedRoles` incase the sizes of `frameworkRoles` and `suppressedRoles` differ. But I think it should be fine to loop through since we do not anticipate the number of roles to be too many.
looks like you fixed it, not sure why you marked it as "dropped".
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review175846
-----------------------------------------------------------
On May 23, 2017, 10:19 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 10:19 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
> src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 1c89a1159d8ac01a40f567beb14ed424ac613912
> src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
> src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
> src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
> src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
> src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
> src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
>
>
> Diff: https://reviews.apache.org/r/57817/diff/9/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Line 309 (original), 309 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689440#file1689440line312>
> >
> > I don't quite follow why you need to have this variable here? Looks like the sorter can be the source of truth of whether a role is activated or not?
>
> Anindya Sinha wrote:
> `FrameworkInfo::roles` indicates all the roles the framework registers as, and `Call::Subscribe::suppressed_roles` is a subset of roles for which the master would not offer resources. The effective `suppressed_roles` can change (after the framework is (re)registered) with `SUPPRESS` and `REVIVE` calls, which means the roles for which offers are sent to frameworks can change based on roles contained in these messages.
>
> Now, the existing `activateFramework()` (and `deactivateFramework()`) activates (and deactivates) all roles of the framework. I think that should not be the case any longer. We should activate (and deactivate) roles of the framework which are not contained in `suppressed_roles` at any given point of time. To achieve this, we keep track of `HierarchicalAllocatorProcess::Framework::Framework::suppressed_roles` to ensure we keep track of `suppressed_roles` based on `SUPPRESS` and `REVIVE` calls during the life cycle of the framework. This would help us determine if we activate (or deactivate) for a specific role based on whether the role is a `suppressed_roles`.
>
> Vinod Kone wrote:
> I see. Makes sense. Can you add a comment to activateFramework/deactivateFramework about the new semantics for posterity?
Added a comment in `activateFramework()` and `deactivateFramework()`.
> On May 23, 2017, 9:01 p.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Lines 2804-2808 (patched)
> > <https://reviews.apache.org/r/57817/diff/4-8/?file=1689443#file1689443line2809>
> >
> > why do you need this check? is the below one not sufficient?
>
> Anindya Sinha wrote:
> Just an optimization to not loop through the roles in `suppressedRoles` incase the sizes of `frameworkRoles` and `suppressedRoles` differ. But I think it should be fine to loop through since we do not anticipate the number of roles to be too many.
>
> Vinod Kone wrote:
> looks like you fixed it, not sure why you marked it as "dropped".
Sorry, my mistake. Updated it.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review175846
-----------------------------------------------------------
On May 23, 2017, 10:19 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 10:19 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
> src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 1c89a1159d8ac01a40f567beb14ed424ac613912
> src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
> src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
> src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
> src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
> src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
> src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
>
>
> Diff: https://reviews.apache.org/r/57817/diff/9/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review175846
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp
Line 309 (original), 309 (patched)
<https://reviews.apache.org/r/57817/#comment249177>
I don't quite follow why you need to have this variable here? Looks like the sorter can be the source of truth of whether a role is activated or not?
src/master/allocator/mesos/hierarchical.cpp
Lines 348 (patched)
<https://reviews.apache.org/r/57817/#comment249178>
why the if? is the `sorter::activate` not idempotent?
src/master/allocator/mesos/hierarchical.cpp
Lines 370 (patched)
<https://reviews.apache.org/r/57817/#comment249179>
ditto.
src/master/master.cpp
Lines 2804-2808 (patched)
<https://reviews.apache.org/r/57817/#comment249174>
why do you need this check? is the below one not sufficient?
- Vinod Kone
On May 19, 2017, 6:26 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 6:26 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
> src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 02affe2d6dc76ef91363df04d8d8cbed3beaf34f
> src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
> src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
> src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
> src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
> src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
> src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
>
>
> Diff: https://reviews.apache.org/r/57817/diff/8/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/common/protobuf_utils.hpp
> > Lines 359-361 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735873#file1735873line359>
> >
> > This looks to be just a generic conversion from repeated ptr field to set, not something role specific?
> >
> > This means we should either inline the set construction (using the iterator approach), or expose a generic conversion (would we want to return an Error if there are duplicates?)
Agreed. Removed the function and calling it inline instead.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 391-395 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735877#file1735877line391>
> >
> > Is this stale? Seems to refer to "deactivated" roles whereas the variables are "suppressed" roles
Since `deactivate()` is idempotent (as in next comment), there is no need to add the `if (!framework.suppressedRoles.count(role))` condition, and hence I do not think the comment is needed here.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 449-450 (original), 479-481 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735877#file1735877line479>
> >
> > Not your bug, but it seems wrong to be activating roles here if the framework was deactivated. Would be good to sync with mpark on this original change. It seems to me that if the framework is deactivated via deactivateFramework, we wouldn't want to be activating things here in updateFrameowrk.
Reached out to @mpark. I kept it as-is till he has a chance to look into it.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 485 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735877#file1735877line485>
> >
> > Seems there is a bug here? What about existing roles (not covered by "removed roles" or "added roles" loops above) that have now become suppressed? For these, we want to deactivate them.
Yes, that is correct.
So, the roles that need to be deactivated are the roles which have been removed, as well as the roles which have moved from non-suppressed mode to suppressed mode. Similarly, the roles that need to be activated are the roles that have been added, as well as the roles which have moved from suppressed to non-suppressed mode.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176630
-----------------------------------------------------------
On June 1, 2017, 12:38 a.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 1, 2017, 12:38 a.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 6679d47ea53bbcd68d375654edf6e85890e5772a
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp e140f4d902a43b8fb2390541a357a217896b6228
>
>
> Diff: https://reviews.apache.org/r/57817/diff/12/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 449-450 (original), 479-481 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735877#file1735877line479>
> >
> > Not your bug, but it seems wrong to be activating roles here if the framework was deactivated. Would be good to sync with mpark on this original change. It seems to me that if the framework is deactivated via deactivateFramework, we wouldn't want to be activating things here in updateFrameowrk.
>
> Anindya Sinha wrote:
> Reached out to @mpark. I kept it as-is till he has a chance to look into it.
>
> Anindya Sinha wrote:
> So indeed, we need to activate roles if the framework is not deactivated. Can we push that fix in a separate JIRA/RR once this one is merged?
Added a TODO for now indicating that roles should be activated in `updateFramework()` only if the framework is active.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176630
-----------------------------------------------------------
On June 5, 2017, 10 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 5, 2017, 10 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/15/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On June 1, 2017, 5:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 449-450 (original), 479-481 (patched)
> > <https://reviews.apache.org/r/57817/diff/11/?file=1735877#file1735877line479>
> >
> > Not your bug, but it seems wrong to be activating roles here if the framework was deactivated. Would be good to sync with mpark on this original change. It seems to me that if the framework is deactivated via deactivateFramework, we wouldn't want to be activating things here in updateFrameowrk.
>
> Anindya Sinha wrote:
> Reached out to @mpark. I kept it as-is till he has a chance to look into it.
So indeed, we need to activate roles if the framework is not deactivated. Can we push that fix in a separate JIRA/RR once this one is merged?
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176630
-----------------------------------------------------------
On June 2, 2017, 5:49 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 5:49 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/13/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176630
-----------------------------------------------------------
src/common/protobuf_utils.hpp
Lines 359-361 (patched)
<https://reviews.apache.org/r/57817/#comment250001>
This looks to be just a generic conversion from repeated ptr field to set, not something role specific?
This means we should either inline the set construction (using the iterator approach), or expose a generic conversion (would we want to return an Error if there are duplicates?)
src/master/allocator/mesos/hierarchical.cpp
Lines 364-368 (patched)
<https://reviews.apache.org/r/57817/#comment249994>
Ditto here.
src/master/allocator/mesos/hierarchical.cpp
Lines 391-395 (patched)
<https://reviews.apache.org/r/57817/#comment249993>
Is this stale? Seems to refer to "deactivated" roles whereas the variables are "suppressed" roles
src/master/allocator/mesos/hierarchical.cpp
Line 377 (original), 399-401 (patched)
<https://reviews.apache.org/r/57817/#comment250000>
No need to guard this, it should be idempotent. It might end up being more readable without the reader might be looking for why we leave something activated when we're only trying to avoid deactivated what's already deactivated (but the if check is indirect so the reader has to know that the role being suppressed means it it deactivated already).
src/master/allocator/mesos/hierarchical.cpp
Line 419 (original), 446-448 (patched)
<https://reviews.apache.org/r/57817/#comment249995>
Deactivate is idempotent, no need to guard this? Ditto the thoughts on readability from above.
src/master/allocator/mesos/hierarchical.cpp
Lines 449-450 (original), 479-481 (patched)
<https://reviews.apache.org/r/57817/#comment249998>
Not your bug, but it seems wrong to be activating roles here if the framework was deactivated. Would be good to sync with mpark on this original change. It seems to me that if the framework is deactivated via deactivateFramework, we wouldn't want to be activating things here in updateFrameowrk.
src/master/allocator/mesos/hierarchical.cpp
Lines 485 (patched)
<https://reviews.apache.org/r/57817/#comment249997>
Seems there is a bug here? What about existing roles (not covered by "removed roles" or "added roles" loops above) that have now become suppressed? For these, we want to deactivate them.
- Benjamin Mahler
On June 1, 2017, 12:38 a.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 1, 2017, 12:38 a.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 6679d47ea53bbcd68d375654edf6e85890e5772a
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp e140f4d902a43b8fb2390541a357a217896b6228
>
>
> Diff: https://reviews.apache.org/r/57817/diff/12/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated June 5, 2017, 10 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Added a TODO indicating that roles should be activated in `updateFramework()` only if the framework is active.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
Diff: https://reviews.apache.org/r/57817/diff/15/
Changes: https://reviews.apache.org/r/57817/diff/14-15/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176940
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On June 2, 2017, 9:29 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 9:29 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/14/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated June 2, 2017, 9:29 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Addressed a comment.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
Diff: https://reviews.apache.org/r/57817/diff/14/
Changes: https://reviews.apache.org/r/57817/diff/13-14/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On June 2, 2017, 7:51 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 463 (patched)
> > <https://reviews.apache.org/r/57817/diff/12-13/?file=1736091#file1736091line474>
> >
> > what about new roles that are also suppressed?
>
> Anindya Sinha wrote:
> We do not activate new roles that are suppressed. We only activate roles if they are not suppressed. That is handled here:
>
> ```
> if (!suppressedRoles.count(role)) {
> frameworkSorters.at(role)->activate(frameworkId.value());
> }
> ```
Consolidated this within `rolesToActivate`.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176833
-----------------------------------------------------------
On June 2, 2017, 9:29 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 9:29 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/14/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
> On June 2, 2017, 7:51 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 430-431 (patched)
> > <https://reviews.apache.org/r/57817/diff/12-13/?file=1736091#file1736091line438>
> >
> > what about roles that are suppressed in old state and new state?
If a specific role was suppressed before, and is also supressed now, then that role is already deactivated. I do not think we need to deactivate them again. Similarly, if a role is non-suppressed before and after, we do not need to activate them since they are already activated. Or am I missing something?
> On June 2, 2017, 7:51 p.m., Vinod Kone wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 463 (patched)
> > <https://reviews.apache.org/r/57817/diff/12-13/?file=1736091#file1736091line474>
> >
> > what about new roles that are also suppressed?
We do not activate new roles that are suppressed. We only activate roles if they are not suppressed. That is handled here:
```
if (!suppressedRoles.count(role)) {
frameworkSorters.at(role)->activate(frameworkId.value());
}
```
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176833
-----------------------------------------------------------
On June 2, 2017, 5:49 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 5:49 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/13/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176833
-----------------------------------------------------------
On June 2, 2017, 9:29 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 9:29 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/14/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176833
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp
Lines 430-431 (patched)
<https://reviews.apache.org/r/57817/#comment250312>
what about roles that are suppressed in old state and new state?
src/master/allocator/mesos/hierarchical.cpp
Lines 463 (patched)
<https://reviews.apache.org/r/57817/#comment250313>
what about new roles that are also suppressed?
- Vinod Kone
On June 2, 2017, 5:49 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated June 2, 2017, 5:49 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
> src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
> src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
> src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
> src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
> src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
> src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
> src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
> src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
> src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
> src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
>
>
> Diff: https://reviews.apache.org/r/57817/diff/13/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated June 2, 2017, 5:49 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Addressed comments.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
src/master/allocator/mesos/hierarchical.cpp 8ebdbc6a2b98feab7ce4d7f07b15d8fb92992270
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
src/tests/slave_recovery_tests.cpp 86cf971aa8aea012a74740a4dcbaf5ed2ad9c866
Diff: https://reviews.apache.org/r/57817/diff/13/
Changes: https://reviews.apache.org/r/57817/diff/12-13/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated June 1, 2017, 12:38 a.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Fixed merge conflict.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp b31fbcfc18cbbe1e6b5fb5ccbc234f790326a5b4
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/allocator.hpp b2dcb566c49a1bbb1d955ca46e1c8eef91e62733
src/master/allocator/mesos/hierarchical.hpp 5e7c3068061012c51d4b9220dedf476408016a12
src/master/allocator/mesos/hierarchical.cpp 6679d47ea53bbcd68d375654edf6e85890e5772a
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp c66907bd55cb2eb549ec89f048d41376df556eb9
src/tests/allocator.hpp 0b9fdeaccc65bb6988103ac156e0584cf5df17fe
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp 0082045ccaebe015725c5e178aa89dac32b1c50c
src/tests/persistent_volume_endpoints_tests.cpp 9b820b8f4e0d0e2c62b5c9b420b790cdfb59dd0c
src/tests/reservation_tests.cpp 34628eed671aee7ed69497d282c8043d65620c14
src/tests/resource_offers_tests.cpp 7ad037e5ec1d63cc8c2b8b0ad092e3f5f6f402c8
src/tests/slave_recovery_tests.cpp e140f4d902a43b8fb2390541a357a217896b6228
Diff: https://reviews.apache.org/r/57817/diff/12/
Changes: https://reviews.apache.org/r/57817/diff/11-12/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated May 31, 2017, 2:52 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Fix compilation for tests.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
src/master/allocator/mesos/hierarchical.hpp 82fea40ad09c95f0096934c4210a39a6f0638ed9
src/master/allocator/mesos/hierarchical.cpp ca368bb5ef0dcfe83672004d58a4de6f85d6b4bc
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 14007e08f509446005423e223d5dd76a70744e27
src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp b9e04226a1688499847bc25c7c220c0b82ba8db7
src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668
src/tests/reservation_tests.cpp 47ccf7f6f6ee48236f6794ce3084ce4f425c93c5
src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
src/tests/slave_recovery_tests.cpp df0c5c88786190be06df7ef3602834aa8985cefe
Diff: https://reviews.apache.org/r/57817/diff/11/
Changes: https://reviews.apache.org/r/57817/diff/10-11/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated May 31, 2017, 8 a.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Addressed review comments.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
src/master/allocator/mesos/hierarchical.hpp 82fea40ad09c95f0096934c4210a39a6f0638ed9
src/master/allocator/mesos/hierarchical.cpp ca368bb5ef0dcfe83672004d58a4de6f85d6b4bc
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 14007e08f509446005423e223d5dd76a70744e27
src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
src/tests/hierarchical_allocator_tests.cpp eb2b647d3247a85e8b9b82e5589232c74ad8570f
src/tests/master_allocator_tests.cpp b9e04226a1688499847bc25c7c220c0b82ba8db7
src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668
src/tests/reservation_tests.cpp 47ccf7f6f6ee48236f6794ce3084ce4f425c93c5
src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
src/tests/slave_recovery_tests.cpp df0c5c88786190be06df7ef3602834aa8985cefe
Diff: https://reviews.apache.org/r/57817/diff/10/
Changes: https://reviews.apache.org/r/57817/diff/9-10/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/#review176414
-----------------------------------------------------------
Ship it!
LGTM. Would be great if one of @bmahler, @mpark or @neilc can take a look as well.
- Vinod Kone
On May 23, 2017, 10:19 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 10:19 p.m.)
>
>
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
> framework (re)registration, the allocator does not offer resources for
> those roles to such frameworks. Note that this functionality is added
> for `v1::SUBSCRIBE` only (and not for scheduler driver API).
>
> In addition, the master validates the suppressed roles to be a subset
> of all the roles being (re)registered by the framework.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
> src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
> src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
> src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
> src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 1c89a1159d8ac01a40f567beb14ed424ac613912
> src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
> src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
> src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
> src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
> src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
> src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
>
>
> Diff: https://reviews.apache.org/r/57817/diff/9/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 57817: Offers not sent for suppressed roles as
indicated in `SUBSCRIBE`.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57817/
-----------------------------------------------------------
(Updated May 23, 2017, 10:19 p.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Addressed review comments.
Bugs: MESOS-7015
https://issues.apache.org/jira/browse/MESOS-7015
Repository: mesos
Description
-------
If the `SUBSCRIBE` indicates a subset of roles to be suppressed during
framework (re)registration, the allocator does not offer resources for
those roles to such frameworks. Note that this functionality is added
for `v1::SUBSCRIBE` only (and not for scheduler driver API).
In addition, the master validates the suppressed roles to be a subset
of all the roles being (re)registered by the framework.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7
src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6
src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d
src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b
src/master/allocator/mesos/hierarchical.hpp 123f97cf495bff0f822838e09df0d88818f04da6
src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 1c89a1159d8ac01a40f567beb14ed424ac613912
src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0
src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
src/tests/master_allocator_tests.cpp d05ee441a5120144aff42d78c095e1ce5051a6ac
src/tests/persistent_volume_endpoints_tests.cpp 8c54372b7f6d94f0335561086f2a8cb90373e285
src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad
src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c
src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11
Diff: https://reviews.apache.org/r/57817/diff/9/
Changes: https://reviews.apache.org/r/57817/diff/8-9/
Testing
-------
All tests passed.
Thanks,
Anindya Sinha