| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
| Cc: | Alexander Lakhin <exclusion(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie>, Tomas Vondra <tv(at)fuzzy(dot)cz> |
| Subject: | Re: Don't synchronously wait for already-in-progress IO in read stream |
| Date: | 2026-03-31 18:49:19 |
| Message-ID: | a4t7ztahas3sp5hu5os2bd32u3o2idz6gp6zkhford4zt53n7q@7gp4gtkb36ky |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-03-31 14:25:49 -0400, Melanie Plageman wrote:
> On Tue, Mar 31, 2026 at 8:43 AM Andres Freund <andres(at)anarazel(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2026-03-31 18:50:02 | Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?) |
| Previous Message | Nathan Bossart | 2026-03-31 18:34:46 | Re: Add pg_stat_autovacuum_priority |