Re: About ceph_clock_now()

[Erwan Velu <evelu@redhat.com>]

Before propagating the change to more code base and offer a PR with that, what do you think of this style ?

Is it aligned with the best-pratices of the project ?

https://gist.github.com/ErwanAliasr1/f74f7f046e9300ac2c30

Thanks,

----- Mail original -----
De: "Sage Weil" <sweil@redhat.com>
À: "Erwan Velu" <evelu@redhat.com>
Cc: "Adam C. Emerson" <aemerson@redhat.com>, "The Sacred Order of the Squid Cybernetic" <ceph-devel@vger.kernel.org>
Envoyé: Mercredi 13 Janvier 2016 14:48:00
Objet: Re: About ceph_clock_now()

On Wed, 13 Jan 2016, Erwan Velu wrote:
> >One concern with monotonic clocks, even for elapsed time, is that their epoch is
> >unspecified. You cannot send a monotonic time over the network to another
> >machine nor can you save it to a file and re-read it later. Anything to be
> >serialized must be a real time.
> 
> While speaking about ceph_clock_now(), I was more likely targeting lots of local code like
>   start = ceph_clock_now();
>   ... do_stuff ...
>   elapse_time = ceph_clock_now() - start
> 
> 
> I'm thinking of code like this in void OSD::tick_without_osd_lock():
> Starting at https://github.com/ceph/ceph/blob/master/src/osd/OSD.cc#L4009.
> 
> This code is trying to estimate if a mon is still alive by computing a difference of system time by
> doing a  "if (now - last_pg_stats_sent > max) {" where now & last_pg_stats_sent are the output of "ceph_clock_now()"
> 
> As ceph_clock_now() is using CLOCK_REALTIME, we could have cases where a simple drift caused by a NTP trigger that code.
> As we are only trying to compute a difference of absolute time (and not system time), using a MONOTONIC_* seems much more accurate at my taste.
> 
> I know that's pretty picky but that could exists.

That's a perfect example of where the monotonic time should be used. There 
are a zillion instances of this pattern that could be cleaned up... :)

> >I find it interesting. If you want to change more subsystems to use ceph_time
> >and use monotonic and coarse clocks in appropriate places I think that would be
> >wonderful. It might be worthwhile to focus on the places where we use double to
> >represent a time value.
> >Please read through ceph_time.h and osdc to get a feel for it. If you think it
> >could use improvement, I'd be happy to hear your ideas.
> 
> You mean we could switch some of ceph_clock_now() calls to ceph_time ? 
> We could use coarse_mono_clock to perform that.
> 
> If you agree that switching code like the one I'm speaking is valuable, 
> I can work on it.

That would be great!

sage

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html