MIME-Version: 1.0
References: <CALDaNm0Wg8XwfqUVnBSSahOtNxGDLNOW1_tc4ikZK=tGfqje-A@mail.gmail.com>
 <TYCPR01MB8373A9B6508E44E547A98EF5ED779@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <TYCPR01MB83734A7A0596AC7ADB0DCB51ED779@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <20211217.140309.1173685089580065514.horikyota.ntt@gmail.com>
 <TYCPR01MB83736371B66F90C71365ED11ED7B9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <CAJcOf-dmfinJDr=nOmjX1qNiU_mvW5tG3GyVwsQzRnUBb2ky0A@mail.gmail.com>
 <TYCPR01MB8373176545F7AE2CCDA23B81ED7D9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <TYCPR01MB6128932B793A2E0517421607FB469@TYCPR01MB6128.jpnprd01.prod.outlook.com>
 <TYCPR01MB83732A6796425B20CDC76987ED4A9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
In-Reply-To: <TYCPR01MB83732A6796425B20CDC76987ED4A9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Thu, 17 Feb 2022 15:13:24 +0530
Message-ID: <CAA4eK1+_p89Bm=UvSo6jAu4pjFga+R30+kMWbZ=+mJk5uOsoxQ@mail.gmail.com>
Subject: Re: Failed transaction statistics to measure the logical replication progress
To: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>
Cc: "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Greg Nancarrow <gregn4422@gmail.com>, 
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "vignesh21@gmail.com" <vignesh21@gmail.com>, 
	"sawada.mshk@gmail.com" <sawada.mshk@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, 
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1%2B_p89Bm%3DUvSo6jAu4pjFga%2BR30%2BkMWbZ%3D%2BmJk5uOsoxQ%40mail.gmail.com>
Precedence: bulk

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/=E5=94=90 =E6=B5=B7=
=E8=8B=B1 <tanghy.fnst@fujitsu.com> wrote:
> > 4)
> > +void
> > +pgstat_send_subworker_xact_stats(LogicalRepWorker *repWorker, bool
> > +force) {
> > +     static TimestampTz last_report =3D 0;
> > +     PgStat_MsgSubWorkerXactEnd msg;
> > +
> > +     if (!force)
> > +     {
> > ...
> > +             if (!TimestampDifferenceExceeds(last_report, now,
> > PGSTAT_STAT_INTERVAL))
> > +                     return;
> > +             last_report =3D now;
> > +     }
> > +
> > ...
> > +     if (repWorker->commit_count =3D=3D 0 && repWorker->abort_count =
=3D=3D
> > 0)
> > +             return;
> > ...
> >
> > I think it's better to check commit_count and abort_count first, then c=
heck if
> > reach PGSTAT_STAT_INTERVAL.
> > Otherwise if commit_count and abort_count are 0, it is possible that th=
e value
> > of last_report has been updated but it didn't send stats in fact. In th=
is 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.

--=20
With Regards,
Amit Kapila.