You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anindya Sinha <an...@apple.com> on 2016/02/02 01:36:20 UTC
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/
-----------------------------------------------------------
(Updated Feb. 2, 2016, 12:36 a.m.)
Review request for mesos and Adam B.
Summary (updated)
-----------------
Support sharing of resources through reference counting of resources.
Bugs: MESOS-4431
https://issues.apache.org/jira/browse/MESOS-4431
Repository: mesos
Description (updated)
-------
Added new Offer::Operation of SHARE and UNSHARE for resources.
Added ShareInfo within Resources protobuf to allow for sharing of resources
and keep track of consumers of such resources.
Shareable resources keeps track of its consumers and makes decision based on
that during task launch and task termination.
Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
Diffs (updated)
-----
include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466
include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed
include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
src/master/allocator/mesos/hierarchical.cpp 1a07d69016407e5aad2209586da37fecbcddb765
src/master/allocator/sorter/drf/sorter.cpp db47d640e36c0302d7c6254a9c58caa878feac01
src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83
src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07
src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
Diff: https://reviews.apache.org/r/42992/diff/
Testing
-------
make check done.
Thanks,
Anindya Sinha
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/
-----------------------------------------------------------
(Updated Feb. 14, 2016, 6:24 a.m.)
Review request for mesos and Adam B.
Bugs: MESOS-4431
https://issues.apache.org/jira/browse/MESOS-4431
Repository: mesos
Description (updated)
-------
Shareable resources keeps track of its consumers and makes decision based on
that during task launch and task termination.
Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
Diffs (updated)
-----
include/mesos/resources.hpp fe8a5745ea7d4943c47ac22c73db70488c6dfa9f
include/mesos/v1/resources.hpp c27927e4f0d7f45e69fe3312b2423afb64c5c51e
src/common/resources.cpp 5d731870542166cec11f9956ccdf16207b2d22cc
src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
src/master/master.hpp 2f2ad2ada508e1923bf995ab124367a3b082b572
src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
src/v1/resources.cpp 207eb61d6a6d03d314539d42751cac65fcffa9af
Diff: https://reviews.apache.org/r/42992/diff/
Testing
-------
make check done.
Thanks,
Anindya Sinha
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Anindya Sinha <an...@apple.com>.
> On Feb. 11, 2016, 3:14 p.m., Guangya Liu wrote:
> > src/common/resources.cpp, lines 202-205
> > <https://reviews.apache.org/r/42992/diff/3/?file=1236756#file1236756line202>
> >
> > Does this require that the num_consumers should be same?
No. This just checks that we are comparing 2 resources that both have share set (for shareable resources), or niether have share set (for regular non-shareable resources). It does not mandate num_consumers to be equal.
> On Feb. 11, 2016, 3:14 p.m., Guangya Liu wrote:
> > src/common/resources.cpp, lines 335-338
> > <https://reviews.apache.org/r/42992/diff/3/?file=1236756#file1236756line335>
> >
> > does this require the num_consumers be same when subtract?
No. This just checks that we are comparing 2 resources that both have share set (for shareable resources), or niether have share set (for regular non-shareable resources). It does not mandate num_consumers to be equal.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/#review118865
-----------------------------------------------------------
On Feb. 5, 2016, 10:57 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2016, 10:57 p.m.)
>
>
> Review request for mesos and Adam B.
>
>
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3
> include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
> include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116
> include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
> src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
> src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
> src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
> src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
> src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
> src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
> src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
> src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
> src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
>
> Diff: https://reviews.apache.org/r/42992/diff/
>
>
> Testing
> -------
>
> make check done.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/#review118865
-----------------------------------------------------------
src/common/resources.cpp (lines 202 - 205)
<https://reviews.apache.org/r/42992/#comment180178>
Does this require that the num_consumers should be same?
src/common/resources.cpp (lines 335 - 338)
<https://reviews.apache.org/r/42992/#comment180194>
does this require the num_consumers be same when subtract?
src/common/resources.cpp (line 729)
<https://reviews.apache.org/r/42992/#comment180176>
add a period to the end.
src/common/resources.cpp (line 734)
<https://reviews.apache.org/r/42992/#comment180177>
s/Resource type/Resoure
`Resource type` is usally specifying if the resources is `SCALAR`, `SET` or `RANGES`.
src/common/resources.cpp (lines 1653 - 1656)
<https://reviews.apache.org/r/42992/#comment180183>
Can you please add some comments to clarify the difference between `None` and `0` for `num_consumers`?
- Guangya Liu
On 二月 5, 2016, 10:57 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> -----------------------------------------------------------
>
> (Updated 二月 5, 2016, 10:57 p.m.)
>
>
> Review request for mesos and Adam B.
>
>
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3
> include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
> include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116
> include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
> src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
> src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
> src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
> src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
> src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
> src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
> src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
> src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
> src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
>
> Diff: https://reviews.apache.org/r/42992/diff/
>
>
> Testing
> -------
>
> make check done.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Anindya Sinha <an...@apple.com>.
> On Feb. 13, 2016, 12:57 a.m., Greg Mann wrote:
> > include/mesos/resources.hpp, line 222
> > <https://reviews.apache.org/r/42992/diff/3/?file=1236753#file1236753line222>
> >
> > I might recommend renaming this method, due to the ubiquitous `get()` method of the `Future` and `Option` types. Perhaps `find()` instead?
There is a method `Option<Resources> find(const Resources& targets) const;` which does different from what this function does. This method extracts the Resource object from Resources that matches the passed in Resource& by not comparing the num_consumers.
So, let us rename this to:
`Option<Resource> locate(const Resource& that) const;`
Does it sound ok to you?
> On Feb. 13, 2016, 12:57 a.m., Greg Mann wrote:
> > include/mesos/mesos.proto, lines 988-989
> > <https://reviews.apache.org/r/42992/diff/3/?file=1236752#file1236752line988>
> >
> > I'm curious if we'll really need these operations? Might it be enough to simply make a persistent volume shareable when it's created? The same might be the case for other shareable resources as well.
I think that supporting SHARE as add-on operations for resources that can be shared (but are not created as shared) is useful. Same argument for UNSHARE.
Keeping in mind that this feature is for sharing resources (not just sharing of persistent volumes), I feel that these 2 apis have a need to co-exist.
> On Feb. 13, 2016, 12:57 a.m., Greg Mann wrote:
> > include/mesos/mesos.proto, line 688
> > <https://reviews.apache.org/r/42992/diff/3/?file=1236752#file1236752line688>
> >
> > I don't think this field belongs in the protobuf. If this information doesn't need to be communicated on the wire, the master can simply track it internally, and reconstruct it upon agent reregistration after master failover.
As stated in the design doc, this has been done to make implementation simpler. Resource is used quite extensively in code and hence it was easier to embed num_consumers in it.
I agree we can keep num_consumers as a hashmap, such as:
`hashmap<Resource, uint32_t> consumers;` and expose it in a singleton or something like that such that multiple classes can have access to it, esp. the sorter, allocator and master.
However, the num_consumers vary based on its context. For example:
i) For role, it contains the total tasks within a specific role that master has assigned this resource. This is across frameworks.
ii) For framework, it contains the total tasks within a specific framework that master has assigned this resource.
iii) For slave, it contains the total tasks within a slave that master has assigned this resource (this should be same as (i) for persistent volumes).
etc...
So we would need to maintain multiple hashmaps to reflect this. Secondly, there needs to be a `hashmap::find` for every access to num_consumers which can be avoided if we embed it within Resource::ShareInfo.
Another point that has been raised is if we should reconsider and actually expose `num_consumerss` to frameworks since that would give a good idea for frameworks to know when a `DESTROY` can be issued. Note that the `DESTROY` is not guaranteed to succeed though since another framework of the same role might have issued a launch task using this resource in the mantime (and that is the reason we did not expose this in the first place). However, I think we can consider exposing this to frameworks if we feel that would provide frameworks with some meaningful information regarding possibility of DESTROY being successful. Note that the same issue exists with non-shareable persistent volumes as well, but the use case is not as bad since there can be only 1 task that can use the persistent volume.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/#review119090
-----------------------------------------------------------
On Feb. 5, 2016, 10:57 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2016, 10:57 p.m.)
>
>
> Review request for mesos and Adam B.
>
>
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3
> include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
> include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116
> include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
> src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
> src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
> src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
> src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
> src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
> src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
> src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
> src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
> src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
>
> Diff: https://reviews.apache.org/r/42992/diff/
>
>
> Testing
> -------
>
> make check done.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/#review119090
-----------------------------------------------------------
include/mesos/mesos.proto (line 688)
<https://reviews.apache.org/r/42992/#comment180360>
I don't think this field belongs in the protobuf. If this information doesn't need to be communicated on the wire, the master can simply track it internally, and reconstruct it upon agent reregistration after master failover.
include/mesos/mesos.proto (lines 988 - 989)
<https://reviews.apache.org/r/42992/#comment180361>
I'm curious if we'll really need these operations? Might it be enough to simply make a persistent volume shareable when it's created? The same might be the case for other shareable resources as well.
include/mesos/resources.hpp (line 222)
<https://reviews.apache.org/r/42992/#comment180366>
I might recommend renaming this method, due to the ubiquitous `get()` method of the `Future` and `Option` types. Perhaps `find()` instead?
- Greg Mann
On Feb. 5, 2016, 10:57 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2016, 10:57 p.m.)
>
>
> Review request for mesos and Adam B.
>
>
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3
> include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
> include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116
> include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
> src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
> src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
> src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
> src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
> src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
> src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
> src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
> src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
> src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
>
> Diff: https://reviews.apache.org/r/42992/diff/
>
>
> Testing
> -------
>
> make check done.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 42992: Support sharing of resources through
reference counting of resources.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42992/
-----------------------------------------------------------
(Updated Feb. 5, 2016, 10:57 p.m.)
Review request for mesos and Adam B.
Bugs: MESOS-4431
https://issues.apache.org/jira/browse/MESOS-4431
Repository: mesos
Description
-------
Added new Offer::Operation of SHARE and UNSHARE for resources.
Added ShareInfo within Resources protobuf to allow for sharing of resources
and keep track of consumers of such resources.
Shareable resources keeps track of its consumers and makes decision based on
that during task launch and task termination.
Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
Diffs (updated)
-----
include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3
include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1
include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116
include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af
src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13
src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2
src/master/allocator/mesos/hierarchical.cpp a9d2c23162892e22220f97d89a076d2311091d91
src/master/allocator/sorter/drf/sorter.cpp 18797e42a9c2bd20392020237cfae600a5ffe12c
src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58
src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22
src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c
src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca
src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7
Diff: https://reviews.apache.org/r/42992/diff/
Testing
-------
make check done.
Thanks,
Anindya Sinha