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/14 17:01:57 UTC

Review Request 61620: Tracked successful task fetches rather than total.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/
-----------------------------------------------------------

Review request for mesos and Jiang Yan Xu.


Bugs: MESOS-7888
    https://issues.apache.org/jira/browse/MESOS-7888


Repository: mesos


Description
-------

In the fetcher task metrics, track the number of times we
successfully fetch all the task URIs rather than the total
number of times we try to fetch task URIs. The success count
is easier to observe and explain, and is consistent with the
metrics nomenclature for other pending metrics.


Diffs
-----

  docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
  src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
  src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
  src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 


Diff: https://reviews.apache.org/r/61620/diff/1/


Testing
-------

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 61620: Tracked successful task fetches rather than total.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/#review183375
-----------------------------------------------------------



Patch looks great!

Reviews applied: [61620]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 21, 2017, 12:10 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61620/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7888
>     https://issues.apache.org/jira/browse/MESOS-7888
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the fetcher task metrics, track the number of times we
> successfully fetch all the task URIs rather than the total
> number of times we try to fetch task URIs. The success count
> is easier to observe and explain, and is consistent with the
> metrics nomenclature for other pending metrics.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
>   src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
>   src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
>   src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 
> 
> 
> Diff: https://reviews.apache.org/r/61620/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61620: Tracked successful task fetches rather than total.

Posted by Jiang Yan Xu <ya...@jxu.me>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/#review183374
-----------------------------------------------------------


Ship it!




Ship It!

- Jiang Yan Xu


On Aug. 21, 2017, 12:10 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61620/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7888
>     https://issues.apache.org/jira/browse/MESOS-7888
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the fetcher task metrics, track the number of times we
> successfully fetch all the task URIs rather than the total
> number of times we try to fetch task URIs. The success count
> is easier to observe and explain, and is consistent with the
> metrics nomenclature for other pending metrics.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
>   src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
>   src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
>   src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 
> 
> 
> Diff: https://reviews.apache.org/r/61620/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61620: Tracked successful task fetches rather than total.

Posted by James Peach <jp...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/
-----------------------------------------------------------

(Updated Aug. 21, 2017, 7:10 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
-------

Rebased and addressed review feedback.


Bugs: MESOS-7888
    https://issues.apache.org/jira/browse/MESOS-7888


Repository: mesos


Description
-------

In the fetcher task metrics, track the number of times we
successfully fetch all the task URIs rather than the total
number of times we try to fetch task URIs. The success count
is easier to observe and explain, and is consistent with the
metrics nomenclature for other pending metrics.


Diffs (updated)
-----

  docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
  src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
  src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
  src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 


Diff: https://reviews.apache.org/r/61620/diff/2/

Changes: https://reviews.apache.org/r/61620/diff/1-2/


Testing
-------

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 61620: Tracked successful task fetches rather than total.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/#review182896
-----------------------------------------------------------



Patch looks great!

Reviews applied: [61620]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 14, 2017, 5:01 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61620/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2017, 5:01 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7888
>     https://issues.apache.org/jira/browse/MESOS-7888
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the fetcher task metrics, track the number of times we
> successfully fetch all the task URIs rather than the total
> number of times we try to fetch task URIs. The success count
> is easier to observe and explain, and is consistent with the
> metrics nomenclature for other pending metrics.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
>   src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
>   src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
>   src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 
> 
> 
> Diff: https://reviews.apache.org/r/61620/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61620: Tracked successful task fetches rather than total.

Posted by Jiang Yan Xu <ya...@jxu.me>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/#review183360
-----------------------------------------------------------




src/slave/containerizer/fetcher.cpp
Line 575 (original), 573 (patched)
<https://reviews.apache.org/r/61620/#comment259390>

    `onAny` takes a `void(const Future<T>&)` callback so this looks odd, even thought it probably compiles due to the defer wrapper.
    
    Since this has a `then` continuation below, can we just put it there?


- Jiang Yan Xu


On Aug. 14, 2017, 10:01 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61620/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2017, 10:01 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7888
>     https://issues.apache.org/jira/browse/MESOS-7888
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the fetcher task metrics, track the number of times we
> successfully fetch all the task URIs rather than the total
> number of times we try to fetch task URIs. The success count
> is easier to observe and explain, and is consistent with the
> metrics nomenclature for other pending metrics.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
>   src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
>   src/slave/containerizer/fetcher_process.hpp 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
>   src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 
> 
> 
> Diff: https://reviews.apache.org/r/61620/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>