RE: Design of pg_stat_subscription_workers vs pgstats

Takamichi Osumi (Fujitsu) <osumi.takamichi@fujitsu.com>

From: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Andres Freund <andres@anarazel.de>, "David G. Johnston" <david.g.johnston@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-02-26T08:05:17Z
Lists: pgsql-hackers
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.


(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

Commits

  1. Reconsider pg_stat_subscription_workers view.

  2. Add a view to show the stats of subscription workers.