Re: Collect statistics about conflicts in logical replication

On Fri, Aug 30, 2024 at 9:40 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Aug 29, 2024 at 11:06 AM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> >
> > Agreed. Here is new version patch which change the names as suggested. I also
> > rebased the patch based on another renaming commit 640178c9.
> >
>
> Thanks for the patch. Few minor things:
>
> 1)
> conflict.h:
> * This enum is used in statistics collection (see
> * PgStat_StatSubEntry::conflict_count) as well, therefore, when adding new
> * values or reordering existing ones, ensure to review and potentially adjust
> * the corresponding statistics collection codes.
>
> --We shall mention PgStat_BackendSubEntry as well.
>
> 026_stats.pl:
> 2)
> # Now that the table is empty, the
> # update conflict (update_existing) ERRORs will stop happening.
>
> --Shall it be update_exists instead of update_existing here:
>
> 3)
> This is an existing comment above insert_exists conflict capture:
> # Wait for the apply error to be reported.
>
> --Shall we change to:
> # Wait for the subscriber to report apply error and insert_exists conflict.
>

1) I have tested the patch, works well.
2) Verified headers inclusions, all good
3) All my comments (very old ones when the patch was initially posted)
are now addressed.

So apart from the comments I posted in [1], I have no more comments.

[1]: https://www.postgresql.org/message-id/CAJpy0uAZpzustNOMBhxBctHHWbBA%3DsnTAYsLpoWZg%2BcqegmD-A%40mail.gmail.com

thanks
Shveta