RE: Skipping logical replication transactions on subscriber side

On Tuesday, September 21, 2021 12:53 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I've attached the updated version patches. Please review them.

Thanks for updating the patch,
here are a few comments on the v14-0001 patch.

1)
+ hash_ctl.keysize = sizeof(Oid);
+ hash_ctl.entrysize = sizeof(SubscriptionRelState);
+ not_ready_rels_htab = hash_create("not ready relations in subscription",
+ 64,
+ &hash_ctl,
+ HASH_ELEM | HASH_BLOBS);
+

ISTM we can pass list_length(not_ready_rels_list) as the nelem to hash_create.

2)

+ /*
+ * Search for all the dead subscriptions and error entries in stats
+ * hashtable and tell the stats collector to drop them.
+ */
+ if (subscriptionHash)
+ {
...
+ HTAB *htab;
+

It seems we already delacre a "HTAB *htab;" in function pgstat_vacuum_stat(),
can we use the existing htab here ?

3)

PGSTAT_MTYPE_RESETREPLSLOTCOUNTER,
+ PGSTAT_MTYPE_SUBSCRIPTIONERR,
+ PGSTAT_MTYPE_SUBSCRIPTIONERRRESET,
+ PGSTAT_MTYPE_SUBSCRIPTIONERRPURGE,
+ PGSTAT_MTYPE_SUBSCRIPTIONPURGE,
PGSTAT_MTYPE_AUTOVAC_START,

Can we append these values at the end of the Enum struct which won't affect the
other Enum values.

Best regards,
Hou zj