public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: [email protected]
Cc: Peter Geoghegan <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: Re: Don't synchronously wait for already-in-progress IO in read stream
Date: Tue, 31 Mar 2026 14:49:19 -0400
Message-ID: <a4t7ztahas3sp5hu5os2bd32u3o2idz6gp6zkhford4zt53n7q@7gp4gtkb36ky> (raw)
In-Reply-To: <CAAKRu_YGG5ABz5xbd1jqSNkAs1EAbFOFibuO6EULYWhy9MDwuw@mail.gmail.com>
References: <CAAKRu_a8htJqvgMAMtA54zX35EZKhx3j7nLDXhksbciCxiNaJQ@mail.gmail.com>
	<wmqrglrv4eq2m2kzgbjbtmmuctklg7q4ylwo22rks7r7mlbf3w@dn6txz6znmtr>
	<us6m5jdjxmveyan77tdg2tysjls4yu3tsohpvjjedz6pnjkyn7@2g6b2asvgug3>
	<CAAKRu_Zp8qY1hYxRp-XLQi=3Dff=j5VmS8b_KJu+MSGDHnbP-A@mail.gmail.com>
	<bhbitjpkaosi75kxqkemt7257rbz67j5vqjp4ya73uwl5zonnx@zz6z4vzqd7zh>
	<[email protected]>
	<CAAKRu_Y2NeYkqf1wymzErYt9aWFXU5ebUSi2xWfsv3wY4HMEbw@mail.gmail.com>
	<CAAKRu_ZRf_t4=5y5nvq3BKoN8=8Dh77O0pi6s2kScXiAnA3eVQ@mail.gmail.com>
	<6yhrh2q7tb2fxszsyjg34uzt66ejbbk4j6tmwov6xaezunzvxx@ghohuopucefv>
	<CAAKRu_YGG5ABz5xbd1jqSNkAs1EAbFOFibuO6EULYWhy9MDwuw@mail.gmail.com>

Hi,

On 2026-03-31 14:25:49 -0400, Melanie Plageman wrote:
> On Tue, Mar 31, 2026 at 8:43 AM Andres Freund <[email protected]> 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





view thread (31+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Don't synchronously wait for already-in-progress IO in read stream
  In-Reply-To: <a4t7ztahas3sp5hu5os2bd32u3o2idz6gp6zkhford4zt53n7q@7gp4gtkb36ky>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox