Re: Some read stream improvements

Hi,

On 2025-03-14 22:03:15 +1300, Thomas Munro wrote:
> I have pushed the new pin limit patches, after some more testing and
> copy editing. I dropped an unnecessary hunk (in read_stream_reset(), a
> change I'd like to make but it didn't belong in this commit) and
> dropped the word "Soft" from GetSoftPinLimit() as it wasn't helping
> comprehension and isn't even true for the local buffer version (which
> I still think might be an issue, will come back to that, but again it
> seemed independent).

Something odd:

I was looking to push a test that increases localbuf.c coverage, which passed
with a previous version of these changes. However, it failed, and it did so
on FreeBSD alone. The same test doesn't fail when tested together with the
rest of the AIO work.

https://cirrus-ci.com/build/5951090857869312
https://cirrus-ci.com/task/6177637229395968

I do not understand what could be OS dependent here. I tried to build with
exactly the same options as CI on freebsd, but couldn't repro the issue.

It's perhaps worth noting that this failed even before my recent localbuf:
commits.

I ran CI with a patch that adds a bunch of debug information and just runs the
relevant test:
https://cirrus-ci.com/task/6516935619248128

2025-03-17 16:19:14.270 UTC client backend[5526] pg_regress/temp LOG: statement: SELECT count(*), max(a) max_a, min(a) min_a, max(cnt) max_cnt FROM test_temp;
2025-03-17 16:19:14.271 UTC client backend[5526] pg_regress/temp DEBUG: NlocalPinnedBuffers=98++
2025-03-17 16:19:14.271 UTC client backend[5526] pg_regress/temp DEBUG: NlocalPinnedBuffers=99--
2025-03-17 16:19:14.271 UTC client backend[5526] pg_regress/temp DEBUG: pgsr create 0xde34f1f57f8: test_temp, max_pinned: 100, NLocPin: 98
2025-03-17 16:19:14.271 UTC client backend[5526] pg_regress/temp DEBUG: pgsr 0xde34f1f57f8: buffer_limit: 2, pinned_buffers: 0, max_pinned: 100, ios_i_p: 0, distance: 1, pending_read_nblocks: 1, NLocPin: 98

comparing that with a local run:

2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp LOG: statement: SELECT count(*), max(a) max_a, min(a) min_a, max(cnt) max_cnt FROM test_temp;
2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp DEBUG: pgsr create 0x56029555cad8: test_temp, max_pinned: 100, NLocPin: 97
2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp DEBUG: pgsr 0x56029555cad8: buffer_limit: 3, pinned_buffers: 0, max_pinned: 100, ios_i_p: 0, distance: 1, pending_read_nblocks: 1, NLocPin: 97
2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp DEBUG: pgsr 0x56029555cad8: StartReadBuffers: wait: 0, pinned: 0 += 1, distance: 1
2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp DEBUG: pgsr 0x56029555cad8: buffer_limit: 3, pinned_buffers: 0, max_pinned: 100, ios_i_p: 0, distance: 1, pending_read_nblocks: 1, NLocPin: 97
2025-03-17 12:18:55.989 EDT client backend[4117093] pg_regress/temp DEBUG: pgsr 0x56029555cad8: StartReadBuffers: wait: 0, pinned: 0 += 1, distance: 1

So one thing is that the pin count differs by 1 at the start of the scan. No
idea why.

But knowing that I was able to repro the issue locally too, by just ensuring
the pin count is one higher by the start of the query.

I still don't know what drives the difference between freebsd and the rest,
but IIUC the reason this fails is just that we are holding too many buffers
pinned, due to some buffers being pinned outside of read_stream.c.

Greetings,

Andres Freund