Re: Skipping logical replication transactions on subscriber side

On Wed, Nov 17, 2021 at 12:43 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tues, Nov 16, 2021 2:31 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > Right. I've fixed this issue and attached an updated patch.
>
> Hi,
>
> Thanks for updating the patch.
> Here are few comments.

Thank you for the comments!

>
> 1)
>
> + <function>pg_stat_reset_subscription_worker</function> ( <parameter>subid</parameter> <type>oid</type>, <optional> <parameter>relid</parameter> <type>oid</type> </optional> )
>
> It seems we should put '<optional>' before the comma(',').

Will fix.

>
>
> 2)
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>subrelid</structfield> <type>oid</type>
> + </para>
> + <para>
> + OID of the relation that the worker is synchronizing; null for the
> + main apply worker
> + </para></entry>
> + </row>
>
> Is the 'subrelid' only used for distinguishing the worker type ? If so, would it
> be clear to have a string value here. I recalled the previous version patch has
> failure_source column but was removed. Maybe I missed something.

As Amit mentioned, users can use this check which table sync worker.

>
>
> 3)
> .
> +extern void pgstat_reset_subworker_stats(Oid subid, Oid subrelid, bool allstats);
>
> I didn't find the code of this functions, maybe we can remove this declaration ?

Will remove.

I'll submit an updated patch.

Regards,

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