Re: Adding per backend commit and rollback counters

Thanks for the v5 patch set!

Here are my review comments:
1) Patches need re-base. Not able to apply.

2) PGSTAT_BACKEND_FLUSH_XACT collision with newly committed
PGSTAT_BACKEND_FLUSH_LOCK.

3) What if pgStatXactCommit and pgStatXactRollback are 0? We unnecessarily
make backend_has_xactstats as true while updating the pending struct
values. In this process unnecessary  ref_lock would be taken and
stats.xact_commit, stats.xact_rollback would be updated with 0 and then
ref_lock is unlocked.

4) PendingBackendStats was static but it was removed in this change, yet
the comment above still mentions it. Also it is killing the purpose of
"Utility routines to report I/O stats for backends ..." comment mentioned
in pgstat_backend.c. It appears the existing helper I/O routines' purpose
is eliminated. This doesn't look good.

4) I don't see any function using plural form like
pg_stat_get_backend_transactions in pg_proc.dat

5) The thread started with only commit and rollback counters, but xid was
added in v3, along with the idea of adding more, such as "seqscans, vacuum
count, analyze count...". This makes me curious: why can't these other
counters be added in this same thread? If these others aren't added now,
why was xid added? I suggest this thread should only include commit and
rollback counters, and xid should be added with the other metrics in a new
thread.


Regards,
Nishant Sharma,
EDB, Pune.
https://www.enterprisedb.com/