You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/06/10 21:50:30 UTC
Review Request 35313: Added slave metrics for revocable resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/
-----------------------------------------------------------
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-2775
https://issues.apache.org/jira/browse/MESOS-2775
Repository: mesos
Description
-------
Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
Note that the existing used metric was not ignoring revocable resources.
Diffs
-----
src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
Diff: https://reviews.apache.org/r/35313/diff/
Testing
-------
Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
Thanks,
Ben Mahler
Re: Review Request 35313: Added slave metrics for revocable resources.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/#review87474
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35312, 35313]
All tests passed.
- Mesos ReviewBot
On June 10, 2015, 7:50 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35313/
> -----------------------------------------------------------
>
> (Updated June 10, 2015, 7:50 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-2775
> https://issues.apache.org/jira/browse/MESOS-2775
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
>
> Note that the existing used metric was not ignoring revocable resources.
>
>
> Diffs
> -----
>
> src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
> src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
> src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
> src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
>
> Diff: https://reviews.apache.org/r/35313/diff/
>
>
> Testing
> -------
>
> Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
>
> Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35313: Added slave metrics for revocable resources.
Posted by Ben Mahler <be...@gmail.com>.
> On June 10, 2015, 9:49 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp, line 4380
> > <https://reviews.apache.org/r/35313/diff/1/?file=982250#file982250line4380>
> >
> > s/std:://
> >
> > Here and two below.
Thanks!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/#review87466
-----------------------------------------------------------
On June 10, 2015, 7:50 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35313/
> -----------------------------------------------------------
>
> (Updated June 10, 2015, 7:50 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-2775
> https://issues.apache.org/jira/browse/MESOS-2775
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
>
> Note that the existing used metric was not ignoring revocable resources.
>
>
> Diffs
> -----
>
> src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
> src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
> src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
> src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
>
> Diff: https://reviews.apache.org/r/35313/diff/
>
>
> Testing
> -------
>
> Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
>
> Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35313: Added slave metrics for revocable resources.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/#review87466
-----------------------------------------------------------
Ship it!
src/slave/slave.cpp
<https://reviews.apache.org/r/35313/#comment139784>
Same as the one Vinod pointed out.
src/slave/slave.cpp
<https://reviews.apache.org/r/35313/#comment139785>
s/std:://
Here and two below.
- Jiang Yan Xu
On June 10, 2015, 12:50 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35313/
> -----------------------------------------------------------
>
> (Updated June 10, 2015, 12:50 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-2775
> https://issues.apache.org/jira/browse/MESOS-2775
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
>
> Note that the existing used metric was not ignoring revocable resources.
>
>
> Diffs
> -----
>
> src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
> src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
> src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
> src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
>
> Diff: https://reviews.apache.org/r/35313/diff/
>
>
> Testing
> -------
>
> Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
>
> Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35313: Added slave metrics for revocable resources.
Posted by Ben Mahler <be...@gmail.com>.
> On June 10, 2015, 9:36 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 4400
> > <https://reviews.apache.org/r/35313/diff/1/?file=982250#file982250line4400>
> >
> > why the temporary?
Didn't fit in a single line for nonRevocable, so I did the same here to keep it consistent, but I'll remove the temporaries.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/#review87462
-----------------------------------------------------------
On June 10, 2015, 7:50 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35313/
> -----------------------------------------------------------
>
> (Updated June 10, 2015, 7:50 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-2775
> https://issues.apache.org/jira/browse/MESOS-2775
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
>
> Note that the existing used metric was not ignoring revocable resources.
>
>
> Diffs
> -----
>
> src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
> src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
> src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
> src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
>
> Diff: https://reviews.apache.org/r/35313/diff/
>
>
> Testing
> -------
>
> Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
>
> Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 35313: Added slave metrics for revocable resources.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35313/#review87462
-----------------------------------------------------------
Ship it!
src/slave/slave.cpp
<https://reviews.apache.org/r/35313/#comment139774>
why the temporary?
- Vinod Kone
On June 10, 2015, 7:50 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35313/
> -----------------------------------------------------------
>
> (Updated June 10, 2015, 7:50 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-2775
> https://issues.apache.org/jira/browse/MESOS-2775
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Same approach as done in [r/35119/](https://reviews.apache.org/r/35119/) for [MESOS-2776](https://issues.apache.org/jira/browse/MESOS-2776)].
>
> Note that the existing used metric was not ignoring revocable resources.
>
>
> Diffs
> -----
>
> src/slave/metrics.hpp 6af7f074d4e41225867e482241988bec3a9806e9
> src/slave/metrics.cpp 7a31ce7e32c1fd61256927cd37d84a646bf5dbda
> src/slave/slave.hpp 4d2c31688b19f101ec851c0d94e7d45aa2f8a76e
> src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac
>
> Diff: https://reviews.apache.org/r/35313/diff/
>
>
> Testing
> -------
>
> Manual testing, I will follow up to get this tested within vinod/jie's integration tests.
>
> Note that in the process of testing this, I realized that the command executor leads to a mixing of revocable / non-revocable resources in the slave. Will file a ticket.
>
>
> Thanks,
>
> Ben Mahler
>
>