Re: Skipping logical replication transactions on subscriber side

On Mon, Sep 27, 2021 at 11:02 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Sep 27, 2021 at 12:50 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Sep 27, 2021 at 12:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Mon, Sep 27, 2021 at 6:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Sat, Sep 25, 2021 at 4:23 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > 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.
> > > >
> > > > What I imagined is to extend the subscription statistics, for
> > > > instance, transaction stats[1]. By having a hash table for
> > > > subscriptions, we can store those statistics into an entry of the hash
> > > > table and we can think of subscription errors as also statistics of
> > > > the subscription. So we can have another hash table for errors in an
> > > > entry of the subscription hash table. For example, the subscription
> > > > entry struct will be something like:
> > > >
> > > > typedef struct PgStat_StatSubEntry
> > > > {
> > > >     Oid subid; /* hash key */
> > > >
> > > >     HTAB *errors;    /* apply and table sync errors */
> > > >
> > > >     /* transaction stats of subscription */
> > > >     PgStat_Counter xact_commit;
> > > >     PgStat_Counter xact_commit_bytes;
> > > >     PgStat_Counter xact_error;
> > > >     PgStat_Counter xact_error_bytes;
> > > >     PgStat_Counter xact_abort;
> > > >     PgStat_Counter xact_abort_bytes;
> > > >     PgStat_Counter failure_count;
> > > > } PgStat_StatSubEntry;
> > > >
> > >
> > > I think these additional stats will be displayed via
> > > pg_stat_subscription, right? If so, the current stats of that view are
> > > all in-memory and are per LogicalRepWorker which means that for those
> > > stats also we will have different entries for apply and table sync
> > > worker. If this understanding is correct, won't it be better to
> > > represent this as below?
> >
> > I was thinking that we have a different stats view for example
> > pg_stat_subscription_xacts that has entries per subscription. But your
> > idea seems better to me.
>
> I mean that showing statistics (including transaction statistics and
> errors) per logical replication worker seems better to me, no matter
> what view shows these statistics. I'll change the patch in that way.
>

Sounds good.

-- 
With Regards,
Amit Kapila.