Re: 2pc leaks fds

From: Andres Freund <andres(at)anarazel(dot)de>
To: Antonin Houska <ah(at)cybertec(dot)at>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: 2pc leaks fds
Date: 2020-04-08 00:12:49
Message-ID: 20200408001249.2ma2x6s54xcegiu2@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2020-04-08 00:21:00 Re: Make MemoryContextMemAllocated() more precise
Previous Message James Coleman 2020-04-08 00:04:46 Re: [PATCH] Incremental sort (was: PoC: Partial sort)