RE: Failed transaction statistics to measure the logical replication progress - Mailing list pgsql-hackers

On Thursday, October 14, 2021 2:13 PM Osumi, Takamichi wrote:
> On Thursday, October 14, 2021 12:54 PM Hou, Zhijie<houzj.fnst@fujitsu.com> wrote:
> > On Thursday, September 30, 2021 12:15 PM Amit Kapila
> > > On Thu, Sep 30, 2021 at 8:22 AM Hou, Zhijie/侯 志杰wrote:
> > > >
> > > > On Tues, Sep 28, 2021 6:05 PM Amit Kapila
> > > > <amit.kapila16@gmail.com>
> > wrote:
> > > > >
> > > > > Can't we keep the current and new stats both in-memory and
> > > > > persist on disk? So, the persistent stats data will be used to
> > > > > fill the in-memory counters after restarting of workers,
> > > > > otherwise, we will always refer to in-memory values.
> > > >
> > > > I think this approach works, but I have another concern about it.
> > > >
> > > > The current pg_stat_subscription view is listed as "Dynamic
> > > > Statistics Views" in
> > > > the document, the data in it seems about the worker process, and
> > > > the view data
> > > > shows what the current worker did. But if we keep the new xact
> > > > stat persist, then it's not what the current worker did, it looks
> > > > more related to the subscription historic data.
> > > >
> > >
> > > I see your point.
> > >
> > > > Adding a new view seems resonalble, but it will bring another
> > > > subscription related view which might be too much. OTOH, I can see
> > > > there are already some
> > > > different views[1] including xact stat, maybe adding another one
> > > > is accepatble ?
> > > >
> > >
> > > These all views are related to untransmitted to the collector but
> > > what we really need is a view similar to pg_stat_archiver or
> > > pg_stat_bgwriter which gives information about background workers.
> > > Now, the problem as I see is if we go that route then
> > > pg_stat_subscription will no longer remain dynamic view and one
> > > might consider that as a compatibility break. The other idea I have
> > > shared is that we display these stats under the new view introduced
> > > by Sawada-San's patch [1] and probably rename that view as
> > > pg_stat_subscription_worker where all the stats (xact info and last
> > > failure information) about each worker will be displayed. Do you
> > > have any opinion on that idea or do you see any problem with it?
> >
> > Personally, I think it seems reasonable to merge the xact stat into
> > the view from sawada-san's patch.
> >
> > One problem I noticed is that pg_stat_subscription_error currently
> > have a 'count' column which show how many times the last error
> > happened. The xact stat here also have a similar value 'xact_error'. I
> > think we might need to rename it or merge them into one in some way.
> >
> > Besides, if we decide to merge xact stat into
> > pg_stat_subscription_error, some column seems need to be renamed.
> Maybe like:
> > error_message => Last_error_message, command=> last_error_command..
> Yeah, we must make them distinguished clearly.
> 
> I guessed that you are concerned about
> amount of renaming codes that could be a bit large or you come up with a
> necessity to consider the all column names of the pg_stat_subscription_worker
> together all at once in advance.
> 
> It's because my instant impression is,
> when we go with the current xact stats column definitions (xact_commit,
> xact_commit_bytes, xact_error, xact_error_bytes, xact_abort,
> xact_abort_bytes), the renaming problem can be solved if I write one
> additional patch or extend the main patch of xact stats to handle renaming.
> (This can work to keep both threads independent from each other).
> 
> Did you have some concern that cannot be handled by this way ?
Hi,

Currently, I don't find some unsolvable issues in this approach.

Best regards,
Hou zj