Feedback-ID: id4a34324:Fastmail
Date: Tue, 31 Mar 2026 14:49:19 -0400
From: Andres Freund <andres@anarazel.de>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>,
	Nazir Bilal Yavuz <byavuz81@gmail.com>,
 Thomas Munro <thomas.munro@gmail.com>,
	pgsql-hackers@postgresql.org, Peter Geoghegan <pg@bowt.ie>,
 Tomas Vondra <tv@fuzzy.cz>
Subject: Re: Don't synchronously wait for already-in-progress IO in read
 stream
Message-ID: <a4t7ztahas3sp5hu5os2bd32u3o2idz6gp6zkhford4zt53n7q@7gp4gtkb36ky>
References: 
 <CAAKRu_a8htJqvgMAMtA54zX35EZKhx3j7nLDXhksbciCxiNaJQ@mail.gmail.com>
 <wmqrglrv4eq2m2kzgbjbtmmuctklg7q4ylwo22rks7r7mlbf3w@dn6txz6znmtr>
 <us6m5jdjxmveyan77tdg2tysjls4yu3tsohpvjjedz6pnjkyn7@2g6b2asvgug3>
 <CAAKRu_Zp8qY1hYxRp-XLQi=3Dff=j5VmS8b_KJu+MSGDHnbP-A@mail.gmail.com>
 <bhbitjpkaosi75kxqkemt7257rbz67j5vqjp4ya73uwl5zonnx@zz6z4vzqd7zh>
 <9ce8f5d8-8ab2-4aa2-b062-c5d74161069c@gmail.com>
 <CAAKRu_Y2NeYkqf1wymzErYt9aWFXU5ebUSi2xWfsv3wY4HMEbw@mail.gmail.com>
 <CAAKRu_ZRf_t4=5y5nvq3BKoN8=8Dh77O0pi6s2kScXiAnA3eVQ@mail.gmail.com>
 <6yhrh2q7tb2fxszsyjg34uzt66ejbbk4j6tmwov6xaezunzvxx@ghohuopucefv>
 <CAAKRu_YGG5ABz5xbd1jqSNkAs1EAbFOFibuO6EULYWhy9MDwuw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAAKRu_YGG5ABz5xbd1jqSNkAs1EAbFOFibuO6EULYWhy9MDwuw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/a4t7ztahas3sp5hu5os2bd32u3o2idz6gp6zkhford4zt53n7q%407gp4gtkb36ky>
Precedence: bulk

Hi,

On 2026-03-31 14:25:49 -0400, Melanie Plageman wrote:
> On Tue, Mar 31, 2026 at 8:43 AM Andres Freund <andres@anarazel.de> wrote:
> >
> > Looks good to me.
> >
> > Will you push?
> 
> I was going to push but then Bilal asked me off-list if there was some
> reason not to set the members of ReadBuffersOperation outside of
> assert builds. I agree with him that it seems like a future user of
> StartReadBuffersImpl() could make this same mistake. Both of us
> vaguely recall this being done for performance reasons. Before
> committing this test change, I wanted to confirm that we don't want to
> modify the actual prod code the way he does in [1].

I'd be wary of doing that without performance validation. My memory of the
read stream introduction is that it was pretty hard to not regress the fully
cached path, and that relatively small additions showed up.  But I do agree
it'd be nicer if they were valid.

So I'd be inclined to push your fix for now.

Greetings,

Andres Freund