RE: Failed transaction statistics to measure the logical replication progress - Mailing list pgsql-hackers

From osumi.takamichi@fujitsu.com
Subject RE: Failed transaction statistics to measure the logical replication progress
Date
Msg-id TYCPR01MB8373AB2AE1A6EC7B9E012519ED4A9@TYCPR01MB8373.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: Failed transaction statistics to measure the logical replication progress  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
List pgsql-hackers
On Monday, January 3, 2022 2:46 PM Hou, Zhijie/侯 志杰 <houzj.fnst@fujitsu.com> wrote:
> On Wednesday, December 22, 2021 6:14 PM Osumi, Takamichi
> <osumi.takamichi@fujitsu.com> wrote:
> > Attached the new patch v19.
> Hi,
> 
> Thanks for updating the patch.
> 
> --- a/src/include/pgstat.h
> +++ b/src/include/pgstat.h
> @@ -15,6 +15,7 @@
>  #include "portability/instr_time.h"
>  #include "postmaster/pgarch.h"    /* for MAX_XFN_CHARS */
>  #include "replication/logicalproto.h"
> +#include "replication/worker_internal.h"
> 
> I noticed that the patch includes "worker_internal.h " in pgstat.h.
> I think it might be better to only include this file in pgstat.c.
> And it seems we can access MyLogicalRepWorker directly in the following
> functions instead of passing a parameter.
> 
> +extern void pgstat_report_subworker_xact_end(LogicalRepWorker
> *repWorker,
> +
>          LogicalRepMsgType command,
> +
>          bool bforce);
> +extern void pgstat_send_subworker_xact_stats(LogicalRepWorker
> *repWorker,
> +
>          bool force);
Hi, thank you for your review !

Both are fixed. Additionally, I modified
related comments, the header comment of pgstat_send_subworker_xact_stats,
by the change.

One other new improvements in v20 is to have removed the 2nd argument of
pgstat_send_subworker_xact_stats(). When we called it from
commit-like operations, I passed 'false' without exceptions in v19
and noticed that could be removed.

Also, there's a really minor alignment in the source codes.
In pgstat.c, I placed pgstat_report_subworker_xact_end() after pgstat_report_subworker_error(),
and pgstat_send_subworker_xact_stats() after pgstat_send_subscription_purge() and so on
like the order of PgstatCollectorMain() and PgStat_Msg definition, because
my patch's new functions are added after the existing functions
for error handling functions of subscription workers.

Lastly, I changed the error report in pgstat_report_subworker_xact_end()
so that it can be more understandable easily and a bit modern.

Kindly have a look at the attached version.

Best Regards,
    Takamichi Osumi


Attachment

pgsql-hackers by date:

Previous
From: "Gunnar \"Nick\" Bluth"
Date:
Subject: Re: [PATCH] pg_stat_toast v6
Next
From: Simon Riggs
Date:
Subject: Re: Documenting when to retry on serialization failure