RE: Failed transaction statistics to measure the logical replication progress

From: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
To: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "vignesh21(at)gmail(dot)com" <vignesh21(at)gmail(dot)com>, "amit(dot)kapila16(at)gmail(dot)com" <amit(dot)kapila16(at)gmail(dot)com>, "sawada(dot)mshk(at)gmail(dot)com" <sawada(dot)mshk(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Greg Nancarrow' <gregn4422(at)gmail(dot)com>
Subject: RE: Failed transaction statistics to measure the logical replication progress
Date: 2022-01-04 11:47:26
Message-ID: TYCPR01MB8373AB2AE1A6EC7B9E012519ED4A9@TYCPR01MB8373.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Monday, January 3, 2022 2:46 PM Hou, Zhijie/侯 志杰 <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> On Wednesday, December 22, 2021 6:14 PM Osumi, Takamichi
> <osumi(dot)takamichi(at)fujitsu(dot)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 Content-Type Size
v20-0001-Fix-alignments-of-TAP-tests-for-pg_stat_subscrip.patch application/octet-stream 7.4 KB
v20-0002-Extend-pg_stat_subscription_workers-to-include-g.patch application/octet-stream 26.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2022-01-04 11:49:28 Re: Documenting when to retry on serialization failure
Previous Message Gunnar "Nick" Bluth 2022-01-04 11:29:09 Re: [PATCH] pg_stat_toast v6