You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2016/01/03 04:04:01 UTC
Review Request 41847: WIP: Updated allocation slack when slave was
updated.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
Review request for mesos and Klaus Ma.
Repository: mesos
Description
-------
WIP: Updated allocation slack when slave was updated.
Diffs
-----
src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
Diff: https://reviews.apache.org/r/41847/diff/
Testing
-------
Thanks,
Guangya Liu
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
(Updated 一月 23, 2016, 3:26 a.m.)
Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145
Repository: mesos
Description
-------
Updated allocation slack when slave was updated.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp 65c7e6b15c5308c0910667e1b12f39b21293a316
src/tests/hierarchical_allocator_tests.cpp b1cb955b7eb1213c7ba4a9c5181545bb49154f06
Diff: https://reviews.apache.org/r/41847/diff/
Testing
-------
make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
Thanks,
Guangya Liu
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Jan. 15, 2016, 6:21 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
>
> Joseph Wu wrote:
> I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
>
> The current patch is a pretty roundabout way of writing this:
> ```
> slaves[slaveId].total = oversubscribed +
> slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
> ```
>
> ^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
>
> Guangya Liu wrote:
> My logic here is that for update `updateSlave`, do not touch other logic but only add the `allocationSlack` when `enableReservationOversubscription` is enabled, this may be cleaner? As I was only adding `allocationSlack` when `enableReservationOversubscription` is enabled.
The `enableReservationOversubscription` flag shouldn't even matter here. If you really want to be strict, you can add this:
```
if (enableReservationOversubscription) {
CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
}
```
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On Jan. 13, 2016, 4:55 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 4:55 a.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 16, 2016, 2:21 a.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On 一月 13, 2016, 12:55 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 12:55 p.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Jan. 15, 2016, 6:21 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
The current patch is a pretty roundabout way of writing this:
```
slaves[slaveId].total = oversubscribed +
slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
```
^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On Jan. 13, 2016, 4:55 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 4:55 a.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Jan. 15, 2016, 6:21 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
>
> Joseph Wu wrote:
> I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
>
> The current patch is a pretty roundabout way of writing this:
> ```
> slaves[slaveId].total = oversubscribed +
> slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
> ```
>
> ^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
>
> Guangya Liu wrote:
> My logic here is that for update `updateSlave`, do not touch other logic but only add the `allocationSlack` when `enableReservationOversubscription` is enabled, this may be cleaner? As I was only adding `allocationSlack` when `enableReservationOversubscription` is enabled.
>
> Joseph Wu wrote:
> The `enableReservationOversubscription` flag shouldn't even matter here. If you really want to be strict, you can add this:
> ```
> if (enableReservationOversubscription) {
> CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
> }
> ```
>
> Guangya Liu wrote:
> @Joseph, so the logic you proposed is as this?
>
> if (enableReservationOversubscription) {
> slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> } else {
> slaves[slaveId].total = total.nonRevocable() + oversubscribed;
> }
>
> Yes, the above logic can make allocator call only 1 filter. If this is your proposal, I will update the patch and move the `nonUsage` before this patch.
Not quite:
```
slaves[slaveId].total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
// Only add this if you really want to check the flag.
// But optimistic offers shouldn't even be considered in this method.
if (enableReservationOversubscription) {
CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
}
```
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On Jan. 13, 2016, 4:55 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 4:55 a.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 16, 2016, 2:21 a.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
>
> Joseph Wu wrote:
> I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
>
> The current patch is a pretty roundabout way of writing this:
> ```
> slaves[slaveId].total = oversubscribed +
> slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
> ```
>
> ^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
My logic here is that for update `updateSlave`, do not touch other logic but only add the `allocationSlack` when `enableReservationOversubscription` is enabled, this may be cleaner? As I was only adding `allocationSlack` when `enableReservationOversubscription` is enabled.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On 一月 13, 2016, 12:55 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 12:55 p.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 16, 2016, 2:21 a.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
>
> Joseph Wu wrote:
> I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
>
> The current patch is a pretty roundabout way of writing this:
> ```
> slaves[slaveId].total = oversubscribed +
> slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
> ```
>
> ^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
>
> Guangya Liu wrote:
> My logic here is that for update `updateSlave`, do not touch other logic but only add the `allocationSlack` when `enableReservationOversubscription` is enabled, this may be cleaner? As I was only adding `allocationSlack` when `enableReservationOversubscription` is enabled.
>
> Joseph Wu wrote:
> The `enableReservationOversubscription` flag shouldn't even matter here. If you really want to be strict, you can add this:
> ```
> if (enableReservationOversubscription) {
> CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
> }
> ```
@Joseph, so the logic you proposed is as this?
if (enableReservationOversubscription) {
slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
} else {
slaves[slaveId].total = total.nonRevocable() + oversubscribed;
}
Yes, the above logic can make allocator call only 1 filter. If this is your proposal, I will update the patch and move the `nonUsage` before this patch.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On 一月 13, 2016, 12:55 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 12:55 p.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 16, 2016, 2:21 a.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 529-535
> > <https://reviews.apache.org/r/41847/diff/4/?file=1195636#file1195636line529>
> >
> > What if you did this?
> > ```
> > slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> > ```
> >
> > Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
> >
> > ---
> >
> > This would have fewer operations too (1 filter operation instead of 2).
>
> Guangya Liu wrote:
> I think that the current logic is that when enableReservationOversubscription is true, we only need to add the allocation slack to the total resources, there is no other change except this.
>
> The `allocationSlack()` also has only 1 filter `isAllocationSlack` , comments?
>
> Joseph Wu wrote:
> I'm talking about the `.nonRevocable()` and the `allocationSlack()` filters.
>
> The current patch is a pretty roundabout way of writing this:
> ```
> slaves[slaveId].total = oversubscribed +
> slaves[slaveId].total.filter([](const Resource& resource) { return !isUsageSlack(resource); });
> ```
>
> ^ This makes it explicit that we're just resetting the `USAGE_SLACK` component and leaving everything else untouched.
>
> Guangya Liu wrote:
> My logic here is that for update `updateSlave`, do not touch other logic but only add the `allocationSlack` when `enableReservationOversubscription` is enabled, this may be cleaner? As I was only adding `allocationSlack` when `enableReservationOversubscription` is enabled.
>
> Joseph Wu wrote:
> The `enableReservationOversubscription` flag shouldn't even matter here. If you really want to be strict, you can add this:
> ```
> if (enableReservationOversubscription) {
> CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
> }
> ```
>
> Guangya Liu wrote:
> @Joseph, so the logic you proposed is as this?
>
> if (enableReservationOversubscription) {
> slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
> } else {
> slaves[slaveId].total = total.nonRevocable() + oversubscribed;
> }
>
> Yes, the above logic can make allocator call only 1 filter. If this is your proposal, I will update the patch and move the `nonUsage` before this patch.
>
> Joseph Wu wrote:
> Not quite:
> ```
> slaves[slaveId].total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
>
> // Only add this if you really want to check the flag.
> // But optimistic offers shouldn't even be considered in this method.
> if (enableReservationOversubscription) {
> CHECK_EQ(Resources(), slaves[slaveId].total.allocationSlack());
> }
> ```
Got it, will update here to `slaves[slaveId].total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;` as you proposed. Thanks ;-)
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
On 一月 13, 2016, 12:55 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 12:55 p.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/#review114832
-----------------------------------------------------------
Test looks good. Just a suggestion:
src/master/allocator/mesos/hierarchical.cpp (lines 529 - 535)
<https://reviews.apache.org/r/41847/#comment175684>
What if you did this?
```
slaves[slaveId]total = slaves[slaveId].total.nonUsageSlack() + oversubscribed;
```
Where `nonUsageSlack` is a helper that filters out usage slack. (You could just use a Resource filter directly here.)
---
This would have fewer operations too (1 filter operation instead of 2).
- Joseph Wu
On Jan. 13, 2016, 4:55 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41847/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 4:55 a.m.)
>
>
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
>
>
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated allocation slack when slave was updated.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
> src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
>
> Diff: https://reviews.apache.org/r/41847/diff/
>
>
> Testing
> -------
>
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
(Updated 一月 13, 2016, 12:55 p.m.)
Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145
Repository: mesos
Description
-------
Updated allocation slack when slave was updated.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp d541bfa3f4190865c65d35c9d1ffdb8a3f194056
src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
Diff: https://reviews.apache.org/r/41847/diff/
Testing
-------
make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
Thanks,
Guangya Liu
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
(Updated 一月 13, 2016, 2:32 a.m.)
Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
Changes
-------
Rebase
Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145
Repository: mesos
Description
-------
Updated allocation slack when slave was updated.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f
src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
Diff: https://reviews.apache.org/r/41847/diff/
Testing (updated)
-------
make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose
Thanks,
Guangya Liu
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
(Updated 一月 8, 2016, 6:59 a.m.)
Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145
Repository: mesos
Description
-------
Updated allocation slack when slave was updated.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f
src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
Diff: https://reviews.apache.org/r/41847/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 41847: Updated allocation slack when slave was
updated.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41847/
-----------------------------------------------------------
(Updated 一月 3, 2016, 9:22 a.m.)
Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
Summary (updated)
-----------------
Updated allocation slack when slave was updated.
Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145
Repository: mesos
Description (updated)
-------
Updated allocation slack when slave was updated.
Diffs
-----
src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
src/tests/hierarchical_allocator_tests.cpp e044f832c2c16e53e663c6ced5452649bb0dcb59
Diff: https://reviews.apache.org/r/41847/diff/
Testing (updated)
-------
make
make check
Thanks,
Guangya Liu