Threading in BGWorkers (!)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
26 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

James Sewell
We need some kind of concrete plan for what is a
usable amount of functionality and what has to be done to get it.

This is exactly the type of discussion I'm after. 

I'll start.

A usable amount of functionality would be the ability to start threads from:
  • a background worker
These cases should be bound by *at least* the following rules:
  • no signal handling from threads
  • no calls into PostgreSQL functions from threads

The patch I supplied is one of the requirements to get there - I would love help to discover the others.


The contents of this email are confidential and may be subject to legal or professional privilege and copyright. No representation is made that this email is free of viruses or other defects. If you have received this communication in error, you may not copy or distribute any part of it or otherwise disclose its contents to anyone. Please advise the sender of your incorrect receipt of this correspondence.
Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

Andres Freund
In reply to this post by Thomas Munro-5
Hi,

On 2020-07-29 13:41:02 +1200, Thomas Munro wrote:
> One practical problem with this change is that some systems have a
> stub definition of pthread_sigmask() that does nothing, when you don't
> link against the threading library.  Realistically, most *useful*
> builds of PostgreSQL bring it in indirectly (via SSL, LDAP, blah
> blah), but it so happens that a bare bones build and make check on
> this here FreeBSD box hangs in CHECK DATABASE waiting for the
> checkpointer to signal it.  I can fix that by putting -lthr into
> LDFLAGS, so we'd probably have to figure out how to do that on our
> supported systems.

Couldn't this be driven by --disable-thread-safety?

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

Andres Freund
In reply to this post by Tom Lane-2
Hi,

On 2020-07-28 21:52:20 -0400, Tom Lane wrote:
> >> The rationale here is that as far as I can tell this is the *only* blocker to using multithreaded code in a BGWorker which can't be avoided by adhering to strict code rules (eg: no PG calls from non-main threads, no interaction with signals from non-main threads).
>
> TBH, though, I do not buy this argument for a millisecond.  I don't
> think that anything is going to come out of multithreading a bgworker
> but blood and tears.  Perhaps someday we'll make a major push to
> make the backend code (somewhat(?)) thread safe ... but I'm not on
> board with making one-line-at-a-time changes in hopes of getting
> partway there.  We need some kind of concrete plan for what is a
> usable amount of functionality and what has to be done to get it.

Why not? Our answer to threading inside functions has been, for quite a
while, that it's kinda ok if the threads never call into postgres and
can never escape the lifetime of a function. But that's not actually
really safe due to the signal handler issue. Whether it's a normal
backend or a bgworker doesn't really play a role here, no?

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

Tom Lane-2
Andres Freund <[hidden email]> writes:
> On 2020-07-28 21:52:20 -0400, Tom Lane wrote:
>> TBH, though, I do not buy this argument for a millisecond.  I don't
>> think that anything is going to come out of multithreading a bgworker
>> but blood and tears.  Perhaps someday we'll make a major push to
>> make the backend code (somewhat(?)) thread safe ... but I'm not on
>> board with making one-line-at-a-time changes in hopes of getting
>> partway there.  We need some kind of concrete plan for what is a
>> usable amount of functionality and what has to be done to get it.

> Why not? Our answer to threading inside functions has been, for quite a
> while, that it's kinda ok if the threads never call into postgres and
> can never escape the lifetime of a function. But that's not actually
> really safe due to the signal handler issue.

In other words, it's *not* safe and never has been.  I see no good reason
to believe that the signal handler issue is the only one.  Even if it is,
not being able to call any postgres infrastructure is a pretty huge
handicap.

So I stand by the position that we need an actual plan here, not just
chipping away at one-liner things that might or might not improve
matters noticeably.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

Robert Haas
On Thu, Jul 30, 2020 at 2:55 PM Tom Lane <[hidden email]> wrote:
> > Why not? Our answer to threading inside functions has been, for quite a
> > while, that it's kinda ok if the threads never call into postgres and
> > can never escape the lifetime of a function. But that's not actually
> > really safe due to the signal handler issue.
>
> In other words, it's *not* safe and never has been.  I see no good reason
> to believe that the signal handler issue is the only one.  Even if it is,
> not being able to call any postgres infrastructure is a pretty huge
> handicap.

I find this line of argument really unfair. It's true that there might
be issues other than the signal handler one, but so what? That is not
a principled argument against fixing the signal handler part of the
problem, which is the only *known* problem with the use case Andres
describes. It is also true that it would be more useful to enable
additional use cases rather than just this one, but that is not a
principled argument against enabling this one.

My only present concern about the proposal actually in front of us --
that is to say, use pthread_sigmask() rather than sigprocmask() -- is
Thomas's observation that on his system doing so breaks the world.
That seems to be quite a serious problem. If we are deciding whether
to use one function or another some purpose and they are equally good
for the core code but one is better for people who want to play around
with extensions, we may as well use the one that's better for that
purpose. We need not give such experimentation our unqualified
endorsement; we need only decide against obstructing it unnecessarily.
But when such a substitution risks breaking things that work today,
the calculus gets a lot more complicated. Unless we can find a way to
avoid that risk, I don't think this is a good trade-off.

But more broadly I think it is well past time that we look into making
the backend more thread-friendly. The fact that "it's *not* safe and
never has been" has not prevented people from doing it. We don't end
up with people going "oh, well sigprocmask uh oh so I better give up
now." What we end up with is people just going right ahead and doing
it, probably without even thinking about sigprocmask, and ending up
with low-probability failure conditions, which seems likely to hurt
PostgreSQL's reputation for reliability with no compensating benefit.
Or alternatively they hack core, which sucks, or they switch to some
non-PG database, which also sucks.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Reply | Threaded
Open this post in threaded view
|

Re: Threading in BGWorkers (!)

James Sewell-2
In reply to this post by Tom Lane-2
I see no good reason to believe that the signal handler issue is the only one.  Even if it is,
not being able to call any postgres infrastructure is a pretty huge
handicap.

(changed emails to get rid of the nasty employer notice...)

It's at least a workable handicap that I'm happy to deal with.

I can say with 100% confidence that people coming from non C languages will be using threading in Postgres backends as interop matures (and it's happening fast now). A lot of the time they won't even know they are using threads as it will be done by libraries they make use of transparently. 

Let's help them to avoid unsafe code now, not wait until they show up on this list with a critical failure and tap at the big sign that says "NO THREADING".

- james
12