| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Zhijie Hou <houzhijie22(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Add a hook for handling logical decoding messages on subscribers. |
| Date: | 2026-11-10 01:21:32 |
| Message-ID: | CAD21AoC-pOvm54YzNiihFS90kSDRWr=qG6BE-9HW-YW=tqRubw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Nov 9, 2026 at 11:02 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Mon, Nov 2, 2026 at 4:47 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Sat, Oct 31, 2026 at 6:05 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
> > >
> > > On Sat, Oct 31, 2026 at 06:34 AM Masahiko Sawada wrote:
> > > >
> > > > It seems to me worth adding a check for tablesync workers since both
> > > > the apply workers and the tablesync workers call
> > > > walrcv_startstreaming() and it would prevent mistakes by future
> > > > changes.
> > > >
> > > > I've attached the updated patch.
> > >
> > > While reviewing how transactional logical messages interact with
> > > subtransaction aborts, I noticed an issue with streamed transactions. In
> > > non-streaming mode, it works as expected (i.e., the message is not
> > > applied if the subtransaction is aborted). However, in streaming mode,
> > > if a transactional message is emitted in a subtransaction that is later
> > > aborted, the STREAM ABORT message does not discard or roll back that
> > > logical message, so it still gets applied in both streaming=on and
> > > streaming=parallel modes.
> > >
> > > I think the reason is that ReorderBufferApplyMessage() passes the
> > > top-level txn rather than change->txn to rb->stream_message(), so
> > > logical decoding always sends the top-level XID instead of the
> > > subtransaction XID when outputting a streamed message. Thus the apply
> > > worker cannot associate the message with the subtransaction and cannot
> > > roll it back properly.
> > >
> > > Strictly speaking, this is an existing issue, but it becomes more
> > > visible once subscribers can handle logical messages via this hook. So
> > > it may be worth considering whether to fix it along with this patch or
> > > separately.
> > >
> >
> > Right, it didn't have any impact till now for native replication as we
> > were just ignoring these messages on subscribers. I think this could
> > have impacted output-plugins if they used this functionality. I think
> > we can fix this separately but it is better to fix it before this
> > patch as this patch can expose a hidden issue.
>
> Agreed that we should fix it before this patch.
>
> The Hou's patch looks good. I prefer to apply it to only master as
> external output plugin might require changes similar to what the patch
> does for test_decoding.
>
> And thank you for the 0002 patch. I'll include it in the next version patch.
I've attached the new patches. I just updated the commit message for
the 0001 patch, and added some fixes and regression tests for the 0002
patch. Please review them.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Fix-subtransaction-XID-in-streamed-logical-messag.patch | text/x-patch | 5.9 KB |
| v7-0002-Add-a-hook-for-handling-logical-messages-on-subsc.patch | text/x-patch | 121.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | João Rito | 2026-11-10 01:31:55 | Re: [PATCH] reindexdb: Fix schema name lookup in parallel mode |
| Previous Message | Henson Choi | 2026-11-10 00:52:56 | Re: [PATCH] Consider window functions when reordering GROUP BY items |