Re: 2pc leaks fds - Mailing list pgsql-hackers

From Andres Freund
Subject Re: 2pc leaks fds
Date
Msg-id 20200408001249.2ma2x6s54xcegiu2@alap3.anarazel.de
Whole thread Raw
In response to Re: 2pc leaks fds  (Antonin Houska <ah@cybertec.at>)
Responses Re: 2pc leaks fds  (Michael Paquier <michael@paquier.xyz>)
Re: 2pc leaks fds  (Antonin Houska <ah@cybertec.at>)
List pgsql-hackers
Hi,

I pushed a fix. While it might not be the best medium/long term fix, it
unbreaks 2PC.  Perhaps we should add an open item to track whether we
want to fix this differently?


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...

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: James Coleman
Date:
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Next
From: Jeff Davis
Date:
Subject: Re: Make MemoryContextMemAllocated() more precise