Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-04-06 Thread Andres Freund
Hi, On 2023-04-03 12:00:30 -0700, Andres Freund wrote: > It's not great, I agree. I tried to make it easier to read in this version by > a) changing GetVisibilityMapPins() as I proposed > b) added a new variable "recheckVmPins", that gets set in >if (unlockedTargetBuffer) >and >if (oth

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-04-03 Thread Andres Freund
Hi, On 2023-04-03 14:25:59 +0200, Tomas Vondra wrote: > On 4/3/23 00:40, Andres Freund wrote: > > Hi, > > > > On 2023-03-28 19:17:21 -0700, Andres Freund wrote: > >> On 2023-03-28 18:21:02 -0700, Andres Freund wrote: > >>> Here's a draft patch. > >> > >> Attached is v2, with a stupid bug fixed and

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-04-03 Thread Tomas Vondra
On 4/3/23 00:40, Andres Freund wrote: > Hi, > > On 2023-03-28 19:17:21 -0700, Andres Freund wrote: >> On 2023-03-28 18:21:02 -0700, Andres Freund wrote: >>> Here's a draft patch. >> >> Attached is v2, with a stupid bug fixed and a bit of comment / pgindent >> polish. > > I'd welcome some review (

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-04-02 Thread Andres Freund
Hi, On 2023-03-28 19:17:21 -0700, Andres Freund wrote: > On 2023-03-28 18:21:02 -0700, Andres Freund wrote: > > Here's a draft patch. > > Attached is v2, with a stupid bug fixed and a bit of comment / pgindent > polish. I'd welcome some review (Tomas?), but otherwise I'm planning to push ahead w

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-28 Thread Andres Freund
Hi, On 2023-03-28 18:21:02 -0700, Andres Freund wrote: > Here's a draft patch. Attached is v2, with a stupid bug fixed and a bit of comment / pgindent polish. Greetings, Andres Freund >From 6f27018765f167a8d2a52bc7c9068cb09ca8d82c Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 24 Oct

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-28 Thread Andres Freund
Hi, On 2023-03-25 11:17:07 -0700, Andres Freund wrote: > I don't see how that's easily possible with the current lock ordering > rules. At least without giving up using RBM_ZERO_AND_LOCK for extending or > stuffing even more things to happen with the the extension lock held, which I > don't think

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-25 Thread Peter Geoghegan
On Sat, Mar 25, 2023 at 11:17 AM Andres Freund wrote: > Thinking more about this, I think there's no inherent deadlock danger with > reading the VM while holding a buffer lock, "just" an efficiency issue. If we > avoid needing to do IO nearly all the time, by trying to pin the right page > before

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-25 Thread Andres Freund
Hi, On 2023-03-25 12:57:17 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2023-03-25 14:34:25 +0100, Tomas Vondra wrote: > >> Can't we ensure we actually lock the vm buffer too in ReadBufferBI, > >> before calling ReadBufferExtended? Or am I confused and that's not > >> possible for some r

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-25 Thread Tom Lane
Andres Freund writes: > On 2023-03-25 14:34:25 +0100, Tomas Vondra wrote: >> Can't we ensure we actually lock the vm buffer too in ReadBufferBI, >> before calling ReadBufferExtended? Or am I confused and that's not >> possible for some reason? > Note that this is using P_NEW. I.e. we don't know t

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-25 Thread Andres Freund
Hi, On 2023-03-25 14:34:25 +0100, Tomas Vondra wrote: > On 3/25/23 03:57, Andres Freund wrote: > > 2) Change relevant code so that we only return a valid vmbuffer if we could > > do > >so without blocking / IO and, obviously, skip updating the VM if we > >couldn't get the buffer. > > >

Re: hio.c does visibilitymap_pin()/IO while holding buffer lock

2023-03-25 Thread Tomas Vondra
On 3/25/23 03:57, Andres Freund wrote: > Hi, > > Starting with > > commit 7db0cd2145f2bce84cac92402e205e4d2b045bf2 > Author: Tomas Vondra > Date: 2021-01-17 22:11:39 +0100 > > Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZE > That's a bummer :-( > RelationGetBufferForTuple do