RE: Failed transaction statistics to measure the logical replication progress

On Thursday, October 21, 2021 10:19 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Mon, Oct 18, 2021 at 7:03 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Thu, Oct 14, 2021 at 9:23 AM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Thursday, September 30, 2021 12:15 PM Amit Kapila
> > > <amit(dot)kapila16(at)gmail(dot)com>
> > > >
> > > > 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..
> > >
> >
> > Don't you think that keeping the view name as
> > pg_stat_subscription_error would be a bit confusing if it has to
> > display xact_info? Isn't it better to change it to
> > pg_stat_subscription_worker or some other worker-specific generic
> > name?
>
> I agree that it'd be better to include xact info to pg_stat_subscription_errors
> view rather than making pg_stat_subscription a cumulative view. It would be
> more simple and consistent.
...
>I'll change the view name in the next version patch.
Thanks a lot, Sawasa-san.

I've created a new patch that extends pg_stat_subscription_workers
to include other transaction statistics.

Note that this patch depends on v18 patch-set in [1]
and needs to be after the perl modules' namespace changes
conducted recently by commit b3b4d8e68ae83f432f43f035c7eb481ef93e1583.

There are other several major changes compared to the previous version.

(1)
Addressing several streaming transactions running in parallel on the pub that
can send unexpected order of partial data demarcated stream start and stream stop.
Even if one of them aborts, now bytes calculation should be correct.

(2)
Updates of stats on the view at either commit prepared or rollback prepared time.
This means we don't lost prepared transaction size even after server restart
and user can see the results of two phase operation at those timings.

(3)
Addition of TAP tests.

(4)
Renames of other existing columns so that those meanings are more apparent
in align with other new stats.

Some important details are written in the comments of the attached patch.

[1] - https://www.postgresql.org/message-id/CAD21AoCTtQgfy57AxB4q8KUOpRH8rkHN%3Ds_9p9Pvno_XoBK5wg%40mail.gmail.com

Best Regards,
Takamichi Osumi