From: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | James Coleman <jtc331(at)gmail(dot)com> |
Subject: | Re: Add last_commit_lsn to pg_stat_database |
Date: | 2023-09-19 12:16:24 |
Message-ID: | CAJ7c6TPg0EerjTx0swG0TZf+_v8NFusNcNxqRsb4Kek6nmNSTw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-09-19 12:27:31 | Re: [PoC] pg_upgrade: allow to upgrade publisher node |
Previous Message | Amit Langote | 2023-09-19 12:00:02 | Re: remaining sql/json patches |