Re: Add a hook for handling logical decoding messages on subscribers.

Hi,

On Tue, Aug 4, 2026 at 1:26 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> > Nice call-out about the more-than-once delivery and the idempotency
> > requirement. It's true for logical replication in general, but would....
>
> The patch already mentions in the doc:
>
> + <para>
> + Received messages are only acted upon if an extension installed on
> + the subscriber has registered a handler for them; otherwise they are
> + received and discarded. A message may be passed to the handler more
> + than once, for example after an apply worker restart, so a handler
> + must be prepared to process the same message repeatedly.
> + </para>
>
> Does it work for you?

WFM. I missed that, sorry for the noise!

> > 4/
> > + msg_data->prefix = pstrdup(pq_getmsgstring(in));
> > + msg = palloc(len + 1);
> >
> > I understand this isn't a memory leak, because ApplyMessageContext is
> > reset after every message is handled at the end of the apply loop in
> > LogicalRepApplyLoop(). But if the intention were to hand the caller
> > the message in long-lived memory, this palloc in ApplyMessageContext
> > doesn't achieve that either, since it gets reset after every message
> > anyway. So why not hand the hook the message directly from the
> > received StringInfo input buffer and let hook implementors copy it
> > into whatever context they need?
> >
> > The point is just that the palloc here doesn't give us anything (if
> > I'm not missing something), so I'm wondering whether we can avoid it.
>
> It's just to make the message handling consistent with the rest of
> proto.c does (see logicalrep_read_tuple()). I don't think it would be
> common for the hook to keep the received message in a long-lived
> memory context, so the current API seems fine to me.

Ah, I see it. Also, it's better to send a null-terminated string to
the hooks than sending everything across from the received input
buffer.

> > 6/
> > This is fine, but I personally like the idea Amit posted above: a
> > simple DDL replication example on the publisher and subscriber. Why
> > not have that instead? I'm aware it would mean more LoC for the test
> > extension, but that's fine IMO, since I see DDL replication as a good
> > use case to demonstrate in the test module.
>
> While query-shipping style DDL replication can be implemented using
> this hook, it would not be a great solution to me as neither a test
> module nor a contrib module. For a test module, it would need too much
> things to be implemented as a test module and people would not use it
> as it's not shipped in the package. As for implementing it as a
> contrib module, I think the module would not be integrated with the
> built-in logical replication well. For example, the extension cannot
> respect the table filter and publication options like
> publish_via_partition_root etc. Also, if we support DDL replication in
> the built-in logical replication, the contrib module would no longer
> be necessary but it would be hard to remove it as people might be
> using it.

Agreed to keep it simple the way you have it in the v2 patch.

> > 7/
> > Is it safe to commit the transaction that the non-transactional
> > message was emitted from, given that transaction may have already done
> > some work?
>
> I think that non-transactional messages are never interleaved with the
> remote transaction. We emit a non-transactional message as soon as we
> decode it (see ReorderBufferQueueMessage()) instead of buffering it.
>
> > I haven't verified this in depth, but going by intuition:
> >
> > BEGIN;
> > INSERT ...
> > UPDATE ...
> > DELETE ...
> > emit non-transactional message --> apply worker commits here
> > UPDATE ...
> > DELETE ...
> > COMMIT;
> >
> > Is something like this safe?
>
> In this case, the non-transaction message is sent first and the
> subscriber handles it as a single separate transaction, and then the
> transaction without the message is sent to the subscriber. I think it
> works fine.

Thanks for pointing me to the code. I understand it now. So, there can
never be a case where a non-transactional message is part of an
in-progress transaction on the apply worker, even in streaming mode,
because logical replication can't interleave transactions for apply.
If so, can we assert this or enhance the comment in
apply_handle_message() a bit?

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com