On Fri, Oct 29, 2021 at 4:24 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > I've attached a new version patch. Since the syntax of skipping > transaction id is under the discussion I've attached only the error > reporting patch for now. >
I have some comments on the v19-0001 patch: v19-0001 (1) doc/src/sgml/monitoring.sgml Seems to be missing the word "information": BEFORE: + <entry>At least one row per subscription, showing about errors that + occurred on subscription. AFTER: + <entry>At least one row per subscription, showing information about + errors that occurred on subscription. (2) pg_stat_reset_subscription_worker(subid Oid, relid Oid) First of all, I think that the documentation for this function should make it clear that a non-NULL "subid" parameter is required for both reset cases (tablesync and apply). Perhaps this could be done by simply changing the first sentence to say: "Resets statistics of a single subscription worker error, for a worker running on subscription with <parameter>subid</parameter>." (and then can remove " running on the subscription with <parameter>subid</parameter>" from the last sentence) I think that the documentation for this function should say that it should be used in conjunction with the "pg_stat_subscription_workers" view in order to obtain the required subid/relid values for resetting. (and should provide a link to the documentation for that view) Also, I think that the function documentation should make it clear that the tablesync error case is indicated by a NULL "command" in the information returned from the "pg_stat_subscription_workers" view (otherwise the user needs to look at the server log in order to determine whether the error is for the apply/tablesync worker). Finally, there are currently no tests for this new function. (3) pg_stat_subscription_workers In the documentation for this, the description for the "command" column says: "This field is always NULL if the error was reported during the initial data copy." Some users may not realise that this refers to "tablesync", so perhaps add " (tablesync)" to the end of this sentence, or similar. Regards, Greg Nancarrow Fujitsu Australia
