Re: Failed transaction statistics to measure the logical replication progress

On Tue, Sep 28, 2021 at 7:25 AM osumi(dot)takamichi(at)fujitsu(dot)com
<osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
>
> On Monday, September 27, 2021 1:42 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Wed, Sep 22, 2021 at 10:10 AM osumi(dot)takamichi(at)fujitsu(dot)com
> > <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> > >
> > > Just conducted some cosmetic changes
> > > and rebased my patch, using v14 patch-set in [1].
> > >
> >
> > IIUC, this proposal will allow new xact stats for subscriptions via
> > pg_stat_subscription. One thing that is not clear to me in this patch is that why
> > you choose a different way to store these stats than the existing stats in that
> > view? AFAICS, the other existing stats are stored in-memory in
> > LogicalRepWorker whereas these new stats are stored/fetched via stats
> > collector means these will persist. Isn't it better to be consistent here? I am not
> > sure which is a more appropriate way to store these stats and I would like to
> > hear your and other's thoughts on that matter but it appears a bit awkward to
> > me that some of the stats in the same view are persistent and others are
> > in-memory.
> Yeah, existing stats values of pg_stat_subscription are in-memory.
> I thought xact stats should survive over the restart,
> to summarize and show all accumulative transaction values
> on one subscription for user. But, your pointing out is reasonable,
> mixing two types can be awkward and lack of consistency.
>
> Then, if, we proceed in this direction,
> the place to implement those stats
> would be on the LogicalRepWorker struct, instead ?
>

Or, we can make existing stats persistent and then add these stats on
top of it. Sawada-San, do you have any thoughts on this matter?

> On one hand, what confuses me is that
> in another thread of feature to skip xid,
> I wondered if Sawada-san has started to take
> those xact stats into account (probably in his patch-set),
> because the stats values this thread is taking care of are listed up in the thread.
>

I don't think the Skip Xid patch is going to take care of these
additional stats but Sawada-San can confirm.

--
With Regards,
Amit Kapila.