On Wed, Mar 5, 2025 at 8:08 PM Michael Paquier <michael@paquier.xyz> wrote:
> Honestly, I don't see a reason not to introduce that, like in the
> attached.
This new code races against the session timeout. I see this on timer expiration:
[14:19:55.224](0.000s) # issuing query 34 via background psql:
SELECT state FROM pg_stat_activity WHERE pid = ;
[14:19:55.228](0.004s) # pump_until: process terminated
unexpectedly when searching for "(?^:(^|\n)background_psql:
QUERY_SEPARATOR 34:\r?\n)" with stream: ""
process ended prematurely at
/home/jacob/src/postgres/src/test/perl/PostgreSQL/Test/Utils.pm line
439.
Which makes it seem like some sort of crash, IMO. I don't find that as
easily debuggable as the previous log message, which was
[14:21:33.104](0.001s) # issuing query 32 via background psql:
SELECT pid FROM pg_stat_activity
# WHERE state = 'starting' and wait_event = 'init-pre-auth';
IPC::Run: timeout on timer #1 at
/home/jacob/perl5/lib/perl5/IPC/Run.pm line 3007.
> + WHERE state = 'starting' and wait_event = 'init-pre-auth';});
Did you have thoughts on expanding the check to backend_type [1]?
> + # Give up. The output of the last attempt is logged by query(),
> + # so no need to do anything here.
> + return 0;
One of my primary complaints about the poll_query_until()
implementation is that "giving up" in this case means continuing to
run pieces of the test that have no business running after a timeout,
and increasing the log noise after a failure. I'm not sure how loudly
to complain in this particular case, since I know we use it
elsewhere...
Thanks!
--Jacob
[1] https://postgr.es/m/CAOYmi%2BnxNCQcTQE-tQ7Lwpe4cYc1u-yxwEe5kt2AVN%2BDXXVVbQ%40mail.gmail.com
