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
