You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Dominic Hamon <dh...@twopensource.com> on 2014/04/23 19:13:16 UTC

Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

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

Review request for mesos and Vinod Kone.


Repository: mesos-git


Description
-------

see summary


Diffs
-----

  3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 

Diff: https://reviews.apache.org/r/20615/diff/


Testing
-------

build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure


Thanks,

Dominic Hamon


Re: Review Request 20615: Added explicit Metrics process initialization.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/#review41183
-----------------------------------------------------------

Ship it!


Ship It!

- Ben Mahler


On April 23, 2014, 6:30 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1235
>     https://issues.apache.org/jira/browse/MESOS-1235
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 2439526a3c81ccda2b75cb239dc885a28a3b192c 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Added explicit Metrics process initialization.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/#review41180
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On April 23, 2014, 6:30 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 6:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1235
>     https://issues.apache.org/jira/browse/MESOS-1235
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 2439526a3c81ccda2b75cb239dc885a28a3b192c 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Added explicit Metrics process initialization.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/
-----------------------------------------------------------

(Updated April 23, 2014, 11:30 a.m.)


Review request for mesos and Ben Mahler.


Changes
-------

switched to explicit initialization of metrics.


Summary (updated)
-----------------

Added explicit Metrics process initialization.


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


Repository: mesos-git


Description
-------

see summary


Diffs (updated)
-----

  3rdparty/libprocess/src/process.cpp 2439526a3c81ccda2b75cb239dc885a28a3b192c 

Diff: https://reviews.apache.org/r/20615/diff/


Testing
-------

build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure


Thanks,

Dominic Hamon


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/
-----------------------------------------------------------

(Updated April 23, 2014, 5:59 p.m.)


Review request for mesos and Ben Mahler.


Changes
-------

assigning to @bmahler to shepherd -- vinod.


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


Repository: mesos-git


Description
-------

see summary


Diffs
-----

  3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 

Diff: https://reviews.apache.org/r/20615/diff/


Testing
-------

build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure


Thanks,

Dominic Hamon


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/
-----------------------------------------------------------

(Updated April 23, 2014, 10:28 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos-git


Description
-------

see summary


Diffs (updated)
-----

  3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 

Diff: https://reviews.apache.org/r/20615/diff/


Testing
-------

build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure


Thanks,

Dominic Hamon


Re: Review Request 20615: Added explicit Metrics process initialization.

Posted by Dominic Hamon <dh...@twopensource.com>.

> On April 23, 2014, 10:21 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/tests/metrics_tests.cpp, line 140
> > <https://reviews.apache.org/r/20615/diff/1/?file=565651#file565651line140>
> >
> >     Why do we need to do this only in this test and the next one?
> >     
> >     Isn't the metrics process initialized with libprocess initialization?
> >     
> >     Have we understood what the root cause of the bug (also, please attach the bug id) is?
> 
> Dominic Hamon wrote:
>     metrics process is initialized the first time a metric is added to the system. all the other tests add a counter - this explicitly tries to get the endpoint without metrics to check it starts empty.
>     
>     
>     i couldn't find a ticket so i opened one.
> 
> Ben Mahler wrote:
>     That is indeed unfortunate, any chance we can ensure it's initialized when libprocess is initialized?
> 
> Ben Mahler wrote:
>     To be clear, I'm suggesting a follow up, not for this patch to be changed!

switched to this as it's simple.


- Dominic


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


On April 23, 2014, 11:30 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 11:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1235
>     https://issues.apache.org/jira/browse/MESOS-1235
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 2439526a3c81ccda2b75cb239dc885a28a3b192c 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Ben Mahler <be...@gmail.com>.

> On April 23, 2014, 5:21 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/tests/metrics_tests.cpp, line 140
> > <https://reviews.apache.org/r/20615/diff/1/?file=565651#file565651line140>
> >
> >     Why do we need to do this only in this test and the next one?
> >     
> >     Isn't the metrics process initialized with libprocess initialization?
> >     
> >     Have we understood what the root cause of the bug (also, please attach the bug id) is?
> 
> Dominic Hamon wrote:
>     metrics process is initialized the first time a metric is added to the system. all the other tests add a counter - this explicitly tries to get the endpoint without metrics to check it starts empty.
>     
>     
>     i couldn't find a ticket so i opened one.

That is indeed unfortunate, any chance we can ensure it's initialized when libprocess is initialized?


- Ben


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


On April 23, 2014, 5:28 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 5:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1235
>     https://issues.apache.org/jira/browse/MESOS-1235
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Dominic Hamon <dh...@twopensource.com>.

> On April 23, 2014, 10:21 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/tests/metrics_tests.cpp, line 140
> > <https://reviews.apache.org/r/20615/diff/1/?file=565651#file565651line140>
> >
> >     Why do we need to do this only in this test and the next one?
> >     
> >     Isn't the metrics process initialized with libprocess initialization?
> >     
> >     Have we understood what the root cause of the bug (also, please attach the bug id) is?

metrics process is initialized the first time a metric is added to the system. all the other tests add a counter - this explicitly tries to get the endpoint without metrics to check it starts empty.


i couldn't find a ticket so i opened one.


- Dominic


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


On April 23, 2014, 10:13 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 10:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Ben Mahler <be...@gmail.com>.

> On April 23, 2014, 5:21 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/tests/metrics_tests.cpp, line 140
> > <https://reviews.apache.org/r/20615/diff/1/?file=565651#file565651line140>
> >
> >     Why do we need to do this only in this test and the next one?
> >     
> >     Isn't the metrics process initialized with libprocess initialization?
> >     
> >     Have we understood what the root cause of the bug (also, please attach the bug id) is?
> 
> Dominic Hamon wrote:
>     metrics process is initialized the first time a metric is added to the system. all the other tests add a counter - this explicitly tries to get the endpoint without metrics to check it starts empty.
>     
>     
>     i couldn't find a ticket so i opened one.
> 
> Ben Mahler wrote:
>     That is indeed unfortunate, any chance we can ensure it's initialized when libprocess is initialized?

To be clear, I'm suggesting a follow up, not for this patch to be changed!


- Ben


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


On April 23, 2014, 5:28 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 5:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1235
>     https://issues.apache.org/jira/browse/MESOS-1235
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Re: Review Request 20615: Ensured MetricsProcess is running before trying to access endpoint in tests.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20615/#review41159
-----------------------------------------------------------



3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20615/#comment74568>

    just pull this to the top of the file.



3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20615/#comment74570>

    Why do we need to do this only in this test and the next one?
    
    Isn't the metrics process initialized with libprocess initialization?
    
    Have we understood what the root cause of the bug (also, please attach the bug id) is?



3rdparty/libprocess/src/tests/metrics_tests.cpp
<https://reviews.apache.org/r/20615/#comment74569>

    kill it.


- Vinod Kone


On April 23, 2014, 5:13 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20615/
> -----------------------------------------------------------
> 
> (Updated April 23, 2014, 5:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 6f3795b33b92dd0621867e274649f8b9667ba538 
> 
> Diff: https://reviews.apache.org/r/20615/diff/
> 
> 
> Testing
> -------
> 
> build/3rdparty/libprocess/tests --gtest_filter='Metrics.Snapshot*' --gtest_repeat=-1 --gtest_shuffle --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>