Re: Add last_commit_lsn to pg_stat_database
Aleksander Alekseev <aleksander@timescale.com>
From: Aleksander Alekseev <aleksander@timescale.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: James Coleman <jtc331@gmail.com>
Date: 2023-09-19T12:16:24Z
Lists: pgsql-hackers
Hi, > [...] > As I was thinking about how to improve things, I realized that this > information (since it's for monitoring anyway) fits more naturally > into the stats system. I'd originally thought of exposing it in > pg_stat_wal, but that's per-cluster rather than per-database (indeed, > this is a flaw I hadn't considered in the original patch), so I think > pg_stat_database is the correct location. > > I've attached a patch to track the latest commit's LSN in pg_stat_database. Thanks for the patch. It was marked as "Needs Review" so I decided to take a brief look. All in all the code seems to be fine but I have a couple of nitpicks: - If you are modifying pg_stat_database the corresponding changes to the documentation are needed. - pg_stat_get_db_last_commit_lsn() has the same description as pg_stat_get_db_xact_commit() which is confusing. - pg_stat_get_db_last_commit_lsn() is marked as STABLE which is probably correct. But I would appreciate a second opinion on this. - Wouldn't it be appropriate to add a test or two? - `if (!XLogRecPtrIsInvalid(commit_lsn))` - I suggest adding XLogRecPtrIsValid() macro for better readability -- Best regards, Aleksander Alekseev