Thread: pgsql: tests: Fix race condition in postmaster/002_connection_limits

pgsql: tests: Fix race condition in postmaster/002_connection_limits

From
Andres Freund
Date:
tests: Fix race condition in postmaster/002_connection_limits

The test occasionally failed due to unexpected connection limit errors being
encountered after having waited for FATAL errors on another connection. These
spurious failures were caused by the the backend reporting FATAL errors to the
client before detaching from the PGPROC entry. Adding a sleep(1) before
proc_exit() makes it easy to reproduce that problem.

To fix the issue, add a helper function that waits for postmaster to notice
the process having exited. For now this is implemented by waiting for the
DEBUG2 message that postmaster logs in that case. That's not the prettiest
fix, but simple. If we notice this problem elsewhere, it might be worthwhile
to make this more general, e.g. by adding an injection point.

Reported-by: Tomas Vondra <tomas@vondra.me>
Diagnosed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Tested-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/71d1ed6fe129935ce2764c5d34924512ed046f69

Modified Files
--------------
src/test/postmaster/t/002_connection_limits.pl | 35 +++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)


On 2025-03-07 Fr 1:22 PM, Andres Freund wrote:
> tests: Fix race condition in postmaster/002_connection_limits
>
> The test occasionally failed due to unexpected connection limit errors being
> encountered after having waited for FATAL errors on another connection. These
> spurious failures were caused by the the backend reporting FATAL errors to the
> client before detaching from the PGPROC entry. Adding a sleep(1) before
> proc_exit() makes it easy to reproduce that problem.
>
> To fix the issue, add a helper function that waits for postmaster to notice
> the process having exited. For now this is implemented by waiting for the
> DEBUG2 message that postmaster logs in that case. That's not the prettiest
> fix, but simple. If we notice this problem elsewhere, it might be worthwhile
> to make this more general, e.g. by adding an injection point.
>


New test breaks when log_error_verbosity=verbose. It's adding an SQL 
ERRCODE to the DEBUG string.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com




Hi,

On 2025-03-10 18:46:12 -0400, Andrew Dunstan wrote:
> On 2025-03-07 Fr 1:22 PM, Andres Freund wrote:
> > tests: Fix race condition in postmaster/002_connection_limits
> > 
> > The test occasionally failed due to unexpected connection limit errors being
> > encountered after having waited for FATAL errors on another connection. These
> > spurious failures were caused by the the backend reporting FATAL errors to the
> > client before detaching from the PGPROC entry. Adding a sleep(1) before
> > proc_exit() makes it easy to reproduce that problem.
> > 
> > To fix the issue, add a helper function that waits for postmaster to notice
> > the process having exited. For now this is implemented by waiting for the
> > DEBUG2 message that postmaster logs in that case. That's not the prettiest
> > fix, but simple. If we notice this problem elsewhere, it might be worthwhile
> > to make this more general, e.g. by adding an injection point.
> > 
> 
> 
> New test breaks when log_error_verbosity=verbose. It's adding an SQL ERRCODE
> to the DEBUG string.

Ugh, that's annoying. I guess no good deed (fixing the test) goes
unpunished...

Pushed a fix.

Greetings,

Andres Freund