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

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Refactoring postmaster's code to cleanup after child exit
Date: 2024-08-01 23:57:18
Message-ID: 8f2118b9-79e3-4af7-b2c9-bd5818193ca4@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I committed the first two trivial patches, and have continued to work on
postmaster.c, and how it manages all the child processes.

This is a lot of patches. They're built on top of each other, because
that's the order I developed them in, but they probably could be applied
in different order too. Please help me by reviewing these, before the
stack grows even larger :-). Even partial reviews would be very helpful.
I suggest to start reading them in order, and when you get tired, just
send any comments you have up to that point.

* v3-0001-Make-BackgroundWorkerList-doubly-linked.patch

This is the same refactoring patch I started this thread with.

* v3-0003-Fix-comment-on-processes-being-kept-over-a-restar.patch
* v3-0004-Consolidate-postmaster-code-to-launch-background-.patch

Little refactoring of how postmaster launches the background processes.

* v3-0005-Add-test-for-connection-limits.patch
* v3-0006-Add-test-for-dead-end-backends.patch

A few new TAP tests for dead-end backends and enforcing connection
limits. We didn't have much coverage for these before.

* v3-0007-Use-an-shmem_exit-callback-to-remove-backend-from.patch
* v3-0008-Introduce-a-separate-BackendType-for-dead-end-chi.patch

Some preliminary refactoring towards patch
v3-0010-Assign-a-child-slot-to-every-postmaster-child-pro.patch

* v3-0009-Kill-dead-end-children-when-there-s-nothing-else-.patch

I noticed that we never send SIGTERM or SIGQUIT to dead-end backends,
which seems silly. If the server is shutting down, dead-end backends
might prevent the shutdown from completing. Dead-end backends will
expire after authentication_timoeut (default 60s), so it won't last for
too long, but still seems like we should kill dead-end backends if
they're the only children preventing shutdown from completing.

* 3-0010-Assign-a-child-slot-to-every-postmaster-child-pro.patch

This is what I consider the main patch in this series. Currently, only
regular backens, bgworkers and autovacuum workers have a PMChildFlags
slot, which is used to detect when a postmaster child exits in an
unclean way (in addition to the exit code). This patch assigns a child
slot for all processes, except for dead-end backends. That includes all
the aux processes.

While we're at it, I created separate pools of child slots for different
kinds of backends, which fixes the issue that opening a lot of client
connections can exhaust all the slots, so that background workers or
autovacuum workers cannot start either [1].

[1]
https://www.postgresql.org/message-id/55d2f50c-0b81-4b33-b202-cd2a406d69a3%40iki.fi

* v3-0011-Pass-MyPMChildSlot-as-an-explicit-argument-to-chi.patch

One more little refactoring, to pass MyPMChildSlot to the child process
differently.

Where is all this leading? I'm not sure exactly, but having a postmaster
child slot for every postmaster child seems highly useful. We could move
the ProcSignal machinery to use those slot numbers for the indexes to
the ProcSignal array, instead of ProcSignal, for example. That would
allow all processes to participate in the signalling, even before they
have a PGPROC entry. (Or with Thomas's interrupts refactoring, the
interrupts array). With the multithreading work, PMChild struct could
store a thread id, or whatever is needed for threads to communicate with
each other. In any case, seems like it will come handy.

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

Attachment Content-Type Size
v3-0001-Make-BackgroundWorkerList-doubly-linked.patch text/x-patch 13.2 KB
v3-0002-Refactor-code-to-handle-death-of-a-backend-or-bgw.patch text/x-patch 19.8 KB
v3-0003-Fix-comment-on-processes-being-kept-over-a-restar.patch text/x-patch 2.1 KB
v3-0004-Consolidate-postmaster-code-to-launch-background-.patch text/x-patch 13.4 KB
v3-0005-Add-test-for-connection-limits.patch text/x-patch 6.4 KB
v3-0006-Add-test-for-dead-end-backends.patch text/x-patch 3.2 KB
v3-0007-Use-an-shmem_exit-callback-to-remove-backend-from.patch text/x-patch 4.9 KB
v3-0008-Introduce-a-separate-BackendType-for-dead-end-chi.patch text/x-patch 14.7 KB
v3-0009-Kill-dead-end-children-when-there-s-nothing-else-.patch text/x-patch 7.5 KB
v3-0010-Assign-a-child-slot-to-every-postmaster-child-pro.patch text/x-patch 64.8 KB
v3-0011-Pass-MyPMChildSlot-as-an-explicit-argument-to-chi.patch text/x-patch 10.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-08-02 01:02:46 Re: can we mark upper/lower/textlike functions leakproof?
Previous Message Tom Lane 2024-08-01 23:44:31 Re: Casts from jsonb to other types should cope with json null