Re: Review Request 67423: Skipped metric for non existing paths in gc.

[Zhitao Li]

---
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

Re: Review Request 67423: Skipped metric for non existing paths in gc.

[Jie Yu]

> On June 12, 2018, 6:17 p.m., Jie Yu wrote:
> > src/slave/gc.cpp
> > Lines 262 (patched)
> > 
> >
> > This is a bit hacky. A better way should be modify `os::rmdir` to 
> > return `Try`. The second template parameter is the error code.
> > 
> > Can you add a TODO?

In fact, it should return `Try`


- 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.

[Jie Yu]

---
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)


This is a bit hacky. A better way should be modify `os::rmdir` to return 
`Try`. 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.

[Mesos Reviewbot Windows]

---
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
> 
>