From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: connection establishment versus parallel workers |
Date: | 2024-12-13 02:56:00 |
Message-ID: | CA+hUKG+0Su_d71N3vL9K=JpEHm=FzjZYpNntwAdHi+Qsfbimhg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Dec 13, 2024 at 11:00 AM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
> On Fri, Dec 13, 2024 at 02:29:53AM +1300, Thomas Munro wrote:
> > Here's an experimental patch to try changing that policy. It improves
> > the connection times on my small computer with your test, but I doubt
> > I'm seeing the real issue. But in theory, assuming a backlog of
> > connections and workers to start, I think each server loop should be
> > able to accept and fork one client backend, and fork up to 100
> > (MAX_BGWORKERS_TO_LAUNCH) background workers.
>
> Thanks for the quick response! I'm taking a look at the patch...
After sleeping on the let-a-hundred-workers-fork logic, I dug out
aa1351f1eec4 (2017) and read a couple of interesting things:
One could also question the wisdom of using an O(N^2) processing
method if the system is intended to support so many bgworkers.
So that's about the linear search for workers to queue up for
starting, in BackgroundWorkerStateChange(). Hmm. I suppose we should
only really process PM signals when we see WL_LATCH_SET, or I think
we'd run BackgroundWorkerStateChange() twice as often as before, when
the 0001 patch feeds you WL_LATCH_SET and WL_SOCKET_ACCEPT at the same
time, under your backlog-of-all-kinds-of-events workload. Maybe it
doesn't really matter much, but it'd be an unintended behavioural
change, so here is a patch. You could call it a revert of 1/4 of
239b1753. I wonder if that has any problematic side effects for other
PM signal kinds, but I'm looking and I can't see them. And:
There is talk of rewriting the postmaster to use a WaitEventSet to
avoid the signal-response-delay problem, but I'd argue that this change
should be kept even after that happens (if it ever does).
It did happen, and we did keep that. Someone might want to research
that policy, which gives massive preference to workers over clients
(the 100 is more of a cap, in practice it means "start all pending
workers", a bit like how child exit means "reap all dead children",
while we only accept one socket per server socket per server loop),
but no one has complained in half a decade, until, if I intuited
correctly, this report of what I assume is the unbounded socket event
starvation state. On the other hand, you only said that v16 was worse
than v15, not that v15 was actually good! The workload could easily
have been suffering from undiagnosed and unwanted massive bias on v15
already. And starting a lot of workers might be an even more serious
issue if your system is very slow at forking, or if your system
doesn't even have fork() and if PostgreSQL doesn't have threads.
But like 04a09ee9, for me this is not really about a resource
allocation policy that we could debate and perhaps adjust separately.
The main question is whether complete starvation (DOS) is possible, or
whether the postmaster should always try to service all of its
conceptual work queues within bounded delays, no matter what is
happening on the others. I think it should.
0001 patch is unchanged, 0002 patch sketches out a response to the
observation a couple of paragraphs above.
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Latches-shouldn-t-starve-socket-event-delivery.patch | text/x-patch | 4.3 KB |
v2-0002-Adjust-pending_pm_pmsignal-processing-priority.patch | text/x-patch | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-12-13 03:03:45 | Re: confusing / inefficient "need_transcoding" handling in copy |
Previous Message | Andrei Lepikhov | 2024-12-13 02:51:58 | Re: Add Postgres module info |