Re: postgres_fdw uninterruptible during connection establishment / ProcSignalBarrier

On Fri, Dec 30, 2022 at 10:54 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2022-12-30 10:31:22 +1300, Thomas Munro wrote:
> > On Fri, Dec 30, 2022 at 6:23 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > On 2022-12-08 18:08:15 -0800, Andres Freund wrote:
> > > > On 2022-09-25 16:22:37 -0700, Andres Freund wrote:
> > > > The only alternative way to provide a wrapper that I can think of are to
> > > > a) introduce a new static library that can be linked to by libpqwalreceiver,
> > > > postgres_fdw, dblink
> > > > b) add a header with static inline functions implementing interrupt-processing
> > > > connection establishment for libpq
> > > >
> > > > Neither really has precedent.
> >
> > > Any opinions? Due to the simplicity I'm currently leaning to a header-only
> > > helper, but I don't feel confident about it.
> >
> > The header idea is a little bit sneaky (IIUC: a header that is part of
> > the core tree, but can't be used by core and possibly needs special
> > treatment in 'headercheck' to get the right include search path, can
> > only be used by libpqwalreceiver et al which are allowed to link to
> > libpq), but I think it is compatible with other goals we have
> > discussed in other threads.
>
> Hm, what special search path / headerscheck magic are you thinking of? I think
> something like src/include/libpq/libpq-be-fe-helpers.h defining a bunch of
> static inlines should "just" work?

Oh, I was imagining something slightly different. Not something under
src/include/libpq, but conceptually a separate header-only library
that is above both the backend and libpq. Maybe something like
src/include/febe_util/libpq_connect_interruptible.h. In other words,
I thought your idea b was a header-only version of your idea a. I
think that might be a bit nicer than putting it under libpq?
Superficial difference, perhaps...

And then I assumed that headerscheck would need to be told to add
libpq's header location in -I for that header, but on closer
inspection it already adds that unconditionally so I retract that
comment.

> > I think in the near future we'll probably remove the concept of non-threaded
> > server builds (as proposed before in the post HP-UX 10 cleanup thread, with
> > patches, but not quite over the line yet). Then I think the server could be
> > allowed to link libpq directly? And at that point this code wouldn't be
> > sneaky anymore and could optionally move into a .c. Does that makes sense?
>
> I was wondering about linking in libpq directly as well. But I am not sure
> it's a good idea. I suspect we'd run into some issues around libraries
> (including extensions) linking to different versions of libpq etc - if we
> directly link to libpq that'd end up in tears.
>
> It might be a different story if we had a version of libpq built with
> different symbol names etc. But that's not exactly trivial either.

Hmm, yeah. Some interesting things to think about. Whether it's a
feature or an accident that new backends can pick up new libpq minor
updates without restarting the postmaster, and how we'd manage a
future libpq major version/ABI break. Getting a bit off topic for
this thread I suppose.