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 20015: Added history to metrics library.
Date Thu, 17 Apr 2014 23:16:20 GMT


> On April 17, 2014, 3:09 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/metrics/metric.hpp, lines 58-60
> > <https://reviews.apache.org/r/20015/diff/8/?file=561399#file561399line58>
> >
> >     Would it be better to use an Option<Owned<...>> instead of relying
on NULL?
> 
> Dominic Hamon wrote:
>     the comment explains why 'Option' doesn't work here. I can't get the option and then
call 'set'.
> 
> Ben Mahler wrote:
>     The comment is describing the issue with Option<TimeSeries<...>> but
I'm referring to Option<Owned<...>> which should work?
> 
> Dominic Hamon wrote:
>     Option<Owned<...>> doesn't work for exactly this reason. In push:
>     
>     data->history.get()->set(...)
>     
>     tries to call 'set' on a const Owned<...>&.
>     
>     Owned<Option<...>> won't work for the same reason, as it happens.
>     
>     I could decompose it to:
>     
>     Owned<...> history = data->history.get();
>     history->set(...);
>     data->history = history;
>     
>     but i'm nervous about the copying there...
> 
> Ben Mahler wrote:
>     I'm not sure you understood correctly:
>     
>     "data->history.get()->set(...) tries to call 'set' on a const Owned<...>&"
>     
>     This is actually trying to call 'operator ->() const' on a const Owned<...>&
(which is ok), and this returns a non-const reference of TimeSeries which you can manipulate.

It was actually the 'reset' that was failing. Changed that to an assignment instead and all
seems good.


- Dominic


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


On April 17, 2014, 4:16 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20015/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 4:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-1036
>     https://issues.apache.org/jira/browse/MESOS-1036
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp f4774ada4dbe3fa18b5a8b204f97f59ca015b3c1

>   3rdparty/libprocess/include/process/metrics/gauge.hpp 4f5c1086ac3553319431283165c5451df1a0ee3f

>   3rdparty/libprocess/include/process/metrics/metric.hpp ea64f699fd9ec38745d84c7523133709827f96db

>   3rdparty/libprocess/src/tests/metrics_tests.cpp 0cc9f4bcbbb03ac3a9a2d57f64b944443fcb94bb

> 
> Diff: https://reviews.apache.org/r/20015/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


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