Re: 2pc leaks fds

Michael Paquier <michael@paquier.xyz>

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

Attachments

On Sun, Apr 05, 2020 at 07:56:51PM -0700, Andres Freund wrote:
> I found this while trying to benchmark the effect of my snapshot changes
> on 2pc. I just used the attached pgbench file.
> 
> I've not yet reviewed the change sufficiently to pinpoint the issue.

Indeed.  It takes seconds to show up.

> It's a bit sad that nobody has hit this in the last few months :(.

2PC shines with the code of xlogreader.c in this case because it keeps
opening and closing XLogReaderState for a short amount of time.  So it
is not surprising to me to see this error only months after the fact
because recovery or pg_waldump just use one XLogReaderState.  From
what I can see, the error is that the code only bothers closing
WALOpenSegment->seg when switching to a new segment, but we need also
to close it when finishing the business in XLogReaderFree().

I am adding an open item, and attached is a patch to take care of the
problem.  Thoughts?
--
Michael

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