Re: Misuse of TimestampDifference() in the autoprewarm feature of pg_prewarm - Mailing list pgsql-hackers

From Alexey Kondratov
Subject Re: Misuse of TimestampDifference() in the autoprewarm feature of pg_prewarm
Date
Msg-id 01241062b6bfea2f17e32a65a4f99d61@postgrespro.ru
Whole thread Raw
In response to Re: Misuse of TimestampDifference() in the autoprewarm feature of pg_prewarm  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Misuse of TimestampDifference() in the autoprewarm feature of pg_prewarm  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2020-11-09 23:25, Tom Lane wrote:
> Alexey Kondratov <a.kondratov@postgrespro.ru> writes:
>> On 2020-11-09 21:53, Tom Lane wrote:
>>> 0002 seems like a pretty clear bug fix, though I wonder if this is 
>>> exactly
>>> what we want to do going forward.  It seems like a very large 
>>> fraction of
>>> the callers of TimestampDifference would like to have the value in 
>>> msec,
>>> which means we're doing a whole lot of expensive and error-prone
>>> arithmetic to break down the difference to sec/usec and then put it
>>> back together again.  Let's get rid of that by inventing, say
>>> TimestampDifferenceMilliseconds(...).
> 
>> Yeah, I get into this problem after a bug in another extension —
>> pg_wait_sampling. I have attached 0002, which implements
>> TimestampDifferenceMilliseconds(), so 0003 just uses this new function
>> to solve the initial issues. If it looks good to you, then we can 
>> switch
>> all similar callers to it.
> 
> Yeah, let's move forward with that --- in fact, I'm inclined to
> back-patch it.  (Not till the current release cycle is done, though.
> I don't find this important enough to justify a last-moment patch.)
> 
> BTW, I wonder if we shouldn't make TimestampDifferenceMilliseconds
> round any fractional millisecond up rather than down.  Rounding down
> seems to create a hazard of uselessly waking just before the delay is
> completed.  Better to wake just after.
> 

Yes, it make sense. I have changed TimestampDifferenceMilliseconds() to 
round result up if there is a reminder.

After looking on the autoprewarm code more closely I have realised that 
this 'double dump' issues was not an issues at all. I have just 
misplaced a debug elog(), so its second output in the log was only 
indicating that we calculated delay_in_ms one more time. Actually, even 
with wrong calculation of delay_in_ms the only problem was that we were 
busy looping with ~1 second interval instead of waiting on latch.

It is still a buggy behaviour, but much less harmful than I have 
originally thought.


Regards
-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company
Attachment

pgsql-hackers by date:

Previous
From: Dave Cramer
Date:
Subject: Re: Error on failed COMMIT
Next
From: Fujii Masao
Date:
Subject: Re: [PATCH] Add features to pg_stat_statements