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 2018/06/02 05:30:31 UTC
Review Request 67423: Skipped metric for non existing paths in gc.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67423/
-----------------------------------------------------------
Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
Repository: mesos
Description
-------
Previously, agent gc would increment the "failed" counter if the path
does not exist, but this should not be an issue. This patch skipped such
paths in both "failed" and "succeeded" counters.
Diffs
-----
src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
Diff: https://reviews.apache.org/r/67423/diff/1/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 67423: Skipped metric for non existing paths in gc.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67423/#review204218
-----------------------------------------------------------
PASS: Mesos patch 67423 was successfully built and tested.
Reviews applied: `['67264', '67423']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67423
- Mesos Reviewbot Windows
On June 2, 2018, 5:30 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67423/
> -----------------------------------------------------------
>
> (Updated June 2, 2018, 5:30 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, agent gc would increment the "failed" counter if the path
> does not exist, but this should not be an issue. This patch skipped such
> paths in both "failed" and "succeeded" counters.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
>
>
> Diff: https://reviews.apache.org/r/67423/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 67423: Skipped metric for non existing paths in gc.
Posted by Jie Yu <yu...@gmail.com>.
> On June 12, 2018, 6:17 p.m., Jie Yu wrote:
> > src/slave/gc.cpp
> > Lines 262 (patched)
> > <https://reviews.apache.org/r/67423/diff/2/?file=2035399#file2035399line262>
> >
> > This is a bit hacky. A better way should be modify `os::rmdir` to return `Try<Nothing, int>`. The second template parameter is the error code.
> >
> > Can you add a TODO?
In fact, it should return `Try<Nothing, ErrnoError>`
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67423/#review204623
-----------------------------------------------------------
On June 2, 2018, 5:30 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67423/
> -----------------------------------------------------------
>
> (Updated June 2, 2018, 5:30 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, agent gc would increment the "failed" counter if the path
> does not exist, but this should not be an issue. This patch skipped such
> paths in both "failed" and "succeeded" counters.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
>
>
> Diff: https://reviews.apache.org/r/67423/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 67423: Skipped metric for non existing paths in gc.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67423/#review204623
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/gc.cpp
Lines 262 (patched)
<https://reviews.apache.org/r/67423/#comment287207>
This is a bit hacky. A better way should be modify `os::rmdir` to return `Try<Nothing, int>`. The second template parameter is the error code.
Can you add a TODO?
- Jie Yu
On June 2, 2018, 5:30 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67423/
> -----------------------------------------------------------
>
> (Updated June 2, 2018, 5:30 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, agent gc would increment the "failed" counter if the path
> does not exist, but this should not be an issue. This patch skipped such
> paths in both "failed" and "succeeded" counters.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
>
>
> Diff: https://reviews.apache.org/r/67423/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 67423: Skipped metric for non existing paths in gc.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67423/
-----------------------------------------------------------
(Updated June 12, 2018, 1:23 p.m.)
Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
Repository: mesos
Description
-------
Previously, agent gc would increment the "failed" counter if the path
does not exist, but this should not be an issue. This patch skipped such
paths in both "failed" and "succeeded" counters.
Diffs (updated)
-----
src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
Diff: https://reviews.apache.org/r/67423/diff/3/
Changes: https://reviews.apache.org/r/67423/diff/2-3/
Testing
-------
Thanks,
Zhitao Li