Re: GetBufferDescriptor() being called for local buffers from MarkBufferDirtyHint()
Ayush Tiwari <ayushtiwari.slg01@gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>,
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-06-29T07:59:09Z
Lists: pgsql-hackers
Attachments
- v1-0001-Make-buffer-descriptor-accessors-take-signed-int-.patch (application/octet-stream) patch v1-0001
Hi, On Thu, 11 Jun 2026 at 03:44, Michael Paquier <michael@paquier.xyz> wrote: > On Wed, Jun 10, 2026 at 10:36:22AM -0400, Andres Freund wrote: > > I think it *should* blow up. It doesn't because we're lacking assertions > in > > GetBufferDescriptor(). But I don't think the assertions added in the > patch are > > quite right. > > > > We can't trivially add the correct assertions, because somebody though > it was > > a good idea to give GetBufferDescriptor() a uint32 parameter, which seems > > completely wrong to me. > > This one is not as old as I expected: 3ac88fddd92c. You're right that > switching that to be signed would be a correct first step forward. > Given the discussion here, I tried making the buffer descriptor accessors take a signed index and adding range assertions. Most callers already pass signed buffer indexes derived from Buffer values (e.g. buffer - 1, or -buffer - 1 for local buffers). The only caller I found that passes an unsigned value to GetBufferDescriptor() is ClockSweepTick(), and that normalizes the value before returning it, so it should still be less than NBuffers. The change looked fairly straightforward to me, unless I'm missing something. I also made the same change for GetLocalBufferDescriptor(), since it has the same uint32 signature and takes local buffer indexes. Thoughts? Regards, Ayush
Commits
-
Switch Get[Local]BufferDescriptor() to use a signed value in input
- ba4134075a82 master landed
-
Fix MarkBufferDirtyHint() to not call GetBufferDescriptor() for local buffers
- e18b0cb7344c 19 (unreleased) landed
-
Require share-exclusive lock to set hint bits and to flush
- 82467f627bd4 19 (unreleased) cited