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
