Horton

Thread

Commits

  1. Try to avoid floating-point roundoff error in pg_sleep().

  1. Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-25T18:42:32Z

    I chanced to notice that if you ask pg_sleep for 1ms delay,
what you actually get is 2ms, for example

regression=# \timing on
Timing is on.
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.001);
end loop; end $$;
DO
Time: 2081.175 ms (00:02.081)
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.002);
end loop; end $$;
DO
Time: 2177.407 ms (00:02.177)

A bit of gdb-ing confirms that the delay passed to WaitLatch() is 2ms,
so the problem is fundamentally one of floating-point roundoff error
in pg_sleep's calculation of delay_ms.  I didn't try to figure out
exactly why that's happening.  It may well vary depending on the
absolute magnitude of current values of GetCurrentTimestamp(),
because I don't recall having noticed any such behavior back when
this code was written.

Anyway, I propose trying to get rid of this misbehavior by avoiding
floating point in the delay computation, as attached.  With this
patch I get less surprising behavior:

regression=# \timing on
Timing is on.
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.001);
end loop; end $$;
DO
Time: 1063.997 ms (00:01.064)
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.002);
end loop; end $$;
DO
Time: 2172.849 ms (00:02.173)

The code is a little more tied to TimestampTz being measured in
microseconds than it was before, but it wouldn't really be much
harder to fix if we ever change that.

			regards, tom lane

  2. Re: Avoiding roundoff error in pg_sleep()

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2025-09-25T18:54:34Z

    Hi
 
I suspect, that any user, that run something like pg_sleep(1000000000), 
start transaction, that stops autovacuum and creates other negative effects up to server crash,
and only this user can stop it by command interrupt (all signals only restart 
this sleep or kill whole server).
If I am correct, why Postgres needs this command?
 
Best regard,
Vladlen Popolitov.

On Thursday, September 25, 2025 21:42 MSK, Tom Lane <tgl@sss.pgh.pa.us> wrote:

 
I chanced to notice that if you ask pg_sleep for 1ms delay,
what you actually get is 2ms, for example

regression=# \timing on
Timing is on.
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.001);
end loop; end $$;
DO
Time: 2081.175 ms (00:02.081)
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.002);
end loop; end $$;
DO
Time: 2177.407 ms (00:02.177)

A bit of gdb-ing confirms that the delay passed to WaitLatch() is 2ms,
so the problem is fundamentally one of floating-point roundoff error
in pg_sleep's calculation of delay_ms. I didn't try to figure out
exactly why that's happening. It may well vary depending on the
absolute magnitude of current values of GetCurrentTimestamp(),
because I don't recall having noticed any such behavior back when
this code was written.

Anyway, I propose trying to get rid of this misbehavior by avoiding
floating point in the delay computation, as attached. With this
patch I get less surprising behavior:

regression=# \timing on
Timing is on.
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.001);
end loop; end $$;
DO
Time: 1063.997 ms (00:01.064)
regression=# do $$ begin
for i in 1..1000 loop
perform pg_sleep(0.002);
end loop; end $$;
DO
Time: 2172.849 ms (00:02.173)

The code is a little more tied to TimestampTz being measured in
microseconds than it was before, but it wouldn't really be much
harder to fix if we ever change that.

regards, tom lane

 



-- 
 Best regards,

Vladlen Popolitov.

  3. Re: Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-25T19:36:18Z

    =?utf-8?q?=D0=9F=D0=BE=D0=BF=D0=BE=D0=BB=D0=B8=D1=82=D0=BE=D0=B2_=D0=92=D0=BB=D0=B0=D0=B4=D0=BB=D0=B5=D0=BD?= <v.popolitov@postgrespro.ru> writes:
> I suspect, that any user, that run something like pg_sleep(1000000000), 
> start transaction, that stops autovacuum and creates other negative effects up to server crash,
> and only this user can stop it by command interrupt (all signals only restart 
> this sleep or kill whole server).

How is this different from any other long-running query?
If you'd bothered to test, you'd have seen that pg_sleep()
is interruptible by query cancel, just like anything else,
so "kill whole server" is not required.

We're not in the business of trying to limit users' resource
consumption.  If we wanted to do that, it'd be a very major
undertaking.

			regards, tom lane

  4. Re: Avoiding roundoff error in pg_sleep()

    Robert Haas <robertmhaas@gmail.com> — 2025-09-25T19:41:25Z

    On Thu, Sep 25, 2025 at 2:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I chanced to notice that if you ask pg_sleep for 1ms delay,
> what you actually get is 2ms, for example

Oh, wow. I tested and I get the same behavior.

> Anyway, I propose trying to get rid of this misbehavior by avoiding
> floating point in the delay computation, as attached.

Score one for my blind yet fiery hatred of floating-point arithmetic.
The patch looks good except that (places tongue firmly in cheek) it
will cause problems for users who want to sleep for more than 150,000
years.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  5. Re: Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-25T19:44:34Z

    Robert Haas <robertmhaas@gmail.com> writes:
> The patch looks good except that (places tongue firmly in cheek) it
> will cause problems for users who want to sleep for more than 150,000
> years.

We will all be safely dead before the first such bug report ;-)

			regards, tom lane

  6. Re: Avoiding roundoff error in pg_sleep()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-09-25T19:46:40Z

    On Thu, Sep 25, 2025 at 02:42:32PM -0400, Tom Lane wrote:
> Anyway, I propose trying to get rid of this misbehavior by avoiding
> floating point in the delay computation, as attached.  With this
> patch I get less surprising behavior:
> 
> [...]
> 
> The code is a little more tied to TimestampTz being measured in
> microseconds than it was before, but it wouldn't really be much
> harder to fix if we ever change that.

LGTM.  I considered suggesting initializing the delay before the loop and
then updating it at the end of the loop, but that moves the
CHECK_FOR_INTERRUPTS between the delay calculation and the WaitLatch(),
which seems like it might extend the sleeps a bit (although that might be
negligible in practice).  Otherwise, the code seems to match
float8_timestamptz() somewhat closely, although I doubt it's worth trying
to unify the implementations.

-- 
nathan

  7. Re: Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-25T20:11:14Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Sep 25, 2025 at 02:42:32PM -0400, Tom Lane wrote:
>> Anyway, I propose trying to get rid of this misbehavior by avoiding
>> floating point in the delay computation, as attached.

> LGTM.  I considered suggesting initializing the delay before the loop and
> then updating it at the end of the loop, but that moves the
> CHECK_FOR_INTERRUPTS between the delay calculation and the WaitLatch(),
> which seems like it might extend the sleeps a bit (although that might be
> negligible in practice).

Yeah, I did not consider changing the fundamental logic of the loop.
It's true that this implementation will use a minimum of three
GetCurrentTimestamp() calls where in principle you should need only
two when WaitLatch() doesn't exit early.  I'm having a hard time
getting excited about that though.  We know that on just about all
current platforms, reading the clock is a handful-of-nanoseconds
operation.  (cf. nearby discussion of GNU/Hurd where we found that
that OS is far behind the curve; but I imagine they'll have fixed
that by the time anyone would consider Hurd ready for production.)
So it seems pretty negligible in a function that currently has 1ms
delay resolution and is unlikely to have better than 1us resolution
in the future.

> Otherwise, the code seems to match
> float8_timestamptz() somewhat closely, although I doubt it's worth trying
> to unify the implementations.

Comparing that, I guess I should write USECS_PER_SEC not 1e6.
Not that it'd make a difference now, but in some hypothetical
future where somebody is grepping for dependencies on 1us
TimestampTz resolution, it'd help them find this one.

Thanks for looking at it!

			regards, tom lane

  8. Re: Avoiding roundoff error in pg_sleep()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-09-25T20:51:15Z

    On Thu, Sep 25, 2025 at 04:11:14PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> LGTM.  I considered suggesting initializing the delay before the loop and
>> then updating it at the end of the loop, but that moves the
>> CHECK_FOR_INTERRUPTS between the delay calculation and the WaitLatch(),
>> which seems like it might extend the sleeps a bit (although that might be
>> negligible in practice).
> 
> Yeah, I did not consider changing the fundamental logic of the loop.
> It's true that this implementation will use a minimum of three
> GetCurrentTimestamp() calls where in principle you should need only
> two when WaitLatch() doesn't exit early.  I'm having a hard time
> getting excited about that though.  We know that on just about all
> current platforms, reading the clock is a handful-of-nanoseconds
> operation.  (cf. nearby discussion of GNU/Hurd where we found that
> that OS is far behind the curve; but I imagine they'll have fixed
> that by the time anyone would consider Hurd ready for production.)
> So it seems pretty negligible in a function that currently has 1ms
> delay resolution and is unlikely to have better than 1us resolution
> in the future.

Agreed, I'm not too worried about the system calls in this case.  I think I
was more interested in seeing whether we could avoid the complicated float
handling.  Something else that seems to work is moving the initial endtime
calculation to within the loop, like so:

-        delay = endtime - GetNowFloat();
+        if (first)
+        {
+            endtime = GetNowFloat() + secs;
+            delay = secs;
+            first = false;
+        }
+        else
+            delay = endtime - GetNowFloat();

But, in any case, your patch still LGTM.

-- 
nathan

  9. Re: Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-25T21:05:45Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> Agreed, I'm not too worried about the system calls in this case.  I think I
> was more interested in seeing whether we could avoid the complicated float
> handling.  Something else that seems to work is moving the initial endtime
> calculation to within the loop, like so:

> -        delay = endtime - GetNowFloat();
> +        if (first)
> +        {
> +            endtime = GetNowFloat() + secs;
> +            delay = secs;
> +            first = false;
> +        }
> +        else
> +            delay = endtime - GetNowFloat();

Hmm ... I'm not sure I'd trust that to give the right answer
if WaitLatch is interrupted so that we have to recalculate
the delay.

> But, in any case, your patch still LGTM.

Pushed it.  Again, thanks for reviewing!

			regards, tom lane

  10. Re: Avoiding roundoff error in pg_sleep()

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2025-09-26T14:58:01Z

    Hi Tom,

Tom Lane писал(а) 2025-09-25 22:36:
> =?utf-8?q?=D0=9F=D0=BE=D0=BF=D0=BE=D0=BB=D0=B8=D1=82=D0=BE=D0=B2_=D0=92=D0=BB=D0=B0=D0=B4=D0=BB=D0=B5=D0=BD?= 
> <v.popolitov@postgrespro.ru> writes:
>> I suspect, that any user, that run something like 
>> pg_sleep(1000000000), 
>> start transaction, that stops autovacuum and creates other negative 
>> effects up to server crash,
>> and only this user can stop it by command interrupt (all signals only 
>> restart 
>> this sleep or kill whole server).
> 
> How is this different from any other long-running query?
> If you'd bothered to test, you'd have seen that pg_sleep()
> is interruptible by query cancel, just like anything else,
> so "kill whole server" is not required.

  I will try to explain my worries about this. Long running queries could 
run long
due to higher input-output, and they are limited at least by work_mem 
parameter.
pg_sleep() is not limited by anything.

> We're not in the business of trying to limit users' resource
> consumption.  If we wanted to do that, it'd be a very major
> undertaking.

For example, mssql does not have function like this, it has
command WAIT FOR interval . This interval cannot be more than 24H.
And this separate command does not start transaction. It is
possible to include it in transaction block anyway, but if
user needs pause, he get it without disturbing of other users.

In our case the user gets a pause and stops autovacuum, create index
concurrently and other commands relaying to transaction id
for all other users - without choice to avoid this behaviour.

I heard about customer, who had real trouble with this. I can
understand them, because in yearly 1990s I stopped university
mainframe by simple command "DISPLAY MEMORY" asking for all memory,
and later explained to administrator - "I just wanted to see,
what will happen".
It looks like sleep has the wrong place in a function. It should be
an utility command, that does not start a transaction.

Best regards,

Vladlen Popolitov.

  11. Re: Avoiding roundoff error in pg_sleep()

    David G. Johnston <david.g.johnston@gmail.com> — 2025-09-26T15:08:10Z

    On Friday, September 26, 2025, Vladlen Popolitov <v.popolitov@postgrespro.ru>
wrote:

>
> It looks like sleep has the wrong place in a function. It should be
> an utility command, that does not start a transaction.
>

Then write that command.  We’re not going to change pg_sleep.  It’s works
just as advertised.  True, it probably should never be used in production.

I can see a pipeline sequence tossing a wait command into the flow so
having the server accept a wait instruction has at least some immediate
merit.

David J.

  12. Re: Avoiding roundoff error in pg_sleep()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-26T15:10:19Z

    Vladlen Popolitov <v.popolitov@postgrespro.ru> writes:
> Tom Lane писал(а) 2025-09-25 22:36:
>> How is this different from any other long-running query?

>   I will try to explain my worries about this. Long running queries could 
> run long
> due to higher input-output, and they are limited at least by work_mem 
> parameter.
> pg_sleep() is not limited by anything.

So I assume for example, that you would also argue for removing
all looping constructs from pl/pgsql so that users could not
write infinite loops?
	DO $$ BEGIN LOOP END LOOP; END $$;
is just as effective a blocking transaction as pg_sleep,
plus it consumes a CPU.

I think the right answer to concerns like this is transaction_timeout
and similar features, not arbitrary restrictions.

			regards, tom lane

  13. Re: Avoiding roundoff error in pg_sleep()

    Vladlen Popolitov <v.popolitov@postgrespro.ru> — 2025-09-26T15:25:09Z

    Hi David,

David G. Johnston писал(а) 2025-09-26 18:08:
> On Friday, September 26, 2025, Vladlen Popolitov
> <v.popolitov@postgrespro.ru> wrote:
> 
>> It looks like sleep has the wrong place in a function. It should be
>> an utility command, that does not start a transaction.
> 
> Then write that command.  We’re not going to change pg_sleep.
> It’s works just as advertised.

It _does_ not work as it advertised. Help page states:
1) "delay execution of the server process"
2) "pg_sleep makes the current session's process sleep"
In reality it stops other backends too.

> True, it probably should never be
> used in production.
yes, it should not be used, if you know, but even advanced users do
not know the effect of this function. And it can be used by any user
of the server.

I just pointed attention, that this function could have bigger problem
with high values of parameter, not only very small, if you consider
problem with small value is really problem.

-- 
Best regards,

Vladlen Popolitov.

  14. Re: Avoiding roundoff error in pg_sleep()

    David G. Johnston <david.g.johnston@gmail.com> — 2025-09-26T15:32:46Z

    On Friday, September 26, 2025, Vladlen Popolitov <v.popolitov@postgrespro.ru>
wrote:

> Hi David,
>
> David G. Johnston писал(а) 2025-09-26 18:08:
>
>> On Friday, September 26, 2025, Vladlen Popolitov
>> <v.popolitov@postgrespro.ru> wrote:
>>
>> It looks like sleep has the wrong place in a function. It should be
>>> an utility command, that does not start a transaction.
>>>
>>
>> Then write that command.  We’re not going to change pg_sleep.
>> It’s works just as advertised.
>>
>
> It _does_ not work as it advertised. Help page states:
> 1) "delay execution of the server process"
> 2) "pg_sleep makes the current session's process sleep"
> In reality it stops other backends too.
>

That is not reality.  And as noted, any similar reality is not only
manifested by using this function.

You are going to need to construct a test case demonstrating your concerns,
which will likely alleviate them instead.  Or, at least, let you accept
that once you’ve given someone a login to your database they can indeed
perform a DoS on it if you haven’t taken appropriate precautions with
timeouts and such.

David J.

  15. Re: Avoiding roundoff error in pg_sleep()

    Robert Haas <robertmhaas@gmail.com> — 2025-09-26T15:46:02Z

    On Fri, Sep 26, 2025 at 11:25 AM Vladlen Popolitov
<v.popolitov@postgrespro.ru> wrote:
> It _does_ not work as it advertised. Help page states:
> 1) "delay execution of the server process"
> 2) "pg_sleep makes the current session's process sleep"
> In reality it stops other backends too.

This thread is about fixing a bug in pg_sleep. I understand that you
don't like the way that pg_sleep works and think it should work
differently. I think you're wrong, and I think Tom and David are
right, and I think they've been more than generous in taking time to
respond to your concerns. But even if they are wrong and you are
right, it's not right to try to hijack a thread about fixing a bug in
pg_sleep to demand that it be redesigned. Please stop doing that. If
you want to, you can start a separate thread to discuss the problem
you're concerned about, but leave this one for discussion of the bug
fix.

-- 
Robert Haas
EDB: http://www.enterprisedb.com