On Fri, Sep 24, 2021 at 6:44 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Sep 24, 2021 at 8:01 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> >
> > 6.
> > +typedef struct PgStat_StatSubEntry
> > +{
> > + Oid subid; /* hash table key */
> > +
> > + /*
> > + * Statistics of errors that occurred during logical replication. While
> > + * having the hash table for table sync errors we have a separate
> > + * statistics value for apply error (apply_error), because we can avoid
> > + * building a nested hash table for table sync errors in the case where
> > + * there is no table sync error, which is the common case in practice.
> > + *
> >
> > The above comment is not clear to me. Why do you need to have a
> > separate hash table for table sync errors? And what makes it avoid
> > building nested hash table?
>
> In the previous patch, a subscription stats entry
> (PgStat_StatSubEntry) had one hash table that had error entries of
> both apply and table sync. Since a subscription can have one apply
> worker and multiple table sync workers it makes sense to me to have
> the subscription entry have a hash table for them.
>
Sure, but each tablesync worker must have a separate relid. Why can't
we have a single hash table for both apply and table sync workers
which are hashed by sub_id + rel_id? For apply worker, the rel_id will
always be zero (InvalidOId) and tablesync workers will have a unique
OID for rel_id, so we should be able to uniquely identify each of
apply and table sync workers.
--
With Regards,
Amit Kapila.