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/08/10 22:00:51 UTC
Re: Review Request 61260: Added agent garbage collection metrics.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/
-----------------------------------------------------------
(Updated Aug. 10, 2017, 10 p.m.)
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-7842
https://issues.apache.org/jira/browse/MESOS-7842
Repository: mesos
Description
-------
Added some basic sandbox garbage collection metrics to track the number
of paths removals, the number of failed path removal and the number of
paths that are still scheduled for removal.
Diffs (updated)
-----
src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
src/slave/gc_process.hpp PRE-CREATION
src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
Diff: https://reviews.apache.org/r/61260/diff/2/
Changes: https://reviews.apache.org/r/61260/diff/1-2/
Testing
-------
make check (Fedora 26).
Thanks,
James Peach
Re: Review Request 61260: Added agent garbage collection metrics.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 22, 2017, 11 a.m., Jiang Yan Xu wrote:
> > src/slave/gc.cpp
> > Lines 53 (patched)
> > <https://reviews.apache.org/r/61260/diff/3/?file=1801592#file1801592line53>
> >
> > Actually, it didn't occur to me earlier but a better naming pattern would probably be:
> >
> > ```
> > gc/path_removals_succeeded
> > gc/path_removals_failed
> > gc/path_removals_pending
> > ```
> >
> > `slave/` would imply these are metrics from the `Slave` actor which is not true.
Sorry "actor" is not strictly the criteria for namespacing metrics. It should be just as what is it: a namespace, e.g., on the master there are metrics such as `frameworks/<framework_principal>/messages_processed`. Probably not worth overthinking this, this is just me suggesting shorten the name while still keep them clearly grouped/namespaced.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/#review183498
-----------------------------------------------------------
On Aug. 22, 2017, 8:51 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61260/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 8:51 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7842
> https://issues.apache.org/jira/browse/MESOS-7842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added some basic sandbox garbage collection metrics to track the number
> of paths removals, the number of failed path removal and the number of
> paths that are still scheduled for removal.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
> src/slave/gc_process.hpp PRE-CREATION
> src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
>
>
> Diff: https://reviews.apache.org/r/61260/diff/3/
>
>
> Testing
> -------
>
> make check (Fedora 26).
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 61260: Added agent garbage collection metrics.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/#review183498
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/gc.cpp
Lines 53 (patched)
<https://reviews.apache.org/r/61260/#comment259511>
Actually, it didn't occur to me earlier but a better naming pattern would probably be:
```
gc/path_removals_succeeded
gc/path_removals_failed
gc/path_removals_pending
```
`slave/` would imply these are metrics from the `Slave` actor which is not true.
- Jiang Yan Xu
On Aug. 22, 2017, 8:51 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61260/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 8:51 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7842
> https://issues.apache.org/jira/browse/MESOS-7842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added some basic sandbox garbage collection metrics to track the number
> of paths removals, the number of failed path removal and the number of
> paths that are still scheduled for removal.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
> src/slave/gc_process.hpp PRE-CREATION
> src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
>
>
> Diff: https://reviews.apache.org/r/61260/diff/3/
>
>
> Testing
> -------
>
> make check (Fedora 26).
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 61260: Added agent garbage collection metrics.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/
-----------------------------------------------------------
(Updated Aug. 22, 2017, 10:30 p.m.)
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-7842
https://issues.apache.org/jira/browse/MESOS-7842
Repository: mesos
Description
-------
Added some basic sandbox garbage collection metrics to track the number
of paths removals, the number of failed path removal and the number of
paths that are still scheduled for removal.
Diffs (updated)
-----
src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
src/slave/gc_process.hpp PRE-CREATION
src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
Diff: https://reviews.apache.org/r/61260/diff/4/
Changes: https://reviews.apache.org/r/61260/diff/3-4/
Testing
-------
make check (Fedora 26).
Thanks,
James Peach
Re: Review Request 61260: Added agent garbage collection metrics.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/
-----------------------------------------------------------
(Updated Aug. 22, 2017, 3:51 p.m.)
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-7842
https://issues.apache.org/jira/browse/MESOS-7842
Repository: mesos
Description
-------
Added some basic sandbox garbage collection metrics to track the number
of paths removals, the number of failed path removal and the number of
paths that are still scheduled for removal.
Diffs (updated)
-----
src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
src/slave/gc_process.hpp PRE-CREATION
src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
Diff: https://reviews.apache.org/r/61260/diff/3/
Changes: https://reviews.apache.org/r/61260/diff/2-3/
Testing
-------
make check (Fedora 26).
Thanks,
James Peach
Re: Review Request 61260: Added agent garbage collection metrics.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61260/#review183385
-----------------------------------------------------------
Can we cover the failure case in the tests? e.g., `GarbageCollectorIntegrationTest.ROOT_BusyMountPoint`?
src/slave/gc.cpp
Lines 72 (patched)
<https://reviews.apache.org/r/61260/#comment259409>
Can we add a simple comment on why `await()` is used? e.g.,
```
// Make sure the gauge is not accessed after the destruction of `gc`.
```
src/slave/gc.cpp
Lines 196-198 (patched)
<https://reviews.apache.org/r/61260/#comment259408>
IIRC from your slack chat with @mpark this (unable to use `mutable`) is a regression and it should be fixed soon? If so can we make sure a JIRA is filed and a TODO that references it is added here?
- Jiang Yan Xu
On Aug. 10, 2017, 3 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61260/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2017, 3 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7842
> https://issues.apache.org/jira/browse/MESOS-7842
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added some basic sandbox garbage collection metrics to track the number
> of paths removals, the number of failed path removal and the number of
> paths that are still scheduled for removal.
>
>
> Diffs
> -----
>
> src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271
> src/slave/gc_process.hpp PRE-CREATION
> src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535
>
>
> Diff: https://reviews.apache.org/r/61260/diff/2/
>
>
> Testing
> -------
>
> make check (Fedora 26).
>
>
> Thanks,
>
> James Peach
>
>