Re: Skipping logical replication transactions on subscriber side

On Tue, Nov 9, 2021 at 3:07 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Sun, Nov 7, 2021 at 7:50 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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.

As Amit already mentioned, we're planning to add more xact statistics
to this view. I've mentioned that in the commit message.

>
> 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>

Right. Fixed.

>
> 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.

There is one row per subscription worker (apply worker and tablesync
worker). If the same error consecutively occurred, error_count is
incremented and last_error_time is updated. Otherwise, i.g., if a
different error occurred on the apply worker, all statistics are
updated.

>
> 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?

Good idea. Users can know when the subscription stopped due to this
error. Added.

>
> 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.

Removed.

>
> 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

Fixed.

I'll update an updated patch soon.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/