Thanks for confirming Andreas, and will do!

-----Original Message-----
From: Andreas Dilger <adil...@whamcloud.com> 
Sent: Wednesday, August 24, 2022 8:47 PM
To: Ellis Wilson <elliswil...@microsoft.com>
Cc: lustre-discuss@lists.lustre.org
Subject: [EXTERNAL] Re: [lustre-discuss] lproc stats changed snapshot_time from 
unix-epoch to uptime/monotonic in 2.15

Ellis, thanks for reporting this.  This looks like it was a mistake. 

The timestamps should definitely be in wallclock time, but this looks to have 
been changed unintentionally to reduce overhead, and use a u64 instead of 
dealing with timespec64 math, while losing the original intent (there are many 
different ktime_get variants, all alike).

I think many monitoring tools will be unaffected because they use the delta 
between successive timestamps, but having timestamps that are relative to boot 
time is problematic since they may repeat or go backward after a reboot, and 
some tools may use this timestamp when inserting into a tsdb to avoid 
processing lag. 

Please file a ticket, and ideally if you can submit a patch that converts 
ktime_get() to ktime_get_real_ns() for the places that are changed in the patch 
(with a "Fixes:" line to track it against the original patch, which was commit 
ea2cd3af7b).

Cheers, Andreas

> On Aug 24, 2022, at 14:50, Ellis Wilson via lustre-discuss 
> <lustre-discuss@lists.lustre.org> wrote:
> 
> Hi all,
> 
> One of my colleagues noticed that in testing 2.15.1 out the stats returned 
> include snapshot_time showing up in a different fashion than before.  
> Previously, ktime_get_real_ts64 was used to get the current timestamp and 
> that was presented when stats were printed, whereas now uptime is used as 
> returned by ktime_get.  Is there a monotonic requirement to snapshot_time 
> that I'm not thinking about that makes ktime_get more useful?  The previous 
> behavior of getting the current time alongside the stats so you could reason 
> about when they were gotten made more sense to me.  But perhaps Andreas had a 
> different vision for use of snapshot_time given that he was the one who 
> revised it?
> 
> I'm glad to open a bug and propose a patch if this was just a mistake, but 
> figured I'd ask first.
> 
> Best,
> 
> ellis
> _______________________________________________
> lustre-discuss mailing list
> lustre-discuss@lists.lustre.org
> https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.lustre.org%2Flistinfo.cgi%2Flustre-discuss-lustre.org&amp;data=05%7C01%7Celliswilson%40microsoft.com%7C211612b72e78476e056008da8633576a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637969852332016835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Ycw4J8CUQ1WC9c96G2B0ko1gwPO1A4sj9ThFz3xuxuQ%3D&amp;reserved=0
_______________________________________________
lustre-discuss mailing list
lustre-discuss@lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-discuss-lustre.org

Reply via email to