Horton
Menu

Re: Refactoring postmaster's code to cleanup after child exit

Heikki Linnakangas <hlinnaka@iki.fi>

View thread
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andres Freund <andres@anarazel.de>
Cc: Thomas Munro <thomas.munro@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2024-09-06T09:52:37Z
Lists: pgsql-hackers 
On 04/09/2024 17:35, Andres Freund wrote:
> On 2024-08-12 12:55:00 +0300, Heikki Linnakangas wrote:
>> +Running the tests
>> +=================
>> +
>> +NOTE: You must have given the --enable-tap-tests argument to configure.
>> +
>> +Run
>> +    make check
>> +or
>> +    make installcheck
>> +You can use "make installcheck" if you previously did "make install".
>> +In that case, the code in the installation tree is tested.  With
>> +"make check", a temporary installation tree is built from the current
>> +sources and then tested.
>> +
>> +Either way, this test initializes, starts, and stops a test Postgres
>> +cluster.
>> +
>> +See src/test/perl/README for more info about running these tests.
> 
> Is it really useful to have such instructions all over the tree?

That's debatable but I didn't want to go down that rabbit hole with this 
patch.

It's repetitive for sure. But there are small variations in which 
PG_TEST_EXTRA options you need, whether "make installcheck" runs against 
a running server or still creates a temporary cluster, etc.

I tried to deduplicate those instructions by moving the above 
boilerplate to src/test/README, and only noting the variations in the 
subdirectory READMEs. I didn't like the result. It's very helpful to 
have full copy-pasteable commands with all the right "PG_TEST_EXTRA" 
options for each test.

These instructions also don't mention how to run the tests with Meson. 
The first time I wanted to run individual tests with Meson, it took me a 
while to figure it out.

I'll think a little more about how to improve these READMEs, but let's 
take that to a separate thread.

>>  From 93b9e9b6e072f63af9009e0d66ab6d0d62ea8c15 Mon Sep 17 00:00:00 2001
>> From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
>> Date: Mon, 12 Aug 2024 10:55:11 +0300
>> Subject: [PATCH v4 2/8] Add test for dead-end backends
>>
>> The code path for launching a dead-end backend because we're out of
>> slots was not covered by any tests, so add one. (Some tests did hit
>> the case of launching a dead-end backend because the server is still
>> starting up, though, so the gap in our test coverage wasn't as big as
>> it sounds.)
>> ---
>>   src/test/perl/PostgreSQL/Test/Cluster.pm      | 39 +++++++++++++++++++
>>   .../postmaster/t/001_connection_limits.pl     | 17 +++++++-
>>   2 files changed, 55 insertions(+), 1 deletion(-)
> 
> Why does this need to use "raw" connections?  Can't you just create a bunch of
> connections with BackgroundPsql?

No, these need to be connections that haven't sent the startup packet 
the yet.

With Andrew's PqFFI work [1], we could do better. The latest version on 
that thread doesn't expose the async functions like PQconnectStart() 
PQconnectPoll() though, but they can be added.

[1] 
https://www.postgresql.org/message-id/97d1d1b9-d147-f69d-1991-d8794efed41c%40dunslane.net


Unless you have comments on these first two patches which just add 
tests, I'll commit them shortly. Still processing the rest of your 
comments...

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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix test name and username used in failed connection attempts

  2. tests: Don't fail due to high default timeout in postmaster/003_start_stop

  3. tests: Fix race condition in postmaster/002_connection_limits

  4. Pass MyPMChildSlot as an explicit argument to child process

  5. Assign a child slot to every postmaster child process

  6. Kill dead-end children when there's nothing else left

  7. Replace postmaster.c's own backend type codes with BackendType

  8. Use an shmem_exit callback to remove backend from PMChildFlags on exit

  9. Add test for dead-end backends

  10. Add test for connection limits

  11. Fix garbled process name on backend crash

  12. Consolidate postmaster code to launch background processes

  13. Fix comment on processes being kept over a restart

  14. Refactor code to handle death of a backend or bgworker in postmaster

  15. Make BackgroundWorkerList doubly-linked

  16. Minor refactoring of assign_backendlist_entry()

  17. Fix outdated comment; all running bgworkers are in BackendList

  18. Allow building with MSVC and Strawberry perl

  19. Fix rare failure in LDAP tests.

  20. Allow notifications to bgworkers without database connections.