Re: Streamify more code paths

From: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
To: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
Cc: Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Streamify more code paths
Date: 2026-08-04 07:57:47
Message-ID: CABPTF7VfDvwnm=VHnSAR=zRQEYZNBAQmuJ51DXt=c5igep8ePQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Bilal,

On Tue, Aug 4, 2026 at 2:40 PM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Mon, 3 Aug 2026 at 15:25, Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com> wrote:
> >
> > While prototyping with a stress suite, I found memory corruption (and
> > possibly data corruption) issues related to those changes.
> >
> > Issues is related to how the newer bfa3c4f106 affects the old 293e24e507.
> >
> > The 293e24e507 caches the metapage as a pointer, but
> > read_stream_begin_relation may cause that pointer to become freed
> > because of:
> >
> > hashbulkdelete() hash.c:543
> > read_stream_begin_relation(READ_STREAM_MAINTENANCE, ...)
> > read_stream_begin_impl() read_stream.c:806
> > get_tablespace_maintenance_io_concurrency(tablespace_id)
> > get_tablespace() spccache.c
> > SearchSysCache1(TABLESPACEOID, ...) ← cold cache → catalog scan
> > table_open(pg_tablespace)
> > LockRelationOid(1213) lmgr.c:136
> > AcceptInvalidationMessages() ← the flush point
> >
> > causing something like:
> >
> > ERROR: could not open file "base/5/16388.1" (target block 2139062145):
> > previous segment is only 66 blocks
> > CONTEXT: while scanning relation "public.t"
>
> You are right, nice catch!

Thanks for looking into this!

>
> > The simplest fix is to reorder a few lines of code - attached (with a
> > test generated by Claude based on my instructions).
>
> I agree with you, fix LGTM.

After some discussions/debates with Sol, it offers a copying
alternative for fixing the issue:

HashMetaPageData local_metapage;

cachedmetap = _hash_getcachedmetap(rel, ...);
memcpy(&local_metapage, cachedmetap, sizeof(local_metapage));

/* May invalidate and free rd_amcache. */
some_catalog_access();

use(&local_metapage); /* still allocated */

I don't have a strong opinion over them yet. Just putting it on the
table in case the game is over before putting my thoughts together.

> > But probably we should think broader to avoid such issues in the future.
>
> I am not sure how we can enforce this with a code check. Perhaps we
> can add a comment to the read_stream code, but I am not sure how
> effective that would be.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-08-04 08:05:00 Re: Add a hook for handling logical decoding messages on subscribers.
Previous Message Bharath Rupireddy 2026-08-04 07:50:00 Re: enhance wraparound warnings