You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jay Guo <gu...@gmail.com> on 2017/02/22 17:49:44 UTC
Review Request 56938: Added allocation role to tasks/executors in v0
/state API.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56938/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Bugs: MESOS-7158
https://issues.apache.org/jira/browse/MESOS-7158
Repository: mesos
Description
-------
Added allocation role of tasks/executors to the /state v0 API. Unlike
v1 API where allocation_info is nested in each one of resources, we
add that information at task/executor level for v0 API for the sake
of simplicity. Note that resources of one task/executor are guaranteed
to have same allocation role.
Diffs
-----
src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e
src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1
Diff: https://reviews.apache.org/r/56938/diff/
Testing
-------
make check
Thanks,
Jay Guo
Re: Review Request 56938: Added allocation role to tasks/executors in
v0 /state API.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56938/#review166402
-----------------------------------------------------------
This needs to update the following as well:
https://github.com/apache/mesos/blob/807b2343fac186d4fe5f42362c07a729fcaad80f/src/slave/http.cpp#L123-L139
src/common/http.cpp (lines 562 - 567)
<https://reviews.apache.org/r/56938/#comment238322>
A comment on this condition would be good. When is an executor with 0 resources allowed? Is this possible for any executor, or just the command executor?
- Benjamin Mahler
On Feb. 22, 2017, 5:49 p.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56938/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 5:49 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7158
> https://issues.apache.org/jira/browse/MESOS-7158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocation role of tasks/executors to the /state v0 API. Unlike
> v1 API where allocation_info is nested in each one of resources, we
> add that information at task/executor level for v0 API for the sake
> of simplicity. Note that resources of one task/executor are guaranteed
> to have same allocation role.
>
>
> Diffs
> -----
>
> src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e
> src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1
>
> Diff: https://reviews.apache.org/r/56938/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56938: Added allocation role to tasks/executors in
v0 /state API.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56938/#review166577
-----------------------------------------------------------
Fix it, then Ship it!
Looks good, thanks! Can you follow up with changes to the `MasterTest.StateEndpoint` test?
I ran the test and it fails because the executor has no resources. So I've updated the test to pass by adding resources to the executor. I also checked the task's role.
src/common/http.cpp (line 652)
<https://reviews.apache.org/r/56938/#comment238566>
Newline here.
src/master/http.cpp (line 275)
<https://reviews.apache.org/r/56938/#comment238567>
Newline here as well.
src/slave/http.cpp (line 129)
<https://reviews.apache.org/r/56938/#comment238568>
Newline here.
src/tests/slave_tests.cpp (lines 1639 - 1652)
<https://reviews.apache.org/r/56938/#comment238557>
I think we could also check the task's role here? I will add that prior to committing.
In addition, can you follow up with tests for checking the task/executor role the master's state endpoints? i.e. the `MasterTest.StateEndpoint` test, looks like it needs to be updated to launch a task, as done in the `SlaveTest.StateEndpoint` test.
- Benjamin Mahler
On Feb. 23, 2017, 4:49 p.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56938/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2017, 4:49 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7158
> https://issues.apache.org/jira/browse/MESOS-7158
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocation role of tasks/executors to the /state v0 API. Unlike
> v1 API where allocation_info is nested in each one of resources, we
> add that information at task/executor level for v0 API for the sake
> of simplicity. Note that resources of one task/executor are guaranteed
> to have same allocation role.
>
>
> Diffs
> -----
>
> src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e
> src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1
> src/slave/http.cpp 8a9fabf861369d3ae659dce21fa3932f6f7b9161
> src/tests/slave_tests.cpp 61767b155b7df3bbcf413130a36af0d76dfa1553
>
> Diff: https://reviews.apache.org/r/56938/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56938: Added allocation role to tasks/executors in
v0 /state API.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56938/
-----------------------------------------------------------
(Updated Feb. 24, 2017, 12:49 a.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Changes
-------
address bmahler's comments.
Bugs: MESOS-7158
https://issues.apache.org/jira/browse/MESOS-7158
Repository: mesos
Description
-------
Added allocation role of tasks/executors to the /state v0 API. Unlike
v1 API where allocation_info is nested in each one of resources, we
add that information at task/executor level for v0 API for the sake
of simplicity. Note that resources of one task/executor are guaranteed
to have same allocation role.
Diffs (updated)
-----
src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e
src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1
src/slave/http.cpp 8a9fabf861369d3ae659dce21fa3932f6f7b9161
src/tests/slave_tests.cpp 61767b155b7df3bbcf413130a36af0d76dfa1553
Diff: https://reviews.apache.org/r/56938/diff/
Testing
-------
make check
Thanks,
Jay Guo