From Thur, Sep 9, 2021 10:33 PM Masahiko Sawada <sawada.m...@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
