RE: Failed transaction statistics to measure the logical replication progress

On Tuesday, November 9, 2021 12:08 PM Greg Nancarrow <gregn4422(at)gmail(dot)com> wrote:
> On Fri, Nov 5, 2021 at 7:11 PM osumi(dot)takamichi(at)fujitsu(dot)com
> <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> >
>
> I did a quick scan through the latest v8 patch and noticed the following things:
I appreciate your review !

> src/backend/postmaster/pgstat.c
>
> (1) pgstat_recv_subworker_twophase_xact()
> The copying from msg->m_gid to key.gid does not seem to be correct.
> strlen() is being called on a junk value, since key.gid has not been assigned yet.
> It should be changed as follows:
>
> BEFORE:
> + strlcpy(key.gid, msg->m_gid, strlen(key.gid));
> AFTER:
> + strlcpy(key.gid, msg->m_gid, sizeof(key.gid));
Fixed.

> (2) pgstat_get_subworker_prepared_txn()
> Similar to above, strlen() usage is not correct, and should use
> strlcpy() instead of memcpy().
>
> BEFORE:
> + memcpy(key.gid, gid, strlen(key.gid));
> AFTER:
> + strlcpy(key.gid, gid, sizeof(key.gid));
Fixed.

> (3) stats_reset
> Note that the "stats_reset" column has been removed from the
> pg_stat_subscription_workers view in the underlying latest v20 patch.
Yes. I've rebased and updated the patch, paying attention to this point.
Attached the updated version.

Best Regards,
Takamichi Osumi