You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Yong Qiao Wang <yq...@cn.ibm.com> on 2015/12/07 09:41:29 UTC
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/
-----------------------------------------------------------
(Updated Dec. 7, 2015, 8:41 a.m.)
Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
Bugs: MESOS-3944
https://issues.apache.org/jira/browse/MESOS-3944
Repository: mesos
Description
-------
Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
Diffs (updated)
-----
include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
include/mesos/role/role.hpp PRE-CREATION
include/mesos/role/role.proto PRE-CREATION
src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
Diff: https://reviews.apache.org/r/40431/diff/
Testing
-------
1. Make Check successfully;
2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
Thanks,
Yong Qiao Wang
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Adam B <ad...@mesosphere.io>.
> On Dec. 10, 2015, 2:40 a.m., Adam B wrote:
> > include/mesos/master/allocator.proto, line 19
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155772#file1155772line19>
> >
> > Shouldn't this file have `java_package` and `java_outer_classname` just like the other protos?
> > Looks like isolator.proto and oversubscription.proto are missing it too. Would you mind creating a separate patch to fix that?
>
> Yongqiao Wang wrote:
> I am not sure if we need to add java_package and java_outer_classname in those proto files, can you please clarify a little more about why we need to do this?
>
> Adam B wrote:
> Nevermind. That's only necessary for the scheduler/executor API protobufs, since they may need to be consumed by Java processes.
> https://developers.google.com/protocol-buffers/docs/javatutorial
> Dropping the issue.
>
> Yongqiao Wang wrote:
> So can I log another JIRA ticket to fix this issue?
Not necessary. RoleInfo, isolator.proto, and oversubscription.proto are not a part of the scheduler/executor APIs, which are the only Mesos APIs that have Java bindings and would want/need to generate Java classes from the protos. So, since these protos are only used by C++, they don't need the java options. Doesn't hurt to add them, but they won't be used.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
On Dec. 7, 2015, 9:20 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2015, 9:20 p.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/master/allocator.proto, line 19
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155772#file1155772line19>
> >
> > Shouldn't this file have `java_package` and `java_outer_classname` just like the other protos?
> > Looks like isolator.proto and oversubscription.proto are missing it too. Would you mind creating a separate patch to fix that?
>
> Yongqiao Wang wrote:
> I am not sure if we need to add java_package and java_outer_classname in those proto files, can you please clarify a little more about why we need to do this?
>
> Adam B wrote:
> Nevermind. That's only necessary for the scheduler/executor API protobufs, since they may need to be consumed by Java processes.
> https://developers.google.com/protocol-buffers/docs/javatutorial
> Dropping the issue.
So can I log another JIRA ticket to fix this issue?
> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/role/role.proto, line 21
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155774#file1155774line21>
> >
> > Why change the package? Couldn't this still be in `mesos.master`? Then you wouldn't have to change all the other files.
>
> Yongqiao Wang wrote:
> Like other feature(such as quota), I also think role manamgnet is a seprated function, so I define role protobuf in a separated package rather than define it in mesos.proto.
>
> Adam B wrote:
> Ok, I just thought you could reduce code churn by keeping the package name the same, even if you create a new proto file.
> This 'role' proto package is still only useful on the master, right? mesos.scheduler.role and mesos.agent.role would have different messages/fields.
> But this is moot after the implicit roles changes in https://reviews.apache.org/r/41075/ which removes the original RoleInfo.
OK, we can defer this patch until implicit roles chagnes finalize.
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
On Dec. 8, 2015, 5:20 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2015, 5:20 a.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > I'm not sure exactly why you needed to move RoleInfo out of allocator.proto. The RoleInfo we use for the allocator module API doesn't need to be (and perhaps shouldn't be) the same class that we use to display role information like weights in the HTTP endpoint, or even the same class that we persist in the registry. There may be some future role metadata we want to set in the HTTP endpoint that doesn't need to be passed on to the allocator (e.g. Role.description), and maybe even something we set that doesn't need to be persisted. I don't actually think we need 3 or 4 separate RoleInfo-like protobufs, but I want us to think about how each of these APIs could grow apart in their notion of "important role metadata". Is there a real need to move the RoleInfo protobuf?
Currently, RoleInfo protobuf never be used for serialization, so I think we can remove it from allocator.proto, and define a struct in mesos.hpp to communicate between the allocator and master. Then for role information display, then current serialization way(call modle(role*) in http.cpp) is not better, and we should get RoleInfo protobuf back for serialization. Refer to other components(such as quota), I propose to define role protobuf in a separated package rather than define it in mesos.proto. And for persist, we should define proto message in registry.proto which only contians the metadata should be persisted.
- Yong Qiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
On Dec. 8, 2015, 5:20 a.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2015, 5:20 a.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Adam B <ad...@mesosphere.io>.
> On Dec. 10, 2015, 2:40 a.m., Adam B wrote:
> > include/mesos/master/allocator.proto, line 19
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155772#file1155772line19>
> >
> > Shouldn't this file have `java_package` and `java_outer_classname` just like the other protos?
> > Looks like isolator.proto and oversubscription.proto are missing it too. Would you mind creating a separate patch to fix that?
>
> Yongqiao Wang wrote:
> I am not sure if we need to add java_package and java_outer_classname in those proto files, can you please clarify a little more about why we need to do this?
Nevermind. That's only necessary for the scheduler/executor API protobufs, since they may need to be consumed by Java processes.
https://developers.google.com/protocol-buffers/docs/javatutorial
Dropping the issue.
> On Dec. 10, 2015, 2:40 a.m., Adam B wrote:
> > include/mesos/role/role.proto, line 21
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155774#file1155774line21>
> >
> > Why change the package? Couldn't this still be in `mesos.master`? Then you wouldn't have to change all the other files.
>
> Yongqiao Wang wrote:
> Like other feature(such as quota), I also think role manamgnet is a seprated function, so I define role protobuf in a separated package rather than define it in mesos.proto.
Ok, I just thought you could reduce code churn by keeping the package name the same, even if you create a new proto file.
This 'role' proto package is still only useful on the master, right? mesos.scheduler.role and mesos.agent.role would have different messages/fields.
But this is moot after the implicit roles changes in https://reviews.apache.org/r/41075/ which removes the original RoleInfo.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
On Dec. 7, 2015, 9:20 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2015, 9:20 p.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/master/allocator.proto, line 19
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155772#file1155772line19>
> >
> > Shouldn't this file have `java_package` and `java_outer_classname` just like the other protos?
> > Looks like isolator.proto and oversubscription.proto are missing it too. Would you mind creating a separate patch to fix that?
I am not sure if we need to add java_package and java_outer_classname in those proto files, can you please clarify a little more about why we need to do this?
> On Dec. 10, 2015, 10:40 a.m., Adam B wrote:
> > include/mesos/role/role.proto, line 21
> > <https://reviews.apache.org/r/40431/diff/17/?file=1155774#file1155774line21>
> >
> > Why change the package? Couldn't this still be in `mesos.master`? Then you wouldn't have to change all the other files.
Like other feature(such as quota), I also think role manamgnet is a seprated function, so I define role protobuf in a separated package rather than define it in mesos.proto.
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
On Dec. 8, 2015, 5:20 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2015, 5:20 a.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/#review109704
-----------------------------------------------------------
I'm not sure exactly why you needed to move RoleInfo out of allocator.proto. The RoleInfo we use for the allocator module API doesn't need to be (and perhaps shouldn't be) the same class that we use to display role information like weights in the HTTP endpoint, or even the same class that we persist in the registry. There may be some future role metadata we want to set in the HTTP endpoint that doesn't need to be passed on to the allocator (e.g. Role.description), and maybe even something we set that doesn't need to be persisted. I don't actually think we need 3 or 4 separate RoleInfo-like protobufs, but I want us to think about how each of these APIs could grow apart in their notion of "important role metadata". Is there a real need to move the RoleInfo protobuf?
include/mesos/master/allocator.proto (line 19)
<https://reviews.apache.org/r/40431/#comment169315>
Shouldn't this file have `java_package` and `java_outer_classname` just like the other protos?
Looks like isolator.proto and oversubscription.proto are missing it too. Would you mind creating a separate patch to fix that?
include/mesos/role/role.proto (line 21)
<https://reviews.apache.org/r/40431/#comment169316>
Why change the package? Couldn't this still be in `mesos.master`? Then you wouldn't have to change all the other files.
- Adam B
On Dec. 7, 2015, 9:20 p.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40431/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2015, 9:20 p.m.)
>
>
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
>
>
> Bugs: MESOS-3944
> https://issues.apache.org/jira/browse/MESOS-3944
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
>
>
> Diffs
> -----
>
> include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
> include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
> include/mesos/role/role.hpp PRE-CREATION
> include/mesos/role/role.proto PRE-CREATION
> src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
> src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
> src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
> src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
> src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
> src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
> src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
>
> Diff: https://reviews.apache.org/r/40431/diff/
>
>
> Testing
> -------
>
> 1. Make Check successfully;
>
> 2. $ curl http://9.110.48.168:5050/roles
> {"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/
-----------------------------------------------------------
(Updated Dec. 8, 2015, 5:20 a.m.)
Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
Bugs: MESOS-3944
https://issues.apache.org/jira/browse/MESOS-3944
Repository: mesos
Description
-------
Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
Diffs (updated)
-----
include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
include/mesos/role/role.hpp PRE-CREATION
include/mesos/role/role.proto PRE-CREATION
src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
Diff: https://reviews.apache.org/r/40431/diff/
Testing
-------
1. Make Check successfully;
2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
Thanks,
Yong Qiao Wang
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/
-----------------------------------------------------------
(Updated Dec. 8, 2015, 5:15 a.m.)
Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
Bugs: MESOS-3944
https://issues.apache.org/jira/browse/MESOS-3944
Repository: mesos
Description
-------
Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
Diffs (updated)
-----
include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
include/mesos/role/role.hpp PRE-CREATION
include/mesos/role/role.proto PRE-CREATION
src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
Diff: https://reviews.apache.org/r/40431/diff/
Testing
-------
1. Make Check successfully;
2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
Thanks,
Yong Qiao Wang
Re: Review Request 40431: Move RoleInfo message out of allocator.proto
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40431/
-----------------------------------------------------------
(Updated Dec. 7, 2015, 10:12 a.m.)
Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
Bugs: MESOS-3944
https://issues.apache.org/jira/browse/MESOS-3944
Repository: mesos
Description
-------
Currently role protobuf is defined in allocator.proto due to only the traditional DRF allocator uses roles as it’s first level of hierarchy, I think we should move it out and define it in a separated file as quota had in dynamic roles project, because role protobuf will also be used by master to persist.
Diffs (updated)
-----
include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0
include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c
include/mesos/role/role.hpp PRE-CREATION
include/mesos/role/role.proto PRE-CREATION
src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef
src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2
src/master/allocator/mesos/allocator.hpp 97ee80726ad155917811265a983258b0165d3451
src/master/allocator/mesos/hierarchical.hpp 99c742906874c30c39c159e58a65277ade3c07fd
src/master/allocator/mesos/hierarchical.cpp 5da825a1d578a9ee40b4985378fddb3c5fb3b416
src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c
src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42
src/tests/hierarchical_allocator_tests.cpp fb214a829a57529d3f5c49730ae9733f53e622ca
Diff: https://reviews.apache.org/r/40431/diff/
Testing
-------
1. Make Check successfully;
2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}
Thanks,
Yong Qiao Wang