Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Guangya Liu]

> On 一月 20, 2016, 10:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > 
> >
> > Have you considered modifying `Resources::apply` to "create" allocation 
> > slack upon a reservation?  (And to "destroy" allocation slack upon 
> > unreserve?)
> 
> Guangya Liu wrote:
> I do not want to update `Resources::apply` as it will be called when 
> `reserve` and `unreserve` resources, when `reserve` resources, we can simply 
> add some `allocation slack`; when `unreserve` resources, life become complex, 
> we cannot simply decrease the `allocation slack` as if the `allocation slack` 
> decreased, there will be new `unreserved` resources which can be send as 
> offer and launch task because the `allocation slack` still running task and 
> there might be resource confilict for this, that's why I have another 
> following two patches handlding `unreserve` separately.
> 
> Take a case:
> 1) agent: cpus(r1):100;cpus(*){ALLOCATION_SLACK}:100
> 2) framework use up all cpus(*){ALLOCATION_SLACK}:100 resources.
> 3) unreserve 30 cpus.
> 4) The resources would become:
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:100 NOTE: We do 
> not decrease the total allocation slack here.
> Free: cpus(*):30;cpus(r1):70
> Used: cpus(*){ALLOCATION_SLACK}:100
> 5) At this time, we cannot send cpus(*):30 out as offer because there are 
> still 30 over committed allocation slack in use.
> 6) recover 20 allocaiton slack, we can update the total resources as this:
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:80 NOTE: decrease 
> the total allocation slack when recover resource but not when `unreserve` 
> resources in such case.
> Used: cpus(*){ALLOCATION_SLACK}:80
> Free: cpus(*):30;cpus(r1):70
> The allocate can send out offer with cpus(*):20 but not the whole 30 
> resources.
> 7) recover another 10 allocation slack.
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:70 NOTE: from now 
> on, the allocatioin slack is same as reserved resources.
> Used: cpus(*)20;cpus(*){ALLOCATION_SLACK}:70
> Free: cpus(*):10;cpus(r1):70
> The allocater can send out offer with cpus(*):10.
> 
> Joseph Wu wrote:
> Hm...  How about this:
> - You calculate the "total" allocation slack strictly.  
> - * Reserve   => Total allocation slack increases.
> - * Unreserve => Total allocation slack decreases.
> - * Allocator::recoverResources => Total allocation slack unchanged.
> - * Master::removeExecutor (for executors using allocation slack)   => 
> Total allocation slack unchanged.
> - * Master::removeExecutor (for executors using reserved resources) => 
> Total allocation slack increases.
> - * Master::evictExecutor (new method) => Total allocation slack 
> decreases.
>  ^^ The agent can report exactly how much allocation slack should 
> decrease.  We recover all allocation slack from the evicted executor and 
> subtract reserved resources from the new ("evicting") executor.
> 
> In this scheme, the "allocated" allocation slack can surpass the "total" 
> (i.e. MESOS-4442), but the totals should remain consistent.

- * Reserve   => Total allocation slack increases.  > Yes
- * Unreserve => Total allocation slack decreases.  > Maybe no, as we 
cannto decrease the total allocation slack so early because some allocation 
slack maybe in use, so for unreserved case, we need to decrease the total 
allocation slack in `recoverResources` when there are some allocation slack 
resources returned back, check if the total allocation slack needs to be 
decreased or not.
- * Allocator::recoverResources => Total allocation slack unchanged. > As 
above, may need to decrease the total allocation slack for some unreserve cases.
- * Master::removeExecutor (for executors using allocation slack)   => Total 
allocation slack unchanged.  > This will call `recoverResources`, if the 
total allocation slack is greater than reserved resources, then need to 
decrease the allocation slack, this is mainly for some `unreserve` follow up 
acations.
- * Master::removeExecutor (for executors using reserved resources) => Total 
allocation slack increases. > This will call `recoverResources`, if 
recovering reserved resources, the total allocation slack for an agent will not 
be changed but the `available allocation slack` will be increased.


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115497
---


On 一月 20, 2016, 6:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> --

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Joseph Wu]

> On Jan. 20, 2016, 2:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > 
> >
> > Have you considered modifying `Resources::apply` to "create" allocation 
> > slack upon a reservation?  (And to "destroy" allocation slack upon 
> > unreserve?)
> 
> Guangya Liu wrote:
> I do not want to update `Resources::apply` as it will be called when 
> `reserve` and `unreserve` resources, when `reserve` resources, we can simply 
> add some `allocation slack`; when `unreserve` resources, life become complex, 
> we cannot simply decrease the `allocation slack` as if the `allocation slack` 
> decreased, there will be new `unreserved` resources which can be send as 
> offer and launch task because the `allocation slack` still running task and 
> there might be resource confilict for this, that's why I have another 
> following two patches handlding `unreserve` separately.
> 
> Take a case:
> 1) agent: cpus(r1):100;cpus(*){ALLOCATION_SLACK}:100
> 2) framework use up all cpus(*){ALLOCATION_SLACK}:100 resources.
> 3) unreserve 30 cpus.
> 4) The resources would become:
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:100 NOTE: We do 
> not decrease the total allocation slack here.
> Free: cpus(*):30;cpus(r1):70
> Used: cpus(*){ALLOCATION_SLACK}:100
> 5) At this time, we cannot send cpus(*):30 out as offer because there are 
> still 30 over committed allocation slack in use.
> 6) recover 20 allocaiton slack, we can update the total resources as this:
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:80 NOTE: decrease 
> the total allocation slack when recover resource but not when `unreserve` 
> resources in such case.
> Used: cpus(*){ALLOCATION_SLACK}:80
> Free: cpus(*):30;cpus(r1):70
> The allocate can send out offer with cpus(*):20 but not the whole 30 
> resources.
> 7) recover another 10 allocation slack.
> Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:70 NOTE: from now 
> on, the allocatioin slack is same as reserved resources.
> Used: cpus(*)20;cpus(*){ALLOCATION_SLACK}:70
> Free: cpus(*):10;cpus(r1):70
> The allocater can send out offer with cpus(*):10.

Hm...  How about this:
- You calculate the "total" allocation slack strictly.  
- * Reserve   => Total allocation slack increases.
- * Unreserve => Total allocation slack decreases.
- * Allocator::recoverResources => Total allocation slack unchanged.
- * Master::removeExecutor (for executors using allocation slack)   => Total 
allocation slack unchanged.
- * Master::removeExecutor (for executors using reserved resources) => Total 
allocation slack increases.
- * Master::evictExecutor (new method) => Total allocation slack decreases.
 ^^ The agent can report exactly how much allocation slack should decrease.  We 
recover all allocation slack from the evicted executor and subtract reserved 
resources from the new ("evicting") executor.

In this scheme, the "allocated" allocation slack can surpass the "total" (i.e. 
MESOS-4442), but the totals should remain consistent.


- Joseph


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115497
---


On Jan. 19, 2016, 10:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 19, 2016, 10:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Joseph Wu]

> On Jan. 19, 2016, 3:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > 
> >
> > I see several places where you overwrite changes from this review in 
> > the subsequent two.  Did you mean to have three reviews for the changes to 
> > `::updateAllocation`, `::updateAvailable`, and `::recoverResources`?
> 
> Guangya Liu wrote:
> There are three patches handling dynamic reservation and 
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> I have updated the description here.
> 
> Joseph Wu wrote:
> It doesn't seem like you need 3 patches.
> 
> I'd recommend a couple of cleanups:
> 
> * Add a helper for all the duplicate logic you have.  Both 
> `updateAllocation` and `updateAvailable` deal with reservations, in 
> essentially the same way (per optimistic offers).
> * Consolidate the overlapping changes into a single patch.  Just don't 
> overwrite your own changes.
> * Consider a different approach than subtracting allocation slack on 
> unreserve.  You could perhaps, for accounting purposes, elevate a portion of 
> the allocation from revocable to non-revocable.
> 
> Guangya Liu wrote:
> The reason I do not add a helper function because even though the code 
> are almost same, there are indeed difference between logs, `updateAllocation` 
> will have some logs related to `framework` and `agent`, `updateAvailable` 
> will only have some logs related to `agent`. I prefer that we keep the 
> changes separtely to make log message clear for different cases, comments? 
> Thanks.

For the log differences, you can just pass in the `FrameworkID` as an 
`Option frameworkId`.
Then, in your logs, add: `<< (frameworkId.isSome() ? " for framework " + 
frameworkId.get() : "")`.


- Joseph


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115279
---


On Jan. 19, 2016, 10:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 19, 2016, 10:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Guangya Liu]

> On 一月 19, 2016, 11:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > 
> >
> > I see several places where you overwrite changes from this review in 
> > the subsequent two.  Did you mean to have three reviews for the changes to 
> > `::updateAllocation`, `::updateAvailable`, and `::recoverResources`?
> 
> Guangya Liu wrote:
> There are three patches handling dynamic reservation and 
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> I have updated the description here.
> 
> Joseph Wu wrote:
> It doesn't seem like you need 3 patches.
> 
> I'd recommend a couple of cleanups:
> 
> * Add a helper for all the duplicate logic you have.  Both 
> `updateAllocation` and `updateAvailable` deal with reservations, in 
> essentially the same way (per optimistic offers).
> * Consolidate the overlapping changes into a single patch.  Just don't 
> overwrite your own changes.
> * Consider a different approach than subtracting allocation slack on 
> unreserve.  You could perhaps, for accounting purposes, elevate a portion of 
> the allocation from revocable to non-revocable.

The reason I do not add a helper function because even though the code are 
almost same, there are indeed difference between logs, `updateAllocation` will 
have some logs related to `framework` and `agent`, `updateAvailable` will only 
have some logs related to `agent`. I prefer that we keep the changes separtely 
to make log message clear for different cases, comments? Thanks.


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115279
---


On 一月 20, 2016, 6:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated 一月 20, 2016, 6:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Guangya Liu]

> On 一月 20, 2016, 10:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > 
> >
> > Have you considered modifying `Resources::apply` to "create" allocation 
> > slack upon a reservation?  (And to "destroy" allocation slack upon 
> > unreserve?)

I do not want to update `Resources::apply` as it will be called when `reserve` 
and `unreserve` resources, when `reserve` resources, we can simply add some 
`allocation slack`; when `unreserve` resources, life become complex, we cannot 
simply decrease the `allocation slack` as if the `allocation slack` decreased, 
there will be new `unreserved` resources which can be send as offer and launch 
task because the `allocation slack` still running task and there might be 
resource confilict for this, that's why I have another following two patches 
handlding `unreserve` separately.

Take a case:
1) agent: cpus(r1):100;cpus(*){ALLOCATION_SLACK}:100
2) framework use up all cpus(*){ALLOCATION_SLACK}:100 resources.
3) unreserve 30 cpus.
4) The resources would become:
Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:100 NOTE: We do not 
decrease the total allocation slack here.
Free: cpus(*):30;cpus(r1):70
Used: cpus(*){ALLOCATION_SLACK}:100
5) At this time, we cannot send cpus(*):30 out as offer because there are still 
30 over committed allocation slack in use.
6) recover 20 allocaiton slack, we can update the total resources as this:
Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:80 NOTE: decrease the 
total allocation slack when recover resource but not when `unreserve` resources 
in such case.
Used: cpus(*){ALLOCATION_SLACK}:80
Free: cpus(*):30;cpus(r1):70
The allocate can send out offer with cpus(*):20 but not the whole 30 resources.
7) recover another 10 allocation slack.
Total: cpus(*):30;cpus(r1):70;cpus(*){ALLOCATION_SLACK}:70 NOTE: from now on, 
the allocatioin slack is same as reserved resources.
Used: cpus(*)20;cpus(*){ALLOCATION_SLACK}:70
Free: cpus(*):10;cpus(r1):70
The allocater can send out offer with cpus(*):10.


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115497
---


On 一月 20, 2016, 6:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated 一月 20, 2016, 6:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115497
---



src/master/allocator/mesos/hierarchical.cpp (line 662)


Have you considered modifying `Resources::apply` to "create" allocation 
slack upon a reservation?  (And to "destroy" allocation slack upon unreserve?)


- Joseph Wu


On Jan. 19, 2016, 10:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 19, 2016, 10:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Joseph Wu]

> On Jan. 19, 2016, 3:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > 
> >
> > I see several places where you overwrite changes from this review in 
> > the subsequent two.  Did you mean to have three reviews for the changes to 
> > `::updateAllocation`, `::updateAvailable`, and `::recoverResources`?
> 
> Guangya Liu wrote:
> There are three patches handling dynamic reservation and 
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> I have updated the description here.

It doesn't seem like you need 3 patches.

I'd recommend a couple of cleanups:

* Add a helper for all the duplicate logic you have.  Both `updateAllocation` 
and `updateAvailable` deal with reservations, in essentially the same way (per 
optimistic offers).
* Consolidate the overlapping changes into a single patch.  Just don't 
overwrite your own changes.
* Consider a different approach than subtracting allocation slack on unreserve. 
 You could perhaps, for accounting purposes, elevate a portion of the 
allocation from revocable to non-revocable.


- Joseph


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115279
---


On Jan. 19, 2016, 10:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 19, 2016, 10:38 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
> ---
> 
> Update allocation slack resources if reserve some new
> stateless reserved resources. For such case, the allocation
> slack resources will be increased.
> 
> This patch updates both `updateAvailable` and `updateAllocation`
> when reserving some new resources.
> 
> There are three patches handling dynamic reservation and oo phase 1.
> 1) https://reviews.apache.org/r/41791 Reserve new resources via 
> `updateAllocation` and `updateAvailable`. (1/3).
> 2) https://reviews.apache.org/r/42113 Unreserve resources via 
> `updateAllocation` (2/3).
> 3) https://reviews.apache.org/r/42194 Unreserve resources via 
> `updateAvailable` (3/3).
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack when dynamic reserve new resources (1/3).

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
---

(Updated 一月 20, 2016, 6:38 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 dynamic reserve new resources (1/3).


Bugs: MESOS-4145
https://issues.apache.org/jira/browse/MESOS-4145


Repository: mesos


Description (updated)
---

Update allocation slack resources if reserve some new
stateless reserved resources. For such case, the allocation
slack resources will be increased.

This patch updates both `updateAvailable` and `updateAllocation`
when reserving some new resources.

There are three patches handling dynamic reservation and oo phase 1.
1) https://reviews.apache.org/r/41791 Reserve new resources via 
`updateAllocation` and `updateAvailable`. (1/3).
2) https://reviews.apache.org/r/42113 Unreserve resources via 
`updateAllocation` (2/3).
3) https://reviews.apache.org/r/42194 Unreserve resources via `updateAvailable` 
(3/3).


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
  src/tests/hierarchical_allocator_tests.cpp 
e044f832c2c16e53e663c6ced5452649bb0dcb59 

Diff: https://reviews.apache.org/r/41791/diff/


Testing
---

make
make check


Thanks,

Guangya Liu