On Fri, Jan 21, 2022 7:55 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> 2.
> +stop_skipping_changes(bool clear_subskipxid, XLogRecPtr origin_lsn,
> + TimestampTz origin_timestamp)
> +{
> + Assert(is_skipping_changes());
> +
> + ereport(LOG,
> + (errmsg("done skipping logical replication transaction %u",
> + skip_xid)));
>
> Isn't it better to move this LOG at the end of this function? Because
> clear* functions can give an error, so it is better to move it after
> that. I have done that in the attached.
>
+ /* Stop skipping changes */
+ skip_xid = InvalidTransactionId;
+
+ ereport(LOG,
+ (errmsg("done skipping logical replication transaction
%u",
+ skip_xid)));
I think we can move the LOG before resetting skip_xid, otherwise skip_xid would
always be 0 in the LOG.
Regards,
Tang
