On Tue, Jan 4, 2022 at 5:22 PM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> On Friday, December 31, 2021 10:12 AM Tang, Haiying/唐 海英 <tanghy.fnst@fujitsu.com> wrote:
> > 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.
> Yeah, agreed. This fix is right in terms of the variable name aspect.
>
Can't we use pgstat_report_stat() here? Basically, you can update xact
completetion counters during apply, and then from
pgstat_report_stat(), you can invoke a logical replication worker
stats-related function.
--
With Regards,
Amit Kapila.
