Hi, On 2024-12-09 00:12:32 +0100, Tomas Vondra wrote: > Hi, the TAP test 001_connection_limits.pl introduced by 6a1d0d470e84 > seems to have problems with valgrind :-( I reliably get this failure: > > > t/001_connection_limits.pl .. 3/? # Tests were run but no plan was > declared and done_testing() was not seen. > # Looks like your test exited with 29 just after 4. > t/001_connection_limits.pl .. Dubious, test returned 29 (wstat 7424, 0x1d00) > All 4 subtests passed > > > and tmp_check/log/regress_log_001_connection_limits says: > > > [23:48:44.444](1.129s) ok 3 - reserved_connections limit > [23:48:44.445](0.001s) ok 4 - reserved_connections limit: matches > process ended prematurely at > /home/user/work/postgres/src/test/postmaster/../../../src/test/perl/PostgreSQL/Test/BackgroundPsql.pm > line 154. > # Postmaster PID for node "primary" is 198592
I just saw this failure on skink in the BF: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2025-03-04%2015%3A43%3A23 [17:05:56.438](0.247s) ok 3 - reserved_connections limit [17:05:56.438](0.000s) ok 4 - reserved_connections limit: matches process ended prematurely at /home/bf/bf-build/skink-master/HEAD/pgsql/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm line 160. > That BackgroundPsql.pm line is this in wait_connect() > > $self->{run}->pump() > until $self->{stdout} =~ /$banner/ || $self->{timeout}->is_expired; A big part of the problem here imo is the exception behaviour that IPC::Run::pump() has: If pump() is called after all harnessed activities have completed, a "process ended prematurely" exception to be thrown. This allows for simple scripting of external applications without having to add lots of error handling code at each step of the script: Which is, uh, not very compatible with how we use IPC::Run (here and elsewhere). Just ending the test because a connection failed is pretty awful. This behaviour makes it really hard to debug problems. It'd have been a lot easier to understand the problem if we'd seen psql's stderr before the test died. I guess that mean at the very least we'd need to put an eval {} around the ->pump() call., print $self->{stdout}, ->{stderr} and reraise an error? Presumably not just in in wait_connect(), but also at least in pump_until()? Will respond downthread to a potential workaround for the issue. Greetings, Andres Freund
