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

Xuneng Zhou <xunengzhou@gmail.com>

From: Xuneng Zhou <xunengzhou@gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org, Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>, nathandbossart@gmail.com
Date: 2025-08-13T05:29:14Z
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.

Hi,

>  1. The leading block is BM_VALID, so we choose to give it to you
> immediately and not look further (we could look further and return
> more than one consecutive BM_VALID block at a time, but this isn't
> implemented)

I am curious about why this isn't implemented. It looks helpful.
Is there any blocking issue or trade-off for not doing so?

> Next, let me describe how read_stream.c manages its circular buffer queue:
>
> |<------------- queue_size ------------>|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | | | | | | |0|1|2|3|4|5|6|7|8| | | | | |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>              ^                 ^
>              |                 |
>     oldest_buffer_index next_buffer_index
>
> oldest_buffer_index is the consumer end of the queue, and any
> associated IO has to be finished before that buffer can be return by
> read_stream_next_buffer().  next_buffer_index is the producer end of
> the queue, where IOs are started.  StartReadBuffers() takes pointer to
> an array of buffers of the size of the requested read, so
> read_stream.c gives it the address of that part of its queue.
>
> This means that if StartReadBuffer() forwards some buffers because of
> a split, we don't have to do anything at all, because we advance
> next_buffer_index to the location of the start of the next operation,
> and that is where the forwarded buffers landed, ready for the next
> call to StartReadBuffers():
>
> |<------------- queue_size ------------>|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | | | | | | |0|1|2|3|4|5|6|7|8|9|A| | | |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>              ^                 ^
>              |                 |
>     oldest_buffer_index next_buffer_index
>

The format of this part is not aligned well in gmail, so I copy it into vs
code.
Is this layout right? I found second illustration somewhat hard to follow,
especially
the "do nothing" trick and the movement of next_buffef_index in the second
queue.
Maybe I need to read the corresponding code.
[image: image.png]

Best,
Xuneng