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