Re: Skipping logical replication transactions on subscriber side

On Sun, Nov 7, 2021 at 7:50 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I've attached an updated patch. In this version patch, subscription
> worker statistics are collected per-database and handled in a similar
> way to tables and functions. I think perhaps we still need to discuss
> details of how the statistics should be handled but I'd like to share
> the patch for discussion.

While reviewing the v20, I have some initial comments,

+     <row>
+      <entry><structname>pg_stat_subscription_workers</structname><indexterm><primary>pg_stat_subscription_workers</primary></indexterm></entry>
+      <entry>At least one row per subscription, showing about errors that
+      occurred on subscription.
+      See <link linkend="monitoring-pg-stat-subscription-workers">
+      <structname>pg_stat_subscription_workers</structname></link> for details.
+      </entry>

1.
I don't like the fact that this view is very specific for showing the
errors but the name of the view is very generic.  So are we keeping
this name to expand the scope of the view in the future?  If this is
meant only for showing the errors then the name should be more
specific.

2.
Why comment says "At least one row per subscription"? this looks
confusing, I mean if there is no error then there will not be even one
row right?


+  <para>
+   The <structname>pg_stat_subscription_workers</structname> view will contain
+   one row per subscription error reported by workers applying logical
+   replication changes and workers handling the initial data copy of the
+   subscribed tables.
+  </para>

3.
So there will only be one row per subscription?  I did not read the
code, but suppose there was an error due to some constraint now if
that constraint is removed and there is a new error then the old error
will be removed immediately or it will be removed by auto vacuum?  If
it is not removed immediately then there could be multiple errors per
subscription in the view so the comment is not correct.

4.
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_error_time</structfield> <type>timestamp
with time zone</type>
+      </para>
+      <para>
+       Time at which the last error occurred
+      </para></entry>
+     </row>

Will it be useful to know when the first time error occurred?

5.
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>stats_reset</structfield> <type>timestamp with
time zone</type>
+      </para>
+      <para>

The actual view does not contain this column.

6.
+       <para>
+        Resets statistics of a single subscription worker statistics.

/Resets statistics of a single subscription worker statistics/Resets
statistics of a single subscription worker


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com