On Monday, February 28, 2022 11:34 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Sat, Feb 26, 2022 at 1:35 PM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.com> wrote:
> >
> > On Saturday, February 26, 2022 11:51 AM Amit Kapila
> <amit.kapila16@gmail.com> wrote:
> > > I have reviewed the latest version and made a few changes along with
> > > fixing some of the pending comments by Peter Smith. The changes are
> > > as
> > > follows: (a) Removed m_databaseid in PgStat_MsgSubscriptionError as
> > > that is not required now; (b) changed the struct name
> > > PgStat_MsgSubscriptionPurge to PgStat_MsgSubscriptionDrop to make it
> > > similar to DropDb; (c) changed the view name to
> > > pg_stat_subscription_stats, we can reconsider it in future if there
> > > is a consensus on some other name, accordingly changed the reset
> > > function name to pg_stat_reset_subscription_stats; (d) moved some of
> > > the newly added subscription stats functions adjacent to slots to
> > > main the consistency in code; (e) changed comments at few places;
> > > (f) added LATERAL back to system_views query as we refer
> pg_subscription's oid in the function call, previously that was not clear.
> > >
> > > Do let me know what you think of the attached?
> > Hi, thank you for updating the patch !
> > I have a couple of comments on v4.
> >
> > (1)
> >
> > I'm not sure if I'm correct, but I'd say the sync_error_count can come
> > next to the subname as the order of columns.
> > I felt there's case that the column order is somewhat related to the
> > time/processing order (I imagined pg_stat_replication's LSN related
> > columns).
> > If this was right, table sync related column could be the first column
> > as a counter within this patch.
> >
>
> I am not sure if there is such a correlation but even if it is there it doesn't seem
> to fit here completely as sync errors can happen after apply errors in multiple
> ways like via Alter Subscription ... Refresh ...
>
> So, I don't see the need to change the order here. What do you or others think?
In the alter subscription case, any errors after the table sync would increment
apply_error_count.
I mentioned this, because this point of view would impact on the doc read by users
and internal source codes for developers.
I had a concern that when we extend and increase a lot of statistics (not only for this view,
but also other statistics in general), writing doc for statistics needs some alignment for better
readability.
*But*, as you mentioned, in case we don't have such a correlation, I'm okay with the current patch.
Thank you for replying.
Best Regards,
Takamichi Osumi
