From: Antonin Houska <ah@cybertec.at>
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: 2pc leaks fds
Date: Wed, 08 Apr 2020 10:00:21 +0200
Message-ID: <70539.1586332821@antos> (raw)
In-Reply-To: <20200408001249.2ma2x6s54xcegiu2@alap3.anarazel.de>
References: <20200406025651.fpzdb5yyb7qyhqko@alap3.anarazel.de>
<20200406052648.GB2066@paquier.xyz>
<20200406054449.ssmrzj4fav3ewvaq@alap3.anarazel.de>
<26247.1586157152@antos>
<20200408001249.2ma2x6s54xcegiu2@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> wrote:
> On 2020-04-06 09:12:32 +0200, Antonin Houska wrote:
> > Andres Freund <andres@anarazel.de> wrote:
> > 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).
> >
> > I was aware of the problem that the correct function should be used to open
> > the file and that's why this comment was added (although "mandatory" would be
> > more suitable than "preferred"):
> >
> > * BasicOpenFile() is the preferred way to open the segment file in backend
> > * code, whereas open(2) should be used in frontend.
> > */
> > typedef int (*WALSegmentOpen) (XLogSegNo nextSegNo, WALSegmentContext *segcxt,
> > TimeLineID *tli_p);
>
> I don't think that BasicOpenFile() really solves anything here? If
> anything it *exascerbates* the problem, because it will trigger all of
> the "virtual file descriptors" for already opened Files to close() the
> underlying OS FDs. So not even a fully cached table can be seqscanned,
> because that tries to check the file size...
Specifically for 2PC, isn't it better to close some OS-level FD of an
unrelated table scan and then succeed than to ERROR immediately? Anyway,
0dc8ead46 hasn't changed this.
I at least admit that the comment should not recommend particular function,
and that WALRead() should call the appropriate function to close the file,
rather than always calling close().
Can we just pass the existing FD to the callback as an additional argument,
and let the callback close it?
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, andres@anarazel.de, michael@paquier.xyz, alvherre@2ndquadrant.com
Subject: Re: 2pc leaks fds
In-Reply-To: <70539.1586332821@antos>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox