Re: Design of pg_stat_subscription_workers vs pgstats

Below are my comments for the v4 patch.

These are only nitpicking comments now; Otherwise, it LGTM.

(Sorry, now I see there are some overlaps with comments posted in the
last 20 mins so take or leave these as you wish)

======

1. doc/src/sgml/monitoring.sgml

- <para>
- OID of the relation that the worker was processing when the
- error occurred
+ Number of times an error occurred during the application of changes
</para></entry>
</row>

BEFORE
Number of times an error occurred during the application of changes
SUGGESTED
Number of times an error occurred while applying changes

~~~

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>

SUGGESTED (simpler description, more similar to pg_stat_reset_replication_slot)
Reset statistics to zero for a single subscription. If the argument is
<literal>NULL</literal>, reset statistics for all subscriptions.

~~~

3. src/backend/replication/logical/worker.c - comment

+ /* */
+ pgstat_report_subscription_error(MySubscription->oid, !am_tablesync_worker());

BEFORE
Report the worker failed during the application of the change
SUGGESTED
Report the worker failed while applying changes

~~~

4. src/include/pgstat.h - comment

+typedef struct PgStat_MsgResetsubcounter
+{
+ PgStat_MsgHdr m_hdr;
+ Oid m_subid; /* InvalidOid for clearing all subscription
+ * stats */
+} PgStat_MsgResetsubcounter;

BEFORE
InvalidOid for clearing all subscription stats
SUGGESTED
InvalidOid means reset all subscription stats

------
Kind Regards,
Peter Smith.
Fujitsu Australia