Re: pgoutput: schema cache cleanup after streamed 2PC

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: pgoutput: schema cache cleanup after streamed 2PC
Date: 2026-09-18 18:56:26
Message-ID: CAD21AoCoaB_tuGUXXsD1Hhionxmb3aOqNmog6raNLpdP9-am_A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 17, 2026 at 10:20 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Sep 17, 2026 at 6:42 AM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > On Thu, 17 Sept 2026 at 18:22, Hayato Kuroda (Fujitsu)
> > <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> > > Thanks for the quick update. Let me dump my thought in [1] just in case.
> > > Some tests may be needed (no need to include in core though).
> >
> > I used a separate publisher/subscriber test covering both commit and
> > rollback. It confirmed that the cache usage stays flat with the cleanup,
> > I too dont think this explicitly warrants a core test.
> >
> > > > I've moved cleanup_rel_sync_cache(txn->xid, true) to
> > > > pgoutput_stream_prepare_txn() in v2 and updated the comments.
> > >
> > > I think the second argument should be renamed. Do you have anything in your mind?
> > > My idea: mark_schema_sent.
> >
> > Thanks for the suggestion. I initially thought of set_schema_sent, but
> > mark_schema_sent sounds better. I've used that in v3 and updated the nearby
> > comments.
> >
> > > [1]:
> > > IIUC pgoutput_commit_prepared_txn() and pgoutput_rollback_prepared_txn() are used
> > > for both streamed and non-streamed cases. So putting the cleanup for streamed
> > > transactions should be in pgoutput_stream_prepare_txn() as much as possible.
> > >
> > > The main question here is whether we pass true or false for is_commit. I think
> > > true can be used, because no need to re-send RELATION messages once it's handled
> > > by the subscriber side.
> > >
> > > For streaming = on case, an apply worker firstly serialize streamed changes, then
> > > it applies them when STREAM COMMIT or STREAM PREPARE are received. It means
> > > RELATION messages have already handled in PREPARE phase.
> > >
> > > For streaming = parallel case, both leader and parallel apply worker handle
> > > RELATION messages immediately.
> >
> > Thanks for the analysis.
> >
> > Attached v3 with changes.
>
> Thank you for the report and making the patch!
>
> I agree with the analysis and the fix. I've reviewed the v3 patch and
> it looks good to me. IIUC this bug leads to not behavioral problems
> but to memory leaks. I think we can push it without tests. I'm going
> to push it (including back branches), barring any objections.
>

Pushed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Rachitskiy 2026-09-18 19:01:12 Happy Birthday, Tom Lane!
Previous Message Antonin Houska 2026-09-18 18:38:06 Re: REPACK enhancements