On Tuesday, April 28, 2026 4:18 PM Peter Smith <smithpb2250@gmail.com> wrote:
> ======
Thanks for the comments.
> Commit Message
>
> 1.
> Patch also adds LOGICAL_REP_MSG_INTERNAL_MESSAGE, so we need to
> describe how that works and how it has a double-meaning.
I added the enum value desc. The double-meaning feels like an implementation detail
to me, so I didn't add it to the comment. It should be clear from the function
that sends this message.
> 4.
> +/*
> + * Wait for the given transaction to finish.
> + *
> + * Both leader and parallel apply workers can call this function to wait for a
> + * transaction to finish.
> + */
> +void
> +pa_wait_for_depended_transaction(TransactionId xid)
> +{
> + elog(DEBUG1, "wait for depended xid %u", xid);
> +
> + for (;;)
> + {
> + /* XXX wait until given transaction is finished */
> + }
> +
> + elog(DEBUG1, "finish waiting for depended xid %u", xid);
> +}
>
> I don't think we should code infinite CPU loops, even if they are
> intended to be only temporary. IMO each patch needs to be valid
> standalone. So, it's better to use some "safer" code here -- e.g.
> maybe something that iterates a sleep 5s and will give RTE if it waits
> more than 5 min. Also, add a comment to say it is temporary code to
> be replaced by the next patch in this set.
I have reordered the patches to avoid adding temporary code that would make
maintenance harder.
Other comments have been addressed in the latest patch set.
Best Regards,
Hou zj
