On Monday, September 27, 2021 1:42 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Sep 22, 2021 at 10:10 AM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.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 ?
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.
If that's true, its thread and this thread are getting really close.
So, IIUC, we have to discuss this point as well.
Best Regards,
Takamichi Osumi