mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dominic Hamon" <dha...@twopensource.com>
Subject Re: Review Request 18718: Added simple metrics API.
Date Thu, 27 Mar 2014 22:50:41 GMT


> On March 26, 2014, 10:28 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/metrics/metric.cpp, lines 26-35
> > <https://reviews.apache.org/r/18718/diff/17/?file=537651#file537651line26>
> >
> >     Is this safe?
> >     
> >     Once we're inside the Parent destructor, the Child destructor has completed
(for Gauge/Metric). On another thread, we're going to try to invoke get(). Seems unsafe, no?
> 
> Dominic Hamon wrote:
>     Your concern is that we can still call get on the derived class between the derived
and parent destructor runs. You're right - we should remove as the first step in the destructor
of the derived class.
>     
>     I don't know of a good way to enforce it other than checking in the base (Metric)
destructor that the metric has been removed. I'm open to alternatives.
>
> 
> Dominic Hamon wrote:
>     There may be another option:
>     
>     The Metric is either type 'Gauge' or type 'Metric' depending on when during destruction
'get' is called from another thread. 'get' for a derived Metric is always valid to call. 'get'
on the base Metric class is a problem because it is pure virtual.
>     
>     Possible solution then:
>     
>     Metric::get returns a Future<Option<double>> instead and the implementation
in Metric returns Option::none(). Wherever we are in the destruction order, we end up returning
something valid.
>     
>     Thoughts?

take a look at the new version and let me know what you think. i think this solves the problems
we've seen with the async stuff and follows our numerous discussions.


- Dominic


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


On March 27, 2014, 3:50 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18718/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 3:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1036
>     https://issues.apache.org/jira/browse/MESOS-1036
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am e452ec6256042ce9388e59f4586e1084075fe621 
>   3rdparty/libprocess/include/process/metrics/counter.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/metrics/metric.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp PRE-CREATION 
>   3rdparty/libprocess/src/metrics/metrics.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18718/diff/
> 
> 
> Testing
> -------
> 
> Added unit tests. make check.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message