Re: 2pc leaks fds

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