Hello, Thanks for reviewing!
On Tue, Sep 19, 2023 at 8:16 AM Aleksander Alekseev <aleksan...@timescale.com> wrote: > > 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. Updated. > - pg_stat_get_db_last_commit_lsn() has the same description as > pg_stat_get_db_xact_commit() which is confusing. I've fixed this. > - pg_stat_get_db_last_commit_lsn() is marked as STABLE which is > probably correct. But I would appreciate a second opinion on this. Sounds good. > - Wouldn't it be appropriate to add a test or two? Added. > - `if (!XLogRecPtrIsInvalid(commit_lsn))` - I suggest adding > XLogRecPtrIsValid() macro for better readability We have 36 usages of !XLogRecPtrIsInvalid(...) already, so I think we should avoid making this change in this patch. v2 is attached. Regards, James Coleman
v2-0001-Add-last-commit-s-LSN-to-pg_stat_database.patch
Description: Binary data
