Feedback-ID: id4a34324:Fastmail
Date: Sun, 2 Apr 2023 15:40:24 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>,
	pgsql-hackers@postgresql.org, Tomas Vondra <tv@fuzzy.cz>,
	Anastasia Lubennikova <lubennikovaav@gmail.com>,
	Pavan Deolasee <pavan.deolasee@gmail.com>,
	Jeff Janes <jeff.janes@gmail.com>
Subject: Re: hio.c does visibilitymap_pin()/IO while holding buffer lock
Message-ID: <20230402224024.aealekgu2df4cnhs@awork3.anarazel.de>
References: <20230325025740.wzvchp2kromw4zqz@awork3.anarazel.de>
 <0c11032a-d91b-8875-ab7b-4554776390c5@enterprisedb.com>
 <20230325163903.eu67kqcqo4llhc4u@awork3.anarazel.de>
 <2602455.1679763437@sss.pgh.pa.us>
 <20230325181707.at3kwsh7uvkhfype@awork3.anarazel.de>
 <20230329012102.zpviu2e7kyfrovxn@awork3.anarazel.de>
 <20230329021721.l3rujmv3jjmktt5i@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230329021721.l3rujmv3jjmktt5i@awork3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/20230402224024.aealekgu2df4cnhs%40awork3.anarazel.de>
Precedence: bulk

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

I'm still debating with myself whether this commit (or a prerequisite commit)
should move logic dealing with the buffer ordering into
GetVisibilityMapPins(), so we don't need two blocks like this:


		if (otherBuffer == InvalidBuffer || targetBlock <= otherBlock)
			GetVisibilityMapPins(relation, buffer, otherBuffer,
								 targetBlock, otherBlock, vmbuffer,
								 vmbuffer_other);
		else
			GetVisibilityMapPins(relation, otherBuffer, buffer,
								 otherBlock, targetBlock, vmbuffer_other,
								 vmbuffer);
...

		if (otherBuffer != InvalidBuffer)
		{
			if (GetVisibilityMapPins(relation, otherBuffer, buffer,
									 otherBlock, targetBlock, vmbuffer_other,
									 vmbuffer))
				unlockedTargetBuffer = true;
		}
		else
		{
			if (GetVisibilityMapPins(relation, buffer, InvalidBuffer,
									 targetBlock, InvalidBlockNumber,
									 vmbuffer, InvalidBuffer))
				unlockedTargetBuffer = true;
		}
	}


Greetings,

Andres Freund