RE: Skipping logical replication transactions on subscriber side

From Thur, Sep 9, 2021 10:33 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Sorry for the late response. I've attached the updated patches that incorporate
> all comments unless I missed something. Please review them.

Thanks for the new version patches.
Here are some comments for the v13-0001 patch.

1)

+                    pgstat_setheader(&errmsg.m_hdr, PGSTAT_MTYPE_SUBSCRIPTIONERRPURGE);
+                    pgstat_send(&errmsg, len);
+                    errmsg.m_nentries = 0;
+                }

It seems we can invoke pgstat_setheader once before the loop like the
following:

+            errmsg.m_nentries = 0;
+            errmsg.m_subid = subent->subid;
+            pgstat_setheader(&errmsg.m_hdr, PGSTAT_MTYPE_SUBSCRIPTIONERRPURGE);

2)
+                    pgstat_setheader(&submsg.m_hdr, PGSTAT_MTYPE_SUBSCRIPTIONPURGE);
+                    pgstat_send(&submsg, len);

Same as 1), we can invoke pgstat_setheader once before the loop like:
+        submsg.m_nentries = 0;
+        pgstat_setheader(&submsg.m_hdr, PGSTAT_MTYPE_SUBSCRIPTIONPURGE);


3)

+/* ----------
+ * PgStat_MsgSubscriptionErrPurge    Sent by the autovacuum to purge the subscription
+ *                                    errors.

The comments said it's sent by autovacuum, would the manual vacuum also send
this message ?


4)
+
+    pgstat_send(&msg, offsetof(PgStat_MsgSubscriptionErr, m_reset) + sizeof(bool));
+}

Does it look cleaner that we use the offset of m_relid here like the following ?

pgstat_send(&msg, offsetof(PgStat_MsgSubscriptionErr, m_relid));

Best regards,
Hou zj