\watch 0 or \watch 0.00001 doesn't do what I want
Daniel's post [1]/messages/by-id/B2FD26B4-8F64-4552-A603-5CC3DF1C7103@yesql.se 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]: /messages/by-id/B2FD26B4-8F64-4552-A603-5CC3DF1C7103@yesql.se
/messages/by-id/B2FD26B4-8F64-4552-A603-5CC3DF1C7103@yesql.se
--
Heikki Linnakangas
Neon (https://neon.tech)
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)
---- 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
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
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
Attachments:
psql-watch.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 2bb8789750..1c9905bad4 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -5359,6 +5359,10 @@ do_shell(const char *command)
*
* We break this out of exec_command to avoid having to plaster "volatile"
* onto a bunch of exec_command's variables to silence stupider compilers.
+ *
+ * "sleep" is the amount of time to sleep during each loop, measured in
+ * seconds. The internals of this function should use "sleep_ms" for
+ * precise sleep time calculations.
*/
static bool
do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
@@ -5484,10 +5488,10 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (user_title)
snprintf(title, title_len, _("%s\t%s (every %gs)\n"),
- user_title, timebuf, sleep);
+ user_title, timebuf, sleep_ms / 1000.0);
else
snprintf(title, title_len, _("%s (every %gs)\n"),
- timebuf, sleep);
+ timebuf, sleep_ms / 1000.0);
myopt.title = title;
/* Run the query and print out the result */
@@ -5508,7 +5512,7 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (pagerpipe && ferror(pagerpipe))
break;
- if (sleep == 0)
+ if (sleep_ms == 0)
continue;
#ifdef WIN32
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.
Attachments:
v2-0001-Prevent-psql-haning-in-watch-with-submillisecond-.patchapplication/octet-stream; name=v2-0001-Prevent-psql-haning-in-watch-with-submillisecond-.patch; x-unix-mode=0644Download
From a74d8bd8589881e701b2e95b521f1208a9c3c580 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 10 Oct 2024 10:43:54 +0500
Subject: [PATCH v2] Prevent psql haning in \watch with submillisecond interval
---
src/bin/psql/command.c | 11 ++++++++---
src/bin/psql/t/001_basic.pl | 4 ++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 2bb87897505..328d78c73f9 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -5359,6 +5359,10 @@ do_shell(const char *command)
*
* We break this out of exec_command to avoid having to plaster "volatile"
* onto a bunch of exec_command's variables to silence stupider compilers.
+ *
+ * "sleep" is the amount of time to sleep during each loop, measured in
+ * seconds. The internals of this function should use "sleep_ms" for
+ * precise sleep time calculations.
*/
static bool
do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
@@ -5484,10 +5488,10 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (user_title)
snprintf(title, title_len, _("%s\t%s (every %gs)\n"),
- user_title, timebuf, sleep);
+ user_title, timebuf, sleep_ms / 1000.0);
else
snprintf(title, title_len, _("%s (every %gs)\n"),
- timebuf, sleep);
+ timebuf, sleep_ms / 1000.0);
myopt.title = title;
/* Run the query and print out the result */
@@ -5508,7 +5512,8 @@ do_watch(PQExpBuffer query_buf, double sleep, int iter, int min_rows)
if (pagerpipe && ferror(pagerpipe))
break;
- if (sleep == 0)
+ /* Tight loop, no wait needed */
+ if (sleep_ms == 0)
continue;
#ifdef WIN32
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 5f2f4541af0..dedaf18e44f 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -355,6 +355,10 @@ psql_like(
psql_like($node, sprintf('SELECT 1 \watch c=3 i=%g', 0.01),
qr/1\n1\n1/, '\watch with 3 iterations');
+# Sub-millisecond wait should not hang
+psql_like($node, sprintf('SELECT 1 \watch c=3 i=%g', 0.0001),
+ qr/1\n1\n1/, '\watch with 3 iterations');
+
# Check \watch minimum row count
psql_fails_like(
$node,
--
2.42.0
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
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