On Fri, Sep 24, 2021 at 5:53 PM houzj.f...@fujitsu.com
<houzj.f...@fujitsu.com> wrote:
>
> On Tuesday, September 21, 2021 12:53 PM Masahiko Sawada
> <sawada.m...@gmail.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/
