From: | "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, 'Greg Nancarrow' <gregn4422(at)gmail(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>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: Failed transaction statistics to measure the logical replication progress |
Date: | 2021-12-31 01:12:25 |
Message-ID: | TYCPR01MB6128932B793A2E0517421607FB469@TYCPR01MB6128.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wednesday, December 22, 2021 6:14 PM osumi(dot)takamichi(at)fujitsu(dot)com <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
>
> Attached the new patch v19.
>
Thanks for your patch. I think it's better if you could add this patch to the commitfest.
Here are some comments:
1)
+ <structfield>commit_count</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of transactions successfully applied in this subscription.
+ Both COMMIT and COMMIT PREPARED increment this counter.
+ </para></entry>
+ </row>
...
I think the commands (like COMMIT, COMMIT PREPARED ...) can be surrounded with
"<command> </command>", thoughts?
2)
+extern void pgstat_report_subworker_xact_end(LogicalRepWorker *repWorker,
+ LogicalRepMsgType command,
+ bool bforce);
Should "bforce" be "force"?
3)
+ * This should be called before the call of process_syning_tables() so to not
"process_syning_tables()" should be "process_syncing_tables()".
4)
+void
+pgstat_send_subworker_xact_stats(LogicalRepWorker *repWorker, bool force)
+{
+ static TimestampTz last_report = 0;
+ PgStat_MsgSubWorkerXactEnd msg;
+
+ if (!force)
+ {
...
+ if (!TimestampDifferenceExceeds(last_report, now, PGSTAT_STAT_INTERVAL))
+ return;
+ last_report = now;
+ }
+
...
+ if (repWorker->commit_count == 0 && repWorker->abort_count == 0)
+ return;
...
I think it's better to check commit_count and abort_count first, then check if
reach PGSTAT_STAT_INTERVAL.
Otherwise if commit_count and abort_count are 0, it is possible that the value
of last_report has been updated but it didn't send stats in fact. In this case,
last_report is not the real time that send last message.
Regards,
Tang
From | Date | Subject | |
---|---|---|---|
Next Message | houzj.fnst@fujitsu.com | 2021-12-31 01:42:38 | RE: row filtering for logical replication |
Previous Message | Tom Lane | 2021-12-31 01:03:54 | Re: Apple's ranlib warns about protocol_openssl.c |