mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 59858: Allow Gauge metrics to avoid process::defer().
Date Thu, 08 Jun 2017 20:03:07 GMT


> On June 7, 2017, 12:18 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/include/process/metrics/gauge.hpp
> > Line 34 (original), 35-39 (patched)
> > <https://reviews.apache.org/r/59858/diff/2/?file=1743153#file1743153line35>
> >
> >     I believe the gist of this change is to recognize that a `Gauge` has no dependency
on the production of its value to be deferred; instead this decision should be should be left
to the creator of the `Gauge`.
> >     
> >     For most existing `Gauge`s in the Mesos code base we not only depend on some
process to carry out calculcation of values, but also introduce a life-time dependency made
safe by using `Deferred`. This is inherent to the functions passed in here (they could e.g.,
be implicitly converted from a `_Deferred`.
> >     
> >     Given that, I am not sure we need to explain `Deferred` here. I would instead
update the existing comment, e.g., `s/deferred object/producer function/` or similar.
> 
> James Peach wrote:
>     I wanted to capture something about the common usage in the codebase and give some
guidance without second-guessing how this should be used. I would like to make the comment
a bit more expansive so we can explain how to use `Gauge` safely, but I'm open to suggestions
:)

I cannot see how the caveats of function objects used with a `Gauge` are any different from
uses elsewhere (e.g., in continuations). If anything the previous version should have explained
why it choose that particular, restricted interface, but the form here seems natural to me
and requiring no references to possible data or lifetime dependencies.


- Benjamin


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


On June 6, 2017, 11:16 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59858/
> -----------------------------------------------------------
> 
> (Updated June 6, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A `Deferred` function object is always defined to be a
> `std::function`, so when we are constructing a `Gauge` metrics
> is it not strictly required for the callback to be a `Deferred`
> object. In cases where the value is a constant or can be calculated
> by reading constant data, a metrics producer can emit a `Gauge`
> without the taking the additional dispatch cost.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp c5bbeaf5d53c60f3636d1778db47cdb6cdda34e8

>   3rdparty/libprocess/src/tests/metrics_tests.cpp c13520d23ca17144f553fb4588fb8a747ea46e72

> 
> 
> Diff: https://reviews.apache.org/r/59858/diff/3/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


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