Re: Skipping logical replication transactions on subscriber side

On Fri, Sep 24, 2021 at 5:53 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> 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.

Thank you for the comments!

>
> 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.

As Amit pointed out, it seems not necessary to build a temporary hash
table for this purpose.

>
> 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 ?

Right. Will remove it.

>
>
> 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.

Yes, I'll move them to the end of the Enum struct.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/