On Sun, Nov 7, 2021 at 7:50 PM Masahiko Sawada <sawada.m...@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
