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