Re: BUG #19006: Assert(BufferIsPinned) in BufferGetBlockNumber() is triggered for forwarded buffer

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Xuneng Zhou <xunengzhou@gmail.com>
Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org, Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2025-08-09T04:03:41Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix rare bug in read_stream.c's split IO handling.

Attachments

On Sat, Aug 9, 2025 at 3:06 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> Well, one benefit of trying to describe a complex system from the top
> is that higher level stupidity can sometimes become clear...  I
> realised that the queue wouldn't even need to be initialised or
> cleared if StartReadBuffers() had an explicit in/out npinned argument
> to transmit the count between calls along with the buffers, or
> examined after each call.  This is a draft alternative fix that I am
> studying that removes quite a lot of lines and fragility, though time
> is not on my side so I might push the one-liner fix ahead of the
> upcoming freeze and then see what to do about this.

One-liner fix pushed.

Here is a new version of the slightly more ambitious fix, for
discussion.  I need to figure out if there is any measurable
performance impact, but from an interface sanity and fragility POV it
seems infinitely better like this.  I recall being paranoid about
function call width, but it still fits in 6 registers.  I think
perhaps the InvalidBuffer-based API might have seemed a shade more
reasonable before I reworked the pin limiting logic, but once I had to
start scanning for forwarded buffers after every StartReadBuffers()
call, I really should have reconsidered that interface.  In other
words, maybe a bit of myopic path dependency here...