Re: postgres_fdw uninterruptible during connection establishment / ProcSignalBarrier - Mailing list pgsql-hackers

From Andres Freund
Subject Re: postgres_fdw uninterruptible during connection establishment / ProcSignalBarrier
Date
Msg-id 20230121030008.acoon7nk7gnuslx4@awork3.anarazel.de
Whole thread Raw
In response to Re: postgres_fdw uninterruptible during connection establishment / ProcSignalBarrier  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: postgres_fdw uninterruptible during connection establishment / ProcSignalBarrier
List pgsql-hackers
Hi,

On 2023-01-14 14:45:06 +1300, Thomas Munro wrote:
> On Tue, Jan 10, 2023 at 3:05 PM Andres Freund <andres@anarazel.de> wrote:
> > On 2023-01-03 12:05:20 -0800, Andres Freund wrote:
> > > The attached patch adds libpq-be-fe-helpers.h and uses it in libpqwalreceiver,
> > > dblink, postgres_fdw.
> >
> > > As I made libpq-be-fe-helpers.h handle reserving external fds,
> > > libpqwalreceiver now does so. I briefly looked through its users without
> > > seeing cases of leaking in case of errors - which would already have been bad,
> > > since we'd already have leaked a libpq connection/socket.
> > >
> > >
> > > Given the lack of field complaints and the size of the required changes, I
> > > don't think we should backpatch this, even though it's pretty clearly buggy
> > > as-is.
> >
> > Any comments on this? Otherwise I think I'll go with this approach.
> 
> +1.  Not totally convinced about the location but we are free to
> re-organise it any time, and the random CI failures are bad.

Cool.

Updated patch attached. I split it into multiple pieces.
1) A fix for [1], included here because I encountered it while testing
2) Introduction of libpq-be-fe-helpers.h
3) Convert dblink and postgres_fdw to the helper
4) Convert libpqwalreceiver.c to the helper

Even if we eventually decide to backpatch 3), we'd likely not backpatch 4), as
there's no bug (although perhaps the lack of FD handling could be called a
bug?).

There's also some light polishing, improving commit message, comments and
moving some internal helper functions to later in the file.


Greetings,

Andres Freund

[1] https://postgr.es/m/20230121011237.q52apbvlarfv6jm6%40awork3.anarazel.de

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: libpqrcv_connect() leaks PGconn
Next
From: Tom Lane
Date:
Subject: Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?