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 2015/12/30 08:01:41 UTC

Re: Review Request 41334: WIP: Added helper functions to filter allocation slack resources.

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

(Updated 十二月 30, 2015, 7:01 a.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-----------------

WIP: Added helper functions to filter allocation slack resources.


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


Repository: mesos


Description
-------

Added helper functions to filter allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On 一月 7, 2016, 7:04 p.m., Joseph Wu wrote:
> > include/mesos/resources.hpp, line 166
> > <https://reviews.apache.org/r/41334/diff/7/?file=1185995#file1185995line166>
> >
> >     s/ revocable/, resources reserved but not allocated/

Done


> On 一月 7, 2016, 7:04 p.m., Joseph Wu wrote:
> > include/mesos/resources.hpp, lines 167-168
> > <https://reviews.apache.org/r/41334/diff/7/?file=1185995#file1185995line167>
> >
> >     Can you make it clear that this "role" is for the slack, rather than the original role the resource is reserved for?

Done


- Guangya


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


On 一月 7, 2016, 7:40 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated 一月 7, 2016, 7:40 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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



include/mesos/resources.hpp (line 166)
<https://reviews.apache.org/r/41334/#comment173818>

    s/ revocable/, resources reserved but not allocated/



include/mesos/resources.hpp (lines 167 - 168)
<https://reviews.apache.org/r/41334/#comment173820>

    Can you make it clear that this "role" is for the slack, rather than the original role the resource is reserved for?


- Joseph Wu


On Jan. 6, 2016, 11:40 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 11:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

Ship it!


Much easier to read now :)


include/mesos/resources.hpp (lines 166 - 167)
<https://reviews.apache.org/r/41334/#comment176156>

    s/slack resources/slack, resources/
    s/which is/that are/
    
    Ditto for `allocationSlack()` and for v1.


- Joseph Wu


On Jan. 15, 2016, 7:57 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2016, 7:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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




include/mesos/resources.hpp (line 170)
<https://reviews.apache.org/r/41334/#comment186113>

    I think we should also update those helper fuction according to protobuf change, using `throttleable` instead of ALLOCATION_SLACK/USAGE_SLACK, e.g. `throttleable()`, `nonThrottleable()` and `revocable()`.


- Klaus Ma


On March 15, 2016, 10:33 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 三月 15, 2016, 2:33 p.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
  include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
  src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
  src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
  src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 

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


Testing
-------

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 三月 11, 2016, 9:06 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp 388c4f383b77be490cb2debb64a734c6d6e4a176 
  include/mesos/v1/resources.hpp 64ad8bf08230aeaa173325364c91b765f091210e 
  src/common/resources.cpp cf0707209143084d86aff5e4427846f461479c54 
  src/tests/resources_tests.cpp 5357275833383bbff055689c8c1d0fa59791f2d0 
  src/v1/resources.cpp 296c4c2392384a2fcc4f2c1843980ff97e71114d 

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


Testing
-------

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 20, 2016, 2:51 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
-------

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 16, 2016, 3:57 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


Changes
-------

Removed "role" from allocation slack helper.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing (updated)
-------

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On 一月 15, 2016, 2:34 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 715-719
> > <https://reviews.apache.org/r/41334/diff/8/?file=1186623#file1186623line715>
> >
> >     I'm going to revive this discussion because:
> >     
> >     1) This helper has essentially the opposite semantics of other helpers that have an `Option<string>& role` argument.  And it's not obvious why.
> >     2) This is inconsistent with `Resources::isUsageSlack` added in the next review.  Why do some revocable resources get filtered by roles and other not?
> >     3) General question: Why is it so important to filter out one role?  Does it greatly simplify your implementation?  Or will you end up added lots of conditionals to deal with this case?
> 
> Guangya Liu wrote:
>     Actually, this is because we have one open issue for this: Do we need to offer the allocation slack to the role who offered those allocation slack resources, from the previous discussion, seems we can as the current role may also want to run some revocable tasks, so I think that we can remove the `role` parameter from here, comments?
>     
>     A case is as this:
>     agent1: cpus(r1):100;mem(r1):100;cpus(*){ALLOCATION_SLACK}:100;mem(*){ALLOCATION_SLACK}:100
>     
>     so when the offer from  agent1 send to framework with role `r1`, it will get all resources including both reserved resources and allocation slack resources, then the framework can select and decline resources back to mesos master based on task requirement.

@Joseph, I'm removing the role paramter from the helper function as all of the roles should have same view of allocation slack.


- Guangya


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


On 一月 16, 2016, 3:57 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated 一月 16, 2016, 3:57 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On 一月 15, 2016, 2:34 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 715-719
> > <https://reviews.apache.org/r/41334/diff/8/?file=1186623#file1186623line715>
> >
> >     I'm going to revive this discussion because:
> >     
> >     1) This helper has essentially the opposite semantics of other helpers that have an `Option<string>& role` argument.  And it's not obvious why.
> >     2) This is inconsistent with `Resources::isUsageSlack` added in the next review.  Why do some revocable resources get filtered by roles and other not?
> >     3) General question: Why is it so important to filter out one role?  Does it greatly simplify your implementation?  Or will you end up added lots of conditionals to deal with this case?

Actually, this is because we have one open issue for this: Do we need to offer the allocation slack to the role who offered those allocation slack resources, from the previous discussion, seems we can as the current role may also want to run some revocable tasks, so I think that we can remove the `role` parameter from here, comments?

A case is as this:
agent1: cpus(r1):100;mem(r1):100;cpus(*){ALLOCATION_SLACK}:100;mem(*){ALLOCATION_SLACK}:100

so when the offer from  agent1 send to framework with role `r1`, it will get all resources including both reserved resources and allocation slack resources, then the framework can select and decline resources back to mesos master based on task requirement.


- Guangya


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


On 一月 8, 2016, 1:12 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated 一月 8, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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



src/common/resources.cpp (lines 715 - 719)
<https://reviews.apache.org/r/41334/#comment175498>

    I'm going to revive this discussion because:
    
    1) This helper has essentially the opposite semantics of other helpers that have an `Option<string>& role` argument.  And it's not obvious why.
    2) This is inconsistent with `Resources::isUsageSlack` added in the next review.  Why do some revocable resources get filtered by roles and other not?
    3) General question: Why is it so important to filter out one role?  Does it greatly simplify your implementation?  Or will you end up added lots of conditionals to deal with this case?


- Joseph Wu


On Jan. 7, 2016, 5:12 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2016, 5:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 8, 2016, 1:12 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 7, 2016, 7:40 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 7, 2016, 5:48 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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



include/mesos/v1/resources.hpp (line 164)
<https://reviews.apache.org/r/41334/#comment173563>

    Add comments on `role`.


- Klaus Ma


On Jan. 2, 2016, 9:14 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2016, 9:14 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On 一月 6, 2016, 11:32 p.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 712-723
> > <https://reviews.apache.org/r/41334/diff/5/?file=1180262#file1180262line712>
> >
> >     Suggestion:
> >     ```
> >     if (role.isSome() && role.get() != resource.role()) {
> >       return false;
> >     }
> >     
> >     return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK
> >     ```
> >     
> >     Note: `type()` has a default, so you don't need to check `has_type()`.

Thanks Joseph, seems the logic should be

if (role.isSome() && role.get() == resource.role()) {  // Exclude current role's allocation slack from itself, enable this role only use other role's allocation slack.
  return false;
}

return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK


- Guangya


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


On 一月 2, 2016, 1:14 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated 一月 2, 2016, 1:14 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On 一月 6, 2016, 11:32 p.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 712-723
> > <https://reviews.apache.org/r/41334/diff/5/?file=1180262#file1180262line712>
> >
> >     Suggestion:
> >     ```
> >     if (role.isSome() && role.get() != resource.role()) {
> >       return false;
> >     }
> >     
> >     return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK
> >     ```
> >     
> >     Note: `type()` has a default, so you don't need to check `has_type()`.
> 
> Guangya Liu wrote:
>     Thanks Joseph, seems the logic should be
>     
>     if (role.isSome() && role.get() == resource.role()) {  // Exclude current role's allocation slack from itself, enable this role only use other role's allocation slack.
>       return false;
>     }
>     
>     return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK
> 
> Joseph Wu wrote:
>     You should be able to use revocable resources from yourself.  The example of this is a meta-framework (i.e. Marathon) that borrows resources between the frameworks underneath it.
>     
>     Also, excluding the specified `role` seems to be semantically opposite of what the function reads (i.e. "is this resource allocation slack (optionally of the specified role").

Got it, so you mean that if a role has resources as: `cpu(r1):2;cpu(*){ALLOCATION_SLACK}:2` , then the role r1 can also use the allocation slack resources from itself: the resources `cpu(*){ALLOCATION_SLACK}:2`, right?  This is a question in our work group: Does one role need to exclude allocation slack for its reserved resources? The answer should be "No" as one role can still use its own revocable resources as your proposal, right?

Also want to know more about meta-framework Marathon, AFAIK, when I deploy frameworks via Marathon, I can still specify roles for those frameworks, but those starting those frameworks will use resources from Marathon, the tasks on the meta frameworks will still use resources from its own role? So how does the meta-framework leverage: use revocable resources from itself?

The reason that I want to add a parameter of role to `bool Resources::isAllocationSlack` is because I want to add role as filter to exclude resources from itself which can give an option if some customized allocators want to exclude the allocation slack for its own role. An example would be: `cpus(r1):2;cpus(r2):3`, then the allocation slack for `r1` will be `cpus(*){ALLOCATION_SLACK}:3` if `r1` want filter out its own allocation slack, otherwise, `r1`'s allocation slack would be `cpus(*){ALLOCATION_SLACK}:5` if do not want to filter out its own allocation slack. 

My current implementaion in allocator is not excluding one roles's own allocation slack. Will update the comments to clarify this.


- Guangya


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


On 一月 7, 2016, 7:40 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated 一月 7, 2016, 7:40 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

> On Jan. 6, 2016, 3:32 p.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 712-723
> > <https://reviews.apache.org/r/41334/diff/5/?file=1180262#file1180262line712>
> >
> >     Suggestion:
> >     ```
> >     if (role.isSome() && role.get() != resource.role()) {
> >       return false;
> >     }
> >     
> >     return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK
> >     ```
> >     
> >     Note: `type()` has a default, so you don't need to check `has_type()`.
> 
> Guangya Liu wrote:
>     Thanks Joseph, seems the logic should be
>     
>     if (role.isSome() && role.get() == resource.role()) {  // Exclude current role's allocation slack from itself, enable this role only use other role's allocation slack.
>       return false;
>     }
>     
>     return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK

You should be able to use revocable resources from yourself.  The example of this is a meta-framework (i.e. Marathon) that borrows resources between the frameworks underneath it.

Also, excluding the specified `role` seems to be semantically opposite of what the function reads (i.e. "is this resource allocation slack (optionally of the specified role").


- Joseph


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


On Jan. 6, 2016, 11:40 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 11:40 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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


Looks good.  Just address the lack-of-comment as Klaus pointed out.


src/common/resources.cpp (lines 712 - 723)
<https://reviews.apache.org/r/41334/#comment173607>

    Suggestion:
    ```
    if (role.isSome() && role.get() != resource.role()) {
      return false;
    }
    
    return resource.revocable().type() == Resource::RevocableInfo::ALLOCATION_SLACK
    ```
    
    Note: `type()` has a default, so you don't need to check `has_type()`.


- Joseph Wu


On Jan. 1, 2016, 5:14 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2016, 5:14 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
>     https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 2, 2016, 1:14 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 1, 2016, 12:51 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
-------

This helper function is used to filter out allocation slack resources.


Diffs (updated)
-----

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
-------

make
make check


Thanks,

Guangya Liu


Re: Review Request 41334: Added helper functions to filter allocation slack resources.

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

(Updated 一月 1, 2016, 12:08 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, and Klaus Ma.


Summary (updated)
-----------------

Added helper functions to filter allocation slack resources.


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


Repository: mesos


Description (updated)
-------

This helper function is used to filter out allocation slack resources.


Diffs
-----

  include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
  include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
  src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764 

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


Testing
-------

make
make check


Thanks,

Guangya Liu