On Tue, Dec 7, 2021 at 3:12 PM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
Few questions and comments:
========================
1.
The <structname>pg_stat_subscription_workers</structname> view will contain
one row per subscription worker on which errors have occurred, for workers
applying logical replication changes and workers handling the initial data
- copy of the subscribed tables. The statistics entry is removed when the
- corresponding subscription is dropped.
+ copy of the subscribed tables. Also, the row corresponding to the apply
+ worker shows all transaction statistics of both types of workers on the
+ subscription. The statistics entry is removed when the corresponding
+ subscription is dropped.
Why did you choose to show stats for both types of workers in one row?
2.
+ PGSTAT_MTYPE_SUBWORKERXACTEND,
} StatMsgType;
I don't think we comma with the last message type.
3.
+ Oid m_subrelid;
+
+ /* necessary to determine column to increment */
+ LogicalRepMsgType m_command;
+
+} PgStat_MsgSubWorkerXactEnd;
Is m_subrelid used in this patch? If not, why did you keep it? I think
if you choose to show separate stats for table sync and apply worker
then probably it will be used.
4.
/*
+ * Cumulative transaction statistics of subscription worker
+ */
+ PgStat_Counter commit_count;
+ PgStat_Counter error_count;
+ PgStat_Counter abort_count;
+
I think it is better to keep the order of columns as commit_count,
abort_count, error_count in the entire patch.
--
With Regards,
Amit Kapila.