You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/09/28 00:19:48 UTC

Review Request 62644: Added resources to the TaskStatus message.

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

Review request for mesos, Jie Yu and Qian Zhang.


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


Repository: mesos


Description
-------

Added a Resources field names `limited_resource` to the `TaskStatus`
message to convey specific information about a resource limit that has
been violated by a container.

This field propagates the resources in the `ContainerLimitation` message
send by isolators on the agent, and is populated for the following
reason enums:

  * `REASON_CONTAINER_LIMITATION`
  * `REASON_CONTAINER_LIMITATION_DISK`
  * `REASON_CONTAINER_LIMITATION_MEMORY`


Diffs
-----

  include/mesos/mesos.proto 3b2d6bb7034c3157347c8ad3c32aa38c642762f8 
  include/mesos/v1/mesos.proto 4ca48868c033c7285662d7eb182ac3d40f550833 
  src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 


Diff: https://reviews.apache.org/r/62644/diff/1/


Testing
-------

make check (Fedora 26)


Thanks,

James Peach


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62644/#review188078
-----------------------------------------------------------


Ship it!




Ship It!

- Qian Zhang


On Oct. 10, 2017, 8:08 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62644/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2017, 8:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7963
>     https://issues.apache.org/jira/browse/MESOS-7963
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a TaskResourcesLimitation field to the `TaskStatus`
> message to convey specific information about a resource
> limit that has been violated by a container.
> 
> This field propagates the resources from the `ContainerLimitation`
> message sent by isolators on the agent, and is populated for the
> following reasons:
> 
>   * `REASON_CONTAINER_LIMITATION`
>   * `REASON_CONTAINER_LIMITATION_DISK`
>   * `REASON_CONTAINER_LIMITATION_MEMORY`
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 830985a3265b7c104d8fdc50749c395d98f5f3c8 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
>   src/slave/slave.cpp 2e05637ed10a39eb3f4ce953506b1cb5d50caf3c 
> 
> 
> Diff: https://reviews.apache.org/r/62644/diff/3/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by James Peach <jp...@apache.org>.

> On Oct. 11, 2017, 12:48 p.m., Qian Zhang wrote:
> > src/slave/slave.cpp
> > Lines 7043-7044 (patched)
> > <https://reviews.apache.org/r/62644/diff/2/?file=1850024#file1850024line7045>
> >
> >     These two lines can be merged into a single line.

Yes but if you do that, the indentation of the `UPID()` parameter becomes weird and harder to read.


- James


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


On Oct. 10, 2017, 12:08 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62644/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2017, 12:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7963
>     https://issues.apache.org/jira/browse/MESOS-7963
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a TaskResourcesLimitation field to the `TaskStatus`
> message to convey specific information about a resource
> limit that has been violated by a container.
> 
> This field propagates the resources from the `ContainerLimitation`
> message sent by isolators on the agent, and is populated for the
> following reasons:
> 
>   * `REASON_CONTAINER_LIMITATION`
>   * `REASON_CONTAINER_LIMITATION_DISK`
>   * `REASON_CONTAINER_LIMITATION_MEMORY`
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 1bfcc5bb9091075c6fa1bc36e2cd23afe2e0e0ec 
>   include/mesos/v1/mesos.proto d742adbbf24692d2d82aad51316d767af93942d5 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
>   src/slave/slave.cpp 2e05637ed10a39eb3f4ce953506b1cb5d50caf3c 
> 
> 
> Diff: https://reviews.apache.org/r/62644/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62644/#review187650
-----------------------------------------------------------




include/mesos/mesos.proto
Lines 2177 (patched)
<https://reviews.apache.org/r/62644/#comment264706>

    Kill this blank line.



src/slave/slave.cpp
Lines 7043-7044 (patched)
<https://reviews.apache.org/r/62644/#comment264707>

    These two lines can be merged into a single line.


- Qian Zhang


On Oct. 10, 2017, 8:08 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62644/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2017, 8:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7963
>     https://issues.apache.org/jira/browse/MESOS-7963
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a TaskResourcesLimitation field to the `TaskStatus`
> message to convey specific information about a resource
> limit that has been violated by a container.
> 
> This field propagates the resources from the `ContainerLimitation`
> message sent by isolators on the agent, and is populated for the
> following reasons:
> 
>   * `REASON_CONTAINER_LIMITATION`
>   * `REASON_CONTAINER_LIMITATION_DISK`
>   * `REASON_CONTAINER_LIMITATION_MEMORY`
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 1bfcc5bb9091075c6fa1bc36e2cd23afe2e0e0ec 
>   include/mesos/v1/mesos.proto d742adbbf24692d2d82aad51316d767af93942d5 
>   src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
>   src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
>   src/slave/slave.cpp 2e05637ed10a39eb3f4ce953506b1cb5d50caf3c 
> 
> 
> Diff: https://reviews.apache.org/r/62644/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62644/
-----------------------------------------------------------

(Updated Oct. 10, 2017, 12:08 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


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


Repository: mesos


Description (updated)
-------

Added a TaskResourcesLimitation field to the `TaskStatus`
message to convey specific information about a resource
limit that has been violated by a container.

This field propagates the resources from the `ContainerLimitation`
message sent by isolators on the agent, and is populated for the
following reasons:

  * `REASON_CONTAINER_LIMITATION`
  * `REASON_CONTAINER_LIMITATION_DISK`
  * `REASON_CONTAINER_LIMITATION_MEMORY`


Diffs (updated)
-----

  include/mesos/mesos.proto 1bfcc5bb9091075c6fa1bc36e2cd23afe2e0e0ec 
  include/mesos/v1/mesos.proto d742adbbf24692d2d82aad51316d767af93942d5 
  src/common/protobuf_utils.hpp ff0fd01b7a20f597ec6cd916f0bd9c41baa5fd51 
  src/common/protobuf_utils.cpp 04f44f6f63e431c17ec67e234c8da58e7945294d 
  src/slave/slave.cpp 2e05637ed10a39eb3f4ce953506b1cb5d50caf3c 


Diff: https://reviews.apache.org/r/62644/diff/2/

Changes: https://reviews.apache.org/r/62644/diff/1-2/


Testing
-------

make check (Fedora 26)


Thanks,

James Peach


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62644/#review186968
-----------------------------------------------------------




src/slave/slave.cpp
Lines 6962-6965 (patched)
<https://reviews.apache.org/r/62644/#comment263915>

    Looks like we should update `createStatusUpdate`?


- Jie Yu


On Sept. 28, 2017, 12:19 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62644/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 12:19 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7963
>     https://issues.apache.org/jira/browse/MESOS-7963
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a Resources field names `limited_resource` to the `TaskStatus`
> message to convey specific information about a resource limit that has
> been violated by a container.
> 
> This field propagates the resources in the `ContainerLimitation` message
> send by isolators on the agent, and is populated for the following
> reason enums:
> 
>   * `REASON_CONTAINER_LIMITATION`
>   * `REASON_CONTAINER_LIMITATION_DISK`
>   * `REASON_CONTAINER_LIMITATION_MEMORY`
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 3b2d6bb7034c3157347c8ad3c32aa38c642762f8 
>   include/mesos/v1/mesos.proto 4ca48868c033c7285662d7eb182ac3d40f550833 
>   src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
> 
> 
> Diff: https://reviews.apache.org/r/62644/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 62644: Added resources to the TaskStatus message.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62644/#review186964
-----------------------------------------------------------


Ship it!




Ship It!

- Jie Yu


On Sept. 28, 2017, 12:19 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62644/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 12:19 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-7963
>     https://issues.apache.org/jira/browse/MESOS-7963
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a Resources field names `limited_resource` to the `TaskStatus`
> message to convey specific information about a resource limit that has
> been violated by a container.
> 
> This field propagates the resources in the `ContainerLimitation` message
> send by isolators on the agent, and is populated for the following
> reason enums:
> 
>   * `REASON_CONTAINER_LIMITATION`
>   * `REASON_CONTAINER_LIMITATION_DISK`
>   * `REASON_CONTAINER_LIMITATION_MEMORY`
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 3b2d6bb7034c3157347c8ad3c32aa38c642762f8 
>   include/mesos/v1/mesos.proto 4ca48868c033c7285662d7eb182ac3d40f550833 
>   src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
> 
> 
> Diff: https://reviews.apache.org/r/62644/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>