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