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/13 03:37:41 UTC

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

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

(Updated 一月 13, 2016, 2:37 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 for dynamic reserve (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.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
  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).

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Jan. 19, 2016, 3:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line676>
> >
> >     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).

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Jan. 19, 2016, 3:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line676>
> >
> >     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> 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).

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 19, 2016, 11:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line676>
> >
> >     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 for dynamic reserve (1/3).

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 19, 2016, 11:34 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 676
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line676>
> >
> >     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`?

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.


- Guangya


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


On 一月 14, 2016, 12:44 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> -----------------------------------------------------------
> 
> (Updated 一月 14, 2016, 12:44 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.
> 
> 
> 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 for dynamic reserve (1/3).

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review115279
-----------------------------------------------------------


Is there some reason why you've split up these reviews into three pieces?  (42113 and 42194)

See below for my confusion:


src/master/allocator/mesos/hierarchical.cpp (line 676)
<https://reviews.apache.org/r/41791/#comment176234>

    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`?


- Joseph Wu


On Jan. 14, 2016, 4:44 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2016, 4:44 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.
> 
> 
> 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).

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 20, 2016, 10:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line662>
> >
> >     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).

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 20, 2016, 10:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line662>
> >
> >     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/
> -----------------------------------------------------------
> 
> (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).

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Jan. 20, 2016, 2:54 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 662
> > <https://reviews.apache.org/r/41791/diff/8/?file=1196632#file1196632line662>
> >
> >     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).

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/41791/#comment176474>

    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 called.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 三月 11, 2016, 3:01 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 dynamic reserve called.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 

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 called.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 三月 11, 2016, 9:11 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 dynamic reserve called.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 

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 called.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 一月 23, 2016, 5:40 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 called.


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


Repository: mesos


Description (updated)
-------

Updated allocation slack when dynamic reserve called.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 2d01034f43c3653f6233792ee6614fa311249e5c 
  src/master/allocator/mesos/hierarchical.cpp 65c7e6b15c5308c0910667e1b12f39b21293a316 

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).

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
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


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

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 一月 14, 2016, 12:44 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.


Diffs (updated)
-----

  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 for dynamic reserve (1/3).

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 14, 2016, 5:14 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 732-737
> > <https://reviews.apache.org/r/41791/diff/7/?file=1195638#file1195638line732>
> >
> >     Update the comments to list cases, for example:
> >     1. if new `reserved.stateless` > old, pala...
> >     2. if new ... <, pala...
> >       - if xxx
> >       - if xxx

It was handled in following patches


- Guangya


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


On 一月 13, 2016, 12:58 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> -----------------------------------------------------------
> 
> (Updated 一月 13, 2016, 12:58 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.
> 
> 
> 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 for dynamic reserve (1/3).

Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/#review114394
-----------------------------------------------------------



src/master/allocator/mesos/hierarchical.cpp (line 677)
<https://reviews.apache.org/r/41791/#comment175231>

    It should be `newStatelessReserved.contains(total)`; in `contains`, it'll return false if any resource did not contain.



src/master/allocator/mesos/hierarchical.cpp (lines 732 - 737)
<https://reviews.apache.org/r/41791/#comment175252>

    Update the comments to list cases, for example:
    1. if new `reserved.stateless` > old, pala...
    2. if new ... <, pala...
      - if xxx
      - if xxx


- Klaus Ma


On Jan. 13, 2016, 8:58 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2016, 8:58 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.
> 
> 
> 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 for dynamic reserve (1/3).

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 一月 13, 2016, 12:58 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.


Diffs (updated)
-----

  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 for dynamic reserve (1/3).

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41791/
-----------------------------------------------------------

(Updated 一月 13, 2016, 2:53 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.


Diffs (updated)
-----

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

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


Testing
-------

make
make check


Thanks,

Guangya Liu