Horton

Thread

Commits

  1. psql: Fix \watch when using interval values less than 1ms

  1. \watch 0 or \watch 0.00001 doesn't do what I want

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-10-09T13:38:56Z

    Daniel's post [1] on \watch reminded me of this little issue I bumped into:

I wanted to run a query in a tight loop, without any delay. I tried 
"\watch 0", but it didn't do what I wanted:

postgres=# \watch 0
Wed 09 Oct 2024 16:34:19 EEST (every 1s)

  ?column?
----------
         1
(1 row)

Wed 09 Oct 2024 16:34:20 EEST (every 1s)

  ?column?
----------
         1
(1 row)

^C

Then I tried setting the delay really small, but that didn't do what I 
wanted either:

postgres=# \watch 0.00001
Wed 09 Oct 2024 16:36:45 EEST (every 1e-05s)

  ?column?
----------
         1
(1 row)

^C

It runs the query just once and then hangs forever, until I hit CTRL-C 
to cancel.

[1] 
https://www.postgresql.org/message-id/B2FD26B4-8F64-4552-A603-5CC3DF1C7103%40yesql.se

-- 
Heikki Linnakangas
Neon (https://neon.tech)

  2. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-10-09T13:59:35Z

    On 09/10/2024 16:38, Heikki Linnakangas wrote:
> Daniel's post [1] on \watch reminded me of this little issue I bumped into:
> 
> I wanted to run a query in a tight loop, without any delay. I tried 
> "\watch 0", but it didn't do what I wanted:
> 
> postgres=# \watch 0

Correction: This changed in version 16. It works the way I expected on 
v16, but not in earlier versions.

> Then I tried setting the delay really small, but that didn't do what I wanted either:
> 
> postgres=# \watch 0.00001
> Wed 09 Oct 2024 16:36:45 EEST (every 1e-05s)
> 
>  ?column?
> ----------
>         1
> (1 row)
> 
> ^C
> 
> It runs the query just once and then hangs forever, until I hit CTRL-C to cancel.

This issue is present on newer versions still.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

  3. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Srinath Reddy Sadipiralla <srinath.reddy@zohocorp.com> — 2024-10-09T14:07:38Z

    ---- On Wed, 09 Oct 2024 19:08:56 +0530 Heikki Linnakangas <hlinnaka@iki.fi> wrote ---

 
Then I tried setting the delay really small, but that didn't do what I 
wanted either: 
 
postgres=# \watch 0.00001 
Wed 09 Oct 2024 16:36:45 EEST (every 1e-05s) 
 
 ?column? 
---------- 
 1 
(1 row) 
 
^C 
 
It runs the query just once and then hangs forever, until I hit CTRL-C 
to cancel. 
 
hi heikki,but i am in pg 14.7 and the query is running perfectly without getting stuck.

Regards,
Srinath Reddy Sadipiralla
Member Technical Staff
ZOHO

  4. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-09T15:03:03Z

    Heikki Linnakangas <hlinnaka@iki.fi> writes:
> This issue is present on newer versions still.

Here's the problem:

	long		sleep_ms = (long) (sleep * 1000);

If "sleep" is less than 0.0005, sleep_ms rounds to zero, which
results in the subsequent setitimer disarming rather than
arming the interrupt.

There is an uncommented

		if (sleep == 0)
			continue;

in the loop, which I bet some cowboy added to fix the zero-wait
problem you complained of.  But it's doing the wrong thing because
it checks sleep not sleep_ms.

We should change this to test sleep_ms, and we should probably
fix the code that says what the wait interval is to print
sleep_ms/1000.0 not sleep.  And some more comments would be good.

			regards, tom lane

  5. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Michael Paquier <michael@paquier.xyz> — 2024-10-09T23:49:44Z

    On Wed, Oct 09, 2024 at 11:03:03AM -0400, Tom Lane wrote:
> in the loop, which I bet some cowboy added to fix the zero-wait
> problem you complained of.  But it's doing the wrong thing because
> it checks sleep not sleep_ms.
> 
> We should change this to test sleep_ms, and we should probably
> fix the code that says what the wait interval is to print
> sleep_ms/1000.0 not sleep.  And some more comments would be good.

Cowboy of 6f9ee74d45aa reporting for duty.  This was not backpatched
as of the reason written in the commit log.  If folks would rather get
the zero behavior backported, I'm OK with a more aggressive backpatch
if that's the consensus.  A cherry-pick of 6f9ee74d45aa down to v12 is
clean enough, with a small conflict related to pagerpipe so it looks
pretty much fine.

Also, long is 4 bytes on Windows.  Should we just use uint64 for
sleep_ms?  I am not sure that it is worth changing as it still gives
plenty of maximum interval time even with sleep_ms in milliseconds.

Anyway, I agree that this could be better for values with lower
digits, even if \watch would output 0s when using an internal lower
than 10^-3.  How about something like the simple patch attached?
--
Michael

  6. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    x4mmm@yandex-team.ru — 2024-10-10T05:47:25Z

    
> On 10 Oct 2024, at 04:49, Michael Paquier <michael@paquier.xyz> wrote:
> 
> How about something like the simple patch attached?

Let’s add a comment to tight-loop if statement. And a test for the case.


Best regards, Andrey Borodin.

  7. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Michael Paquier <michael@paquier.xyz> — 2024-10-13T03:04:23Z

    On Thu, Oct 10, 2024 at 10:47:25AM +0500, Andrey M. Borodin wrote:
> Let’s add a comment to tight-loop if statement. And a test for the case.

+       /* Tight loop, no wait needed */
+       if (sleep_ms == 0)

Okay about this addition.

+psql_like($node, sprintf('SELECT 1 \watch c=3 i=%g', 0.0001),
+   qr/1\n1\n1/, '\watch with 3 iterations');

And I am fine with this addition as well, for the sleep_ms == 0 case.
--
Michael

  8. Re: \watch 0 or \watch 0.00001 doesn't do what I want

    Michael Paquier <michael@paquier.xyz> — 2024-10-14T03:29:31Z

    On Sun, Oct 13, 2024 at 12:04:23PM +0900, Michael Paquier wrote:
> +       /* Tight loop, no wait needed */
> +       if (sleep_ms == 0)
> 
> Okay about this addition.
> 
> +psql_like($node, sprintf('SELECT 1 \watch c=3 i=%g', 0.0001),
> +   qr/1\n1\n1/, '\watch with 3 iterations');
> 
> And I am fine with this addition as well, for the sleep_ms == 0 case.

I have done an extra lookup, tweaked a few things, and backpatched
that down to v16.
--
Michael