RE: Design of pg_stat_subscription_workers vs pgstats

From: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
To: 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Design of pg_stat_subscription_workers vs pgstats
Date: 2022-02-26 08:05:17
Message-ID: TYCPR01MB837365B4C5E89E3A0E2A7353ED3F9@TYCPR01MB8373.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Saturday, February 26, 2022 11:51 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.

(2) doc/src/sgml/monitoring.sgml

+ Resets statistics for a single subscription shown in the
+ <structname>pg_stat_subscription_stats</structname> view to zero. If
+ the argument is <literal>NULL</literal>, reset statistics for all
+ subscriptions.
</para>

I felt we could improve the first sentence.

From:
Resets statistics for a single subscription shown in the..

To(idea1):
Resets statistics for a single subscription defined by the argument to zero.

Or,
To(idea2):
Resets statistics to zero for a single subscription or for all subscriptions.

Best Regards,
Takamichi Osumi

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2022-02-26 08:27:21 Re: [PATCH] pg_permissions
Previous Message Andres Freund 2022-02-26 07:23:25 Re: Race conditions in 019_replslot_limit.pl