From: | Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: Misuse of TimestampDifference() in the autoprewarm feature of pg_prewarm |
Date: | 2020-11-10 16:58:44 |
Message-ID: | 01241062b6bfea2f17e32a65a4f99d61@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020-11-09 23:25, Tom Lane wrote:
> Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)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 | Content-Type | Size |
---|---|---|
v3-0003-pg_prewarm-add-tap-test-for-autoprewarm-feature.patch | text/x-diff | 2.4 KB |
v3-0002-pg_prewarm-fix-autoprewarm_interval-behaviour.patch | text/x-diff | 1.3 KB |
v3-0001-Implement-TimestampDifferenceMilliseconds.patch | text/x-diff | 2.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2020-11-10 17:12:44 | Re: [PATCH] Add features to pg_stat_statements |
Previous Message | Dave Cramer | 2020-11-10 16:53:20 | Re: Error on failed COMMIT |