You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2016/04/12 07:03:05 UTC
Re: Review Request 45932: Adds task information to container resource
usage information.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45932/
-----------------------------------------------------------
(Updated April 12, 2016, 5:03 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Update summary.
Summary (updated)
-----------------
Adds task information to container resource usage information.
Bugs: MESOS-5030
https://issues.apache.org/jira/browse/MESOS-5030
Repository: mesos
Description
-------
Add stripped TaskInfo's to ResourceUsage.Executor message.
Diffs
-----
CHANGELOG 1f0527e86e333970f7f7879bb2bcbc33f0f44bc3
include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861
include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f
src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
src/tests/oversubscription_tests.cpp 23671746da2ac505d75bc2bd59114697d9161d52
Diff: https://reviews.apache.org/r/45932/diff/
Testing
-------
Added new test to verify ResourceUsage sees task labels.
Thanks,
Zhitao Li
Re: Review Request 45932: Adds task information to container resource
usage information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45932/#review128567
-----------------------------------------------------------
Fix it, then Ship it!
Just some minor tweaks, I'll take care of these for you and commit shortly. Thanks!
include/mesos/v1/mesos.proto (lines 1024 - 1038)
<https://reviews.apache.org/r/45932/#comment191999>
This has drifted from the non-v1 proto. The task_state should be removed?
src/slave/slave.cpp (line 5175)
<https://reviews.apache.org/r/45932/#comment192004>
Since it's optional, we should probably guard this:
```
if (task->has_labels()) {
t->mutable_labels()->CopyFrom(task->labels());
}
```
src/tests/oversubscription_tests.cpp (lines 230 - 233)
<https://reviews.apache.org/r/45932/#comment192010>
How about key1,value1 and key2,value2? I'm not sure if there's any value in having task / executor within the names.
- Ben Mahler
On April 12, 2016, 5:03 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45932/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 5:03 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-5030
> https://issues.apache.org/jira/browse/MESOS-5030
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds task information to container resource usage information.
>
>
> Diffs
> -----
>
> CHANGELOG 1f0527e86e333970f7f7879bb2bcbc33f0f44bc3
> include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861
> include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/oversubscription_tests.cpp 23671746da2ac505d75bc2bd59114697d9161d52
>
> Diff: https://reviews.apache.org/r/45932/diff/
>
>
> Testing
> -------
>
> Added new test to verify ResourceUsage sees task labels.
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 45932: Adds task information to container resource
usage information.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45932/#review128340
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45572, 45932]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 12, 2016, 5:03 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45932/
> -----------------------------------------------------------
>
> (Updated April 12, 2016, 5:03 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-5030
> https://issues.apache.org/jira/browse/MESOS-5030
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds task information to container resource usage information.
>
>
> Diffs
> -----
>
> CHANGELOG 1f0527e86e333970f7f7879bb2bcbc33f0f44bc3
> include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861
> include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/oversubscription_tests.cpp 23671746da2ac505d75bc2bd59114697d9161d52
>
> Diff: https://reviews.apache.org/r/45932/diff/
>
>
> Testing
> -------
>
> Added new test to verify ResourceUsage sees task labels.
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 45932: Adds task information to container resource
usage information.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45932/
-----------------------------------------------------------
(Updated April 12, 2016, 5:03 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-5030
https://issues.apache.org/jira/browse/MESOS-5030
Repository: mesos
Description (updated)
-------
Adds task information to container resource usage information.
Diffs
-----
CHANGELOG 1f0527e86e333970f7f7879bb2bcbc33f0f44bc3
include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861
include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f
src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
src/tests/oversubscription_tests.cpp 23671746da2ac505d75bc2bd59114697d9161d52
Diff: https://reviews.apache.org/r/45932/diff/
Testing
-------
Added new test to verify ResourceUsage sees task labels.
Thanks,
Zhitao Li