RE: Failed transaction statistics to measure the logical replication progress

On Tuesday, November 9, 2021 12:08 PM Greg Nancarrow <gregn4422@gmail.com> wrote:
> On Fri, Nov 5, 2021 at 7:11 PM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.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