RE: Skipping logical replication transactions on subscriber side

On Fri, Jan 21, 2022 7:55 PM Amit Kapila <amit.kapila16@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