Re: 2pc leaks fds

Antonin Houska <ah@cybertec.at>

From: Antonin Houska <ah@cybertec.at>
To: Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>, pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Date: 2020-04-08T07:26:37Z
Lists: pgsql-hackers
Antonin Houska <ah@cybertec.at> wrote:

> Andres Freund <andres@anarazel.de> wrote:
> > But I'm not sure it's quite the right idea. I'm not sure I fully
> > understand the design of 0dc8ead46, but it looks to me like it's
> > intended to allow users of the interface to have different ways of
> > opening files. If we just close() the fd that'd be a bit more limited.
> 
> It should have allowed users to have different ways to *locate the segment*
> file. The WALSegmentOpen callback could actually return file path instead of
> the file descriptor and let WALRead() perform the opening/closing, but then
> the WALRead function would need to be aware whether it is executing in backend
> or in frontend (so it can use the correct function to open/close the file).

Well, #ifdef FRONTEND can be used to distinguish the caller of
WALRead(). However now that I tried to adjust the API, I see that
pg_waldump.c:WALDumpOpenSegment uses specific logic to open the file. So if
the callback only returned the file name, there would be no suitable place for
the things that WALDumpOpenSegment does.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com



Commits

  1. Fix obsolete references to "XLogRead"

  2. Rework XLogReader callback system

  3. Fix XLogReader FD leak that makes backends unusable after 2PC usage.

  4. Refactor WAL file-reading code into WALRead()

  5. Add ArchiveOpts to pass options to ArchiveEntry