Add last_commit_lsn to pg_stat_database

Started by James Colemanover 2 years ago17 messages

jtc331@gmail.com

over 2 years ago

1 attachment(s)

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

In the aforementioned thread [1] I'd proposed a patch that added a SQL
function pg_last_commit_lsn() to expose the most recent commit's LSN.
Robert Haas didn't think the initial version's modifications to
commit_ts made sense, and a subsequent approach adding the value to
PGPROC didn't have strong objections, from what I can see, but it also
didn't generate any enthusiasm.

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.

Regards,
James Coleman

1: /messages/by-id/CAAaqYe9QBiAu+j8rBun_JKBRe-3HeKLUhfVVsYfsxQG0VqLXsA@mail.gmail.com

Attachments:

v1-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchapplication/octet-stream; name=v1-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchDownload

From 00640c49a45e9a11a3634ee35a678cbe15da440e Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 9 Jun 2023 20:12:31 -0500
Subject: [PATCH v1] Add last commit's LSN to pg_stat_database

---
 src/backend/access/transam/twophase.c        | 11 +++++++----
 src/backend/access/transam/xact.c            | 11 ++++++-----
 src/backend/catalog/system_views.sql         |  1 +
 src/backend/utils/activity/pgstat_database.c | 20 +++++++++++++++++++-
 src/backend/utils/activity/pgstat_xact.c     |  4 ++--
 src/backend/utils/adt/pgstatfuncs.c          |  3 +++
 src/include/catalog/pg_proc.dat              |  4 ++++
 src/include/pgstat.h                         |  4 +++-
 src/include/utils/pgstat_internal.h          |  2 +-
 src/test/regress/expected/rules.out          |  1 +
 10 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 068e59bec0..c38e36e37c 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -200,7 +200,7 @@ static GlobalTransaction MyLockedGxact = NULL;
 
 static bool twophaseExitRegistered = false;
 
-static void RecordTransactionCommitPrepared(TransactionId xid,
+static XLogRecPtr RecordTransactionCommitPrepared(TransactionId xid,
 											int nchildren,
 											TransactionId *children,
 											int nrels,
@@ -1494,6 +1494,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	xl_xact_stats_item *commitstats;
 	xl_xact_stats_item *abortstats;
 	SharedInvalidationMessage *invalmsgs;
+	XLogRecPtr commit_lsn = InvalidXLogRecPtr;
 
 	/*
 	 * Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1549,7 +1550,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	 * callbacks will release the locks the transaction held.
 	 */
 	if (isCommit)
-		RecordTransactionCommitPrepared(xid,
+		commit_lsn = RecordTransactionCommitPrepared(xid,
 										hdr->nsubxacts, children,
 										hdr->ncommitrels, commitrels,
 										hdr->ncommitstats,
@@ -1644,7 +1645,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	LWLockRelease(TwoPhaseStateLock);
 
 	/* Count the prepared xact as committed or aborted */
-	AtEOXact_PgStat(isCommit, false);
+	AtEOXact_PgStat(isCommit, commit_lsn, false);
 
 	/*
 	 * And now we can clean up any files we may have left.
@@ -2278,7 +2279,7 @@ ProcessTwoPhaseBuffer(TransactionId xid,
  * We know the transaction made at least one XLOG entry (its PREPARE),
  * so it is never possible to optimize out the commit record.
  */
-static void
+static XLogRecPtr
 RecordTransactionCommitPrepared(TransactionId xid,
 								int nchildren,
 								TransactionId *children,
@@ -2366,6 +2367,8 @@ RecordTransactionCommitPrepared(TransactionId xid,
 	 * in the procarray and continue to hold locks.
 	 */
 	SyncRepWaitForLSN(recptr, true);
+
+	return recptr;
 }
 
 /*
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 8daaa535ed..895e103c70 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1280,7 +1280,7 @@ AtSubStart_ResourceOwner(void)
  * If you change this function, see RecordTransactionCommitPrepared also.
  */
 static TransactionId
-RecordTransactionCommit(void)
+RecordTransactionCommit(XLogRecPtr *commit_lsn)
 {
 	TransactionId xid = GetTopTransactionIdIfAny();
 	bool		markXidCommitted = TransactionIdIsValid(xid);
@@ -1397,7 +1397,7 @@ RecordTransactionCommit(void)
 		/*
 		 * Insert the commit XLOG record.
 		 */
-		XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
+		*commit_lsn = XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
 							nchildren, children, nrels, rels,
 							ndroppedstats, droppedstats,
 							nmsgs, invalMessages,
@@ -2153,6 +2153,7 @@ CommitTransaction(void)
 {
 	TransactionState s = CurrentTransactionState;
 	TransactionId latestXid;
+	XLogRecPtr	commit_lsn = InvalidXLogRecPtr;
 	bool		is_parallel_worker;
 
 	is_parallel_worker = (s->blockState == TBLOCK_PARALLEL_INPROGRESS);
@@ -2264,7 +2265,7 @@ CommitTransaction(void)
 		 * We need to mark our XIDs as committed in pg_xact.  This is where we
 		 * durably commit.
 		 */
-		latestXid = RecordTransactionCommit();
+		latestXid = RecordTransactionCommit(&commit_lsn);
 	}
 	else
 	{
@@ -2368,7 +2369,7 @@ CommitTransaction(void)
 	AtEOXact_Files(true);
 	AtEOXact_ComboCid();
 	AtEOXact_HashTables(true);
-	AtEOXact_PgStat(true, is_parallel_worker);
+	AtEOXact_PgStat(true, commit_lsn, is_parallel_worker);
 	AtEOXact_Snapshot(true, false);
 	AtEOXact_ApplyLauncher(true);
 	AtEOXact_LogicalRepWorkers(true);
@@ -2869,7 +2870,7 @@ AbortTransaction(void)
 		AtEOXact_Files(false);
 		AtEOXact_ComboCid();
 		AtEOXact_HashTables(false);
-		AtEOXact_PgStat(false, is_parallel_worker);
+		AtEOXact_PgStat(false, InvalidXLogRecPtr, is_parallel_worker);
 		AtEOXact_ApplyLauncher(false);
 		AtEOXact_LogicalRepWorkers(false);
 		pgstat_report_xact_timestamp(0);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index c18fea8362..014d1d0433 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1029,6 +1029,7 @@ CREATE VIEW pg_stat_database AS
                     WHEN (D.oid = (0)::oid) THEN 0
                     ELSE pg_stat_get_db_numbackends(D.oid)
                 END AS numbackends,
+            pg_stat_get_db_last_commit_lsn(D.oid) AS last_commit_lsn,
             pg_stat_get_db_xact_commit(D.oid) AS xact_commit,
             pg_stat_get_db_xact_rollback(D.oid) AS xact_rollback,
             pg_stat_get_db_blocks_fetched(D.oid) -
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 7149f22f72..529cf0dee3 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -34,6 +34,7 @@ SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
 
 static int	pgStatXactCommit = 0;
 static int	pgStatXactRollback = 0;
+static XLogRecPtr	pgStatLastCommitLSN = InvalidXLogRecPtr;
 static PgStat_Counter pgLastSessionReportTime = 0;
 
 
@@ -246,7 +247,7 @@ pgstat_fetch_stat_dbentry(Oid dboid)
 }
 
 void
-AtEOXact_PgStat_Database(bool isCommit, bool parallel)
+AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	/* Don't count parallel worker transaction stats */
 	if (!parallel)
@@ -256,7 +257,12 @@ AtEOXact_PgStat_Database(bool isCommit, bool parallel)
 		 * bools, in case the reporting message isn't sent right away.)
 		 */
 		if (isCommit)
+		{
 			pgStatXactCommit++;
+			/* For commits also track the LSN of the commit record. */
+			if (!XLogRecPtrIsInvalid(commit_lsn))
+				pgStatLastCommitLSN = commit_lsn;
+		}
 		else
 			pgStatXactRollback++;
 	}
@@ -273,6 +279,13 @@ pgstat_update_dbstats(TimestampTz ts)
 
 	dbentry = pgstat_prep_database_pending(MyDatabaseId);
 
+	/*
+	 * pgStatLastCommitLSN is only ever set when there's a valid commit, so we
+	 * know the LSN must be valid, and since we're local to the current backend
+	 * we don't have to worry if we're advancing or not.
+	 */
+	dbentry->last_commit_lsn = pgStatLastCommitLSN;
+
 	/*
 	 * Accumulate xact commit/rollback and I/O timings to stats entry of the
 	 * current database.
@@ -304,6 +317,7 @@ pgstat_update_dbstats(TimestampTz ts)
 	pgStatBlockWriteTime = 0;
 	pgStatActiveTime = 0;
 	pgStatTransactionIdleTime = 0;
+	/* No reason to zero out pgStatLastCommitLSN; it's not accumulative. */
 }
 
 /*
@@ -414,6 +428,10 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	PGSTAT_ACCUM_DBCOUNT(sessions_killed);
 #undef PGSTAT_ACCUM_DBCOUNT
 
+	/* Only update last_commit_lsn if our backend has the newest commit. */
+	if (pendingent->last_commit_lsn > sharedent->stats.last_commit_lsn)
+		sharedent->stats.last_commit_lsn = pendingent->last_commit_lsn;
+
 	pgstat_unlock_entry(entry_ref);
 
 	memset(pendingent, 0, sizeof(*pendingent));
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 369239d501..2713b6a4e6 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -38,11 +38,11 @@ static PgStat_SubXactStatus *pgStatXactStack = NULL;
  * Called from access/transam/xact.c at top-level transaction commit/abort.
  */
 void
-AtEOXact_PgStat(bool isCommit, bool parallel)
+AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	PgStat_SubXactStatus *xact_state;
 
-	AtEOXact_PgStat_Database(isCommit, parallel);
+	AtEOXact_PgStat_Database(isCommit, commit_lsn, parallel);
 
 	/* handle transactional stats information */
 	xact_state = pgStatXactStack;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 49adc319fc..d1a4222458 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1065,6 +1065,9 @@ PG_STAT_GET_DBENTRY_INT64(tuples_returned)
 /* pg_stat_get_db_tuples_updated */
 PG_STAT_GET_DBENTRY_INT64(tuples_updated)
 
+/* pg_stat_get_db_last_commit_lsn */
+PG_STAT_GET_DBENTRY_INT64(last_commit_lsn)
+
 /* pg_stat_get_db_xact_commit */
 PG_STAT_GET_DBENTRY_INT64(xact_commit)
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..28d01e5b25 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5534,6 +5534,10 @@
   proname => 'pg_stat_get_db_numbackends', provolatile => 's',
   proparallel => 'r', prorettype => 'int4', proargtypes => 'oid',
   prosrc => 'pg_stat_get_db_numbackends' },
+{ oid => '8000', descr => 'statistics: transactions committed',
+  proname => 'pg_stat_get_db_last_commit_lsn', provolatile => 's',
+  proparallel => 'r', prorettype => 'pg_lsn', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_db_last_commit_lsn' },
 { oid => '1942', descr => 'statistics: transactions committed',
   proname => 'pg_stat_get_db_xact_commit', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => 'oid',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 57a2c0866a..c492759a71 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -18,6 +18,7 @@
 #include "utils/backend_status.h"	/* for backward compatibility */
 #include "utils/relcache.h"
 #include "utils/wait_event.h"	/* for backward compatibility */
+#include "access/xlogdefs.h"
 
 
 /* ----------
@@ -320,6 +321,7 @@ typedef struct PgStat_IO
 
 typedef struct PgStat_StatDBEntry
 {
+	XLogRecPtr	last_commit_lsn;
 	PgStat_Counter xact_commit;
 	PgStat_Counter xact_rollback;
 	PgStat_Counter blocks_fetched;
@@ -702,7 +704,7 @@ extern PgStat_StatSubEntry *pgstat_fetch_stat_subscription(Oid subid);
  * Functions in pgstat_xact.c
  */
 
-extern void AtEOXact_PgStat(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 extern void AtEOSubXact_PgStat(bool isCommit, int nestDepth);
 extern void AtPrepare_PgStat(void);
 extern void PostPrepare_PgStat(void);
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 60fbf9394b..700d9c27bb 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -548,7 +548,7 @@ extern void pgstat_checkpointer_snapshot_cb(void);
 
 extern void pgstat_report_disconnect(Oid dboid);
 extern void pgstat_update_dbstats(TimestampTz ts);
-extern void AtEOXact_PgStat_Database(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 
 extern PgStat_StatDBEntry *pgstat_prep_database_pending(Oid dboid);
 extern void pgstat_reset_database_timestamp(Oid dboid, TimestampTz ts);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7fd81e6a7d..62b909ca92 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1833,6 +1833,7 @@ pg_stat_database| SELECT oid AS datid,
             WHEN (oid = (0)::oid) THEN 0
             ELSE pg_stat_get_db_numbackends(oid)
         END AS numbackends,
+    pg_stat_get_db_last_commit_lsn(oid) AS last_commit_lsn,
     pg_stat_get_db_xact_commit(oid) AS xact_commit,
     pg_stat_get_db_xact_rollback(oid) AS xact_rollback,
     (pg_stat_get_db_blocks_fetched(oid) - pg_stat_get_db_blocks_hit(oid)) AS blks_read,
-- 
2.39.2 (Apple Git-143)

Aleksander Alekseev

aleksander@timescale.com

over 2 years ago

In reply to: James Coleman (#1)

Re: Add last_commit_lsn to pg_stat_database

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

vignesh C

vignesh21@gmail.com

almost 2 years ago

In reply to: James Coleman (#1)

Re: Add last_commit_lsn to pg_stat_database

On Sat, 10 Jun 2023 at 07:57, James Coleman <jtc331@gmail.com> wrote:

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

In the aforementioned thread [1] I'd proposed a patch that added a SQL
function pg_last_commit_lsn() to expose the most recent commit's LSN.
Robert Haas didn't think the initial version's modifications to
commit_ts made sense, and a subsequent approach adding the value to
PGPROC didn't have strong objections, from what I can see, but it also
didn't generate any enthusiasm.

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.

I have changed the status of commitfest entry to "Returned with
Feedback" as Aleksander's comments have not yet been resolved. Please
feel free to post an updated version of the patch and update the
commitfest entry accordingly.

Regards,
Vignesh

James Coleman

jtc331@gmail.com

almost 2 years ago

In reply to: Aleksander Alekseev (#2)

1 attachment(s)

Re: Add last_commit_lsn to pg_stat_database

Hello,

Thanks for reviewing!

On Tue, Sep 19, 2023 at 8:16 AM Aleksander Alekseev
<aleksander@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

Attachments:

v2-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchapplication/octet-stream; name=v2-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchDownload

From b56b6d8c8686e7dd2b77c25d3d9d61908b769878 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 9 Jun 2023 20:12:31 -0500
Subject: [PATCH v2] Add last commit's LSN to pg_stat_database

---
 doc/src/sgml/monitoring.sgml                 |  9 ++++++++
 src/backend/access/transam/twophase.c        | 11 +++++----
 src/backend/access/transam/xact.c            | 11 +++++----
 src/backend/catalog/system_views.sql         |  1 +
 src/backend/utils/activity/pgstat_database.c | 20 +++++++++++++++-
 src/backend/utils/activity/pgstat_xact.c     |  4 ++--
 src/backend/utils/adt/pgstatfuncs.c          |  3 +++
 src/include/catalog/pg_proc.dat              |  4 ++++
 src/include/pgstat.h                         |  4 +++-
 src/include/utils/pgstat_internal.h          |  2 +-
 src/test/regress/expected/rules.out          |  1 +
 src/test/regress/expected/stats.out          | 24 ++++++++++++++++++++
 src/test/regress/sql/stats.sql               | 10 ++++++++
 13 files changed, 90 insertions(+), 14 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index df5242fa80..6f2ea1ddbe 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4414,6 +4414,15 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_commit_lsn</structfield> <type>pg_lsn</type>
+      </para>
+      <para>
+       Write-ahead log location of the last commit in this database
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>xact_commit</structfield> <type>bigint</type>
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 068e59bec0..c38e36e37c 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -200,7 +200,7 @@ static GlobalTransaction MyLockedGxact = NULL;
 
 static bool twophaseExitRegistered = false;
 
-static void RecordTransactionCommitPrepared(TransactionId xid,
+static XLogRecPtr RecordTransactionCommitPrepared(TransactionId xid,
 											int nchildren,
 											TransactionId *children,
 											int nrels,
@@ -1494,6 +1494,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	xl_xact_stats_item *commitstats;
 	xl_xact_stats_item *abortstats;
 	SharedInvalidationMessage *invalmsgs;
+	XLogRecPtr commit_lsn = InvalidXLogRecPtr;
 
 	/*
 	 * Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1549,7 +1550,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	 * callbacks will release the locks the transaction held.
 	 */
 	if (isCommit)
-		RecordTransactionCommitPrepared(xid,
+		commit_lsn = RecordTransactionCommitPrepared(xid,
 										hdr->nsubxacts, children,
 										hdr->ncommitrels, commitrels,
 										hdr->ncommitstats,
@@ -1644,7 +1645,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	LWLockRelease(TwoPhaseStateLock);
 
 	/* Count the prepared xact as committed or aborted */
-	AtEOXact_PgStat(isCommit, false);
+	AtEOXact_PgStat(isCommit, commit_lsn, false);
 
 	/*
 	 * And now we can clean up any files we may have left.
@@ -2278,7 +2279,7 @@ ProcessTwoPhaseBuffer(TransactionId xid,
  * We know the transaction made at least one XLOG entry (its PREPARE),
  * so it is never possible to optimize out the commit record.
  */
-static void
+static XLogRecPtr
 RecordTransactionCommitPrepared(TransactionId xid,
 								int nchildren,
 								TransactionId *children,
@@ -2366,6 +2367,8 @@ RecordTransactionCommitPrepared(TransactionId xid,
 	 * in the procarray and continue to hold locks.
 	 */
 	SyncRepWaitForLSN(recptr, true);
+
+	return recptr;
 }
 
 /*
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 8daaa535ed..895e103c70 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1280,7 +1280,7 @@ AtSubStart_ResourceOwner(void)
  * If you change this function, see RecordTransactionCommitPrepared also.
  */
 static TransactionId
-RecordTransactionCommit(void)
+RecordTransactionCommit(XLogRecPtr *commit_lsn)
 {
 	TransactionId xid = GetTopTransactionIdIfAny();
 	bool		markXidCommitted = TransactionIdIsValid(xid);
@@ -1397,7 +1397,7 @@ RecordTransactionCommit(void)
 		/*
 		 * Insert the commit XLOG record.
 		 */
-		XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
+		*commit_lsn = XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
 							nchildren, children, nrels, rels,
 							ndroppedstats, droppedstats,
 							nmsgs, invalMessages,
@@ -2153,6 +2153,7 @@ CommitTransaction(void)
 {
 	TransactionState s = CurrentTransactionState;
 	TransactionId latestXid;
+	XLogRecPtr	commit_lsn = InvalidXLogRecPtr;
 	bool		is_parallel_worker;
 
 	is_parallel_worker = (s->blockState == TBLOCK_PARALLEL_INPROGRESS);
@@ -2264,7 +2265,7 @@ CommitTransaction(void)
 		 * We need to mark our XIDs as committed in pg_xact.  This is where we
 		 * durably commit.
 		 */
-		latestXid = RecordTransactionCommit();
+		latestXid = RecordTransactionCommit(&commit_lsn);
 	}
 	else
 	{
@@ -2368,7 +2369,7 @@ CommitTransaction(void)
 	AtEOXact_Files(true);
 	AtEOXact_ComboCid();
 	AtEOXact_HashTables(true);
-	AtEOXact_PgStat(true, is_parallel_worker);
+	AtEOXact_PgStat(true, commit_lsn, is_parallel_worker);
 	AtEOXact_Snapshot(true, false);
 	AtEOXact_ApplyLauncher(true);
 	AtEOXact_LogicalRepWorkers(true);
@@ -2869,7 +2870,7 @@ AbortTransaction(void)
 		AtEOXact_Files(false);
 		AtEOXact_ComboCid();
 		AtEOXact_HashTables(false);
-		AtEOXact_PgStat(false, is_parallel_worker);
+		AtEOXact_PgStat(false, InvalidXLogRecPtr, is_parallel_worker);
 		AtEOXact_ApplyLauncher(false);
 		AtEOXact_LogicalRepWorkers(false);
 		pgstat_report_xact_timestamp(0);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index c18fea8362..014d1d0433 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1029,6 +1029,7 @@ CREATE VIEW pg_stat_database AS
                     WHEN (D.oid = (0)::oid) THEN 0
                     ELSE pg_stat_get_db_numbackends(D.oid)
                 END AS numbackends,
+            pg_stat_get_db_last_commit_lsn(D.oid) AS last_commit_lsn,
             pg_stat_get_db_xact_commit(D.oid) AS xact_commit,
             pg_stat_get_db_xact_rollback(D.oid) AS xact_rollback,
             pg_stat_get_db_blocks_fetched(D.oid) -
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 7149f22f72..529cf0dee3 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -34,6 +34,7 @@ SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
 
 static int	pgStatXactCommit = 0;
 static int	pgStatXactRollback = 0;
+static XLogRecPtr	pgStatLastCommitLSN = InvalidXLogRecPtr;
 static PgStat_Counter pgLastSessionReportTime = 0;
 
 
@@ -246,7 +247,7 @@ pgstat_fetch_stat_dbentry(Oid dboid)
 }
 
 void
-AtEOXact_PgStat_Database(bool isCommit, bool parallel)
+AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	/* Don't count parallel worker transaction stats */
 	if (!parallel)
@@ -256,7 +257,12 @@ AtEOXact_PgStat_Database(bool isCommit, bool parallel)
 		 * bools, in case the reporting message isn't sent right away.)
 		 */
 		if (isCommit)
+		{
 			pgStatXactCommit++;
+			/* For commits also track the LSN of the commit record. */
+			if (!XLogRecPtrIsInvalid(commit_lsn))
+				pgStatLastCommitLSN = commit_lsn;
+		}
 		else
 			pgStatXactRollback++;
 	}
@@ -273,6 +279,13 @@ pgstat_update_dbstats(TimestampTz ts)
 
 	dbentry = pgstat_prep_database_pending(MyDatabaseId);
 
+	/*
+	 * pgStatLastCommitLSN is only ever set when there's a valid commit, so we
+	 * know the LSN must be valid, and since we're local to the current backend
+	 * we don't have to worry if we're advancing or not.
+	 */
+	dbentry->last_commit_lsn = pgStatLastCommitLSN;
+
 	/*
 	 * Accumulate xact commit/rollback and I/O timings to stats entry of the
 	 * current database.
@@ -304,6 +317,7 @@ pgstat_update_dbstats(TimestampTz ts)
 	pgStatBlockWriteTime = 0;
 	pgStatActiveTime = 0;
 	pgStatTransactionIdleTime = 0;
+	/* No reason to zero out pgStatLastCommitLSN; it's not accumulative. */
 }
 
 /*
@@ -414,6 +428,10 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	PGSTAT_ACCUM_DBCOUNT(sessions_killed);
 #undef PGSTAT_ACCUM_DBCOUNT
 
+	/* Only update last_commit_lsn if our backend has the newest commit. */
+	if (pendingent->last_commit_lsn > sharedent->stats.last_commit_lsn)
+		sharedent->stats.last_commit_lsn = pendingent->last_commit_lsn;
+
 	pgstat_unlock_entry(entry_ref);
 
 	memset(pendingent, 0, sizeof(*pendingent));
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 369239d501..2713b6a4e6 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -38,11 +38,11 @@ static PgStat_SubXactStatus *pgStatXactStack = NULL;
  * Called from access/transam/xact.c at top-level transaction commit/abort.
  */
 void
-AtEOXact_PgStat(bool isCommit, bool parallel)
+AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	PgStat_SubXactStatus *xact_state;
 
-	AtEOXact_PgStat_Database(isCommit, parallel);
+	AtEOXact_PgStat_Database(isCommit, commit_lsn, parallel);
 
 	/* handle transactional stats information */
 	xact_state = pgStatXactStack;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 49adc319fc..d1a4222458 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1065,6 +1065,9 @@ PG_STAT_GET_DBENTRY_INT64(tuples_returned)
 /* pg_stat_get_db_tuples_updated */
 PG_STAT_GET_DBENTRY_INT64(tuples_updated)
 
+/* pg_stat_get_db_last_commit_lsn */
+PG_STAT_GET_DBENTRY_INT64(last_commit_lsn)
+
 /* pg_stat_get_db_xact_commit */
 PG_STAT_GET_DBENTRY_INT64(xact_commit)
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..377a0a6ae0 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5534,6 +5534,10 @@
   proname => 'pg_stat_get_db_numbackends', provolatile => 's',
   proparallel => 'r', prorettype => 'int4', proargtypes => 'oid',
   prosrc => 'pg_stat_get_db_numbackends' },
+{ oid => '8000', descr => 'statistics: wal location of last committed transaction',
+  proname => 'pg_stat_get_db_last_commit_lsn', provolatile => 's',
+  proparallel => 'r', prorettype => 'pg_lsn', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_db_last_commit_lsn' },
 { oid => '1942', descr => 'statistics: transactions committed',
   proname => 'pg_stat_get_db_xact_commit', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => 'oid',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 57a2c0866a..c492759a71 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -18,6 +18,7 @@
 #include "utils/backend_status.h"	/* for backward compatibility */
 #include "utils/relcache.h"
 #include "utils/wait_event.h"	/* for backward compatibility */
+#include "access/xlogdefs.h"
 
 
 /* ----------
@@ -320,6 +321,7 @@ typedef struct PgStat_IO
 
 typedef struct PgStat_StatDBEntry
 {
+	XLogRecPtr	last_commit_lsn;
 	PgStat_Counter xact_commit;
 	PgStat_Counter xact_rollback;
 	PgStat_Counter blocks_fetched;
@@ -702,7 +704,7 @@ extern PgStat_StatSubEntry *pgstat_fetch_stat_subscription(Oid subid);
  * Functions in pgstat_xact.c
  */
 
-extern void AtEOXact_PgStat(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 extern void AtEOSubXact_PgStat(bool isCommit, int nestDepth);
 extern void AtPrepare_PgStat(void);
 extern void PostPrepare_PgStat(void);
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 60fbf9394b..700d9c27bb 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -548,7 +548,7 @@ extern void pgstat_checkpointer_snapshot_cb(void);
 
 extern void pgstat_report_disconnect(Oid dboid);
 extern void pgstat_update_dbstats(TimestampTz ts);
-extern void AtEOXact_PgStat_Database(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 
 extern PgStat_StatDBEntry *pgstat_prep_database_pending(Oid dboid);
 extern void pgstat_reset_database_timestamp(Oid dboid, TimestampTz ts);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7fd81e6a7d..62b909ca92 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1833,6 +1833,7 @@ pg_stat_database| SELECT oid AS datid,
             WHEN (oid = (0)::oid) THEN 0
             ELSE pg_stat_get_db_numbackends(oid)
         END AS numbackends,
+    pg_stat_get_db_last_commit_lsn(oid) AS last_commit_lsn,
     pg_stat_get_db_xact_commit(oid) AS xact_commit,
     pg_stat_get_db_xact_rollback(oid) AS xact_rollback,
     (pg_stat_get_db_blocks_fetched(oid) - pg_stat_get_db_blocks_hit(oid)) AS blks_read,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 8e63340782..fb4dbad15d 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -782,6 +782,30 @@ SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
  t
 (1 row)
 
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+CREATE TABLE test_commit_increments(i int);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+ ?column? 
+----------
+ t
+(1 row)
+
+DROP TABLE test_commit_increments;
 -- Test pg_stat_bgwriter checkpointer-related stats, together with pg_stat_wal
 SELECT checkpoints_req AS rqst_ckpts_before FROM pg_stat_bgwriter \gset
 -- Test pg_stat_wal (and make a temp table so our temp schema exists)
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index fddf5a8277..7f6180826b 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -387,6 +387,16 @@ SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
 SELECT pg_stat_force_next_flush();
 SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
 
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+CREATE TABLE test_commit_increments(i int);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+DROP TABLE test_commit_increments;
+
 -- Test pg_stat_bgwriter checkpointer-related stats, together with pg_stat_wal
 SELECT checkpoints_req AS rqst_ckpts_before FROM pg_stat_bgwriter \gset
 
-- 
2.39.3 (Apple Git-145)

James Coleman

jtc331@gmail.com

almost 2 years ago

In reply to: vignesh C (#3)

Re: Add last_commit_lsn to pg_stat_database

On Sun, Jan 14, 2024 at 6:01 AM vignesh C <vignesh21@gmail.com> wrote:

On Sat, 10 Jun 2023 at 07:57, James Coleman <jtc331@gmail.com> wrote:

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

In the aforementioned thread [1] I'd proposed a patch that added a SQL
function pg_last_commit_lsn() to expose the most recent commit's LSN.
Robert Haas didn't think the initial version's modifications to
commit_ts made sense, and a subsequent approach adding the value to
PGPROC didn't have strong objections, from what I can see, but it also
didn't generate any enthusiasm.

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.

I have changed the status of commitfest entry to "Returned with
Feedback" as Aleksander's comments have not yet been resolved. Please
feel free to post an updated version of the patch and update the
commitfest entry accordingly.

Thanks for reminding me; I'd lost track of this patch.

Regards,
James Coleman

Peter Smith

smithpb2250@gmail.com

almost 2 years ago

In reply to: James Coleman (#5)

Re: Add last_commit_lsn to pg_stat_database

2024-01 Commitfest.

Hi, This patch has a CF status of "Needs Review", but it seems like
there was some CFbot test failure last time it was run [1]https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/46/4355. Please
have a look and post an updated version if necessary.

======
[1]: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/46/4355

Kind Regards,
Peter Smith.

James Coleman

jtc331@gmail.com

almost 2 years ago

In reply to: Peter Smith (#6)

1 attachment(s)

Re: Add last_commit_lsn to pg_stat_database

On Sun, Jan 21, 2024 at 10:26 PM Peter Smith <smithpb2250@gmail.com> wrote:

2024-01 Commitfest.

Hi, This patch has a CF status of "Needs Review", but it seems like
there was some CFbot test failure last time it was run [1]. Please
have a look and post an updated version if necessary.

======
[1] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/46/4355

Kind Regards,
Peter Smith.

Updated patch attached,

Thanks,
James Coleman

Attachments:

v3-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchapplication/octet-stream; name=v3-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchDownload

From 2b06aa776d23acb64cabbb24efee7f9dc63b5762 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 9 Jun 2023 20:12:31 -0500
Subject: [PATCH v3] Add last commit's LSN to pg_stat_database

---
 doc/src/sgml/monitoring.sgml                 |  9 ++++++++
 src/backend/access/transam/twophase.c        | 11 +++++----
 src/backend/access/transam/xact.c            | 11 +++++----
 src/backend/catalog/system_views.sql         |  1 +
 src/backend/utils/activity/pgstat_database.c | 20 +++++++++++++++-
 src/backend/utils/activity/pgstat_xact.c     |  4 ++--
 src/backend/utils/adt/pgstatfuncs.c          |  3 +++
 src/include/catalog/pg_proc.dat              |  4 ++++
 src/include/pgstat.h                         |  4 +++-
 src/include/utils/pgstat_internal.h          |  2 +-
 src/test/regress/expected/rules.out          |  1 +
 src/test/regress/expected/stats.out          | 24 ++++++++++++++++++++
 src/test/regress/sql/stats.sql               | 10 ++++++++
 13 files changed, 90 insertions(+), 14 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 6e74138a69..3eb2e8a8bf 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3248,6 +3248,15 @@ description | Waiting for a newly initialized WAL file to reach durable storage
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_commit_lsn</structfield> <type>pg_lsn</type>
+      </para>
+      <para>
+       Write-ahead log location of the last commit in this database
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>xact_commit</structfield> <type>bigint</type>
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 8426458f7f..2a477aad86 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -201,7 +201,7 @@ static GlobalTransaction MyLockedGxact = NULL;
 
 static bool twophaseExitRegistered = false;
 
-static void RecordTransactionCommitPrepared(TransactionId xid,
+static XLogRecPtr RecordTransactionCommitPrepared(TransactionId xid,
 											int nchildren,
 											TransactionId *children,
 											int nrels,
@@ -1533,6 +1533,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	xl_xact_stats_item *commitstats;
 	xl_xact_stats_item *abortstats;
 	SharedInvalidationMessage *invalmsgs;
+	XLogRecPtr commit_lsn = InvalidXLogRecPtr;
 
 	/*
 	 * Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1588,7 +1589,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	 * callbacks will release the locks the transaction held.
 	 */
 	if (isCommit)
-		RecordTransactionCommitPrepared(xid,
+		commit_lsn = RecordTransactionCommitPrepared(xid,
 										hdr->nsubxacts, children,
 										hdr->ncommitrels, commitrels,
 										hdr->ncommitstats,
@@ -1683,7 +1684,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	LWLockRelease(TwoPhaseStateLock);
 
 	/* Count the prepared xact as committed or aborted */
-	AtEOXact_PgStat(isCommit, false);
+	AtEOXact_PgStat(isCommit, commit_lsn, false);
 
 	/*
 	 * And now we can clean up any files we may have left.
@@ -2319,7 +2320,7 @@ ProcessTwoPhaseBuffer(TransactionId xid,
  * We know the transaction made at least one XLOG entry (its PREPARE),
  * so it is never possible to optimize out the commit record.
  */
-static void
+static XLogRecPtr
 RecordTransactionCommitPrepared(TransactionId xid,
 								int nchildren,
 								TransactionId *children,
@@ -2407,6 +2408,8 @@ RecordTransactionCommitPrepared(TransactionId xid,
 	 * in the procarray and continue to hold locks.
 	 */
 	SyncRepWaitForLSN(recptr, true);
+
+	return recptr;
 }
 
 /*
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 464858117e..a3634b8a4b 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1280,7 +1280,7 @@ AtSubStart_ResourceOwner(void)
  * If you change this function, see RecordTransactionCommitPrepared also.
  */
 static TransactionId
-RecordTransactionCommit(void)
+RecordTransactionCommit(XLogRecPtr *commit_lsn)
 {
 	TransactionId xid = GetTopTransactionIdIfAny();
 	bool		markXidCommitted = TransactionIdIsValid(xid);
@@ -1397,7 +1397,7 @@ RecordTransactionCommit(void)
 		/*
 		 * Insert the commit XLOG record.
 		 */
-		XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
+		*commit_lsn = XactLogCommitRecord(GetCurrentTransactionStopTimestamp(),
 							nchildren, children, nrels, rels,
 							ndroppedstats, droppedstats,
 							nmsgs, invalMessages,
@@ -2153,6 +2153,7 @@ CommitTransaction(void)
 {
 	TransactionState s = CurrentTransactionState;
 	TransactionId latestXid;
+	XLogRecPtr	commit_lsn = InvalidXLogRecPtr;
 	bool		is_parallel_worker;
 
 	is_parallel_worker = (s->blockState == TBLOCK_PARALLEL_INPROGRESS);
@@ -2264,7 +2265,7 @@ CommitTransaction(void)
 		 * We need to mark our XIDs as committed in pg_xact.  This is where we
 		 * durably commit.
 		 */
-		latestXid = RecordTransactionCommit();
+		latestXid = RecordTransactionCommit(&commit_lsn);
 	}
 	else
 	{
@@ -2369,7 +2370,7 @@ CommitTransaction(void)
 	AtEOXact_Files(true);
 	AtEOXact_ComboCid();
 	AtEOXact_HashTables(true);
-	AtEOXact_PgStat(true, is_parallel_worker);
+	AtEOXact_PgStat(true, commit_lsn, is_parallel_worker);
 	AtEOXact_Snapshot(true, false);
 	AtEOXact_ApplyLauncher(true);
 	AtEOXact_LogicalRepWorkers(true);
@@ -2869,7 +2870,7 @@ AbortTransaction(void)
 		AtEOXact_Files(false);
 		AtEOXact_ComboCid();
 		AtEOXact_HashTables(false);
-		AtEOXact_PgStat(false, is_parallel_worker);
+		AtEOXact_PgStat(false, InvalidXLogRecPtr, is_parallel_worker);
 		AtEOXact_ApplyLauncher(false);
 		AtEOXact_LogicalRepWorkers(false);
 		pgstat_report_xact_timestamp(0);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e43e36f5ac..65ea9526e0 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1051,6 +1051,7 @@ CREATE VIEW pg_stat_database AS
                     WHEN (D.oid = (0)::oid) THEN 0
                     ELSE pg_stat_get_db_numbackends(D.oid)
                 END AS numbackends,
+            pg_stat_get_db_last_commit_lsn(D.oid) AS last_commit_lsn,
             pg_stat_get_db_xact_commit(D.oid) AS xact_commit,
             pg_stat_get_db_xact_rollback(D.oid) AS xact_rollback,
             pg_stat_get_db_blocks_fetched(D.oid) -
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 1203c4cbd1..3dcb0cfd83 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -34,6 +34,7 @@ SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
 
 static int	pgStatXactCommit = 0;
 static int	pgStatXactRollback = 0;
+static XLogRecPtr	pgStatLastCommitLSN = InvalidXLogRecPtr;
 static PgStat_Counter pgLastSessionReportTime = 0;
 
 
@@ -246,7 +247,7 @@ pgstat_fetch_stat_dbentry(Oid dboid)
 }
 
 void
-AtEOXact_PgStat_Database(bool isCommit, bool parallel)
+AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	/* Don't count parallel worker transaction stats */
 	if (!parallel)
@@ -256,7 +257,12 @@ AtEOXact_PgStat_Database(bool isCommit, bool parallel)
 		 * bools, in case the reporting message isn't sent right away.)
 		 */
 		if (isCommit)
+		{
 			pgStatXactCommit++;
+			/* For commits also track the LSN of the commit record. */
+			if (!XLogRecPtrIsInvalid(commit_lsn))
+				pgStatLastCommitLSN = commit_lsn;
+		}
 		else
 			pgStatXactRollback++;
 	}
@@ -280,6 +286,13 @@ pgstat_update_dbstats(TimestampTz ts)
 
 	dbentry = pgstat_prep_database_pending(MyDatabaseId);
 
+	/*
+	 * pgStatLastCommitLSN is only ever set when there's a valid commit, so we
+	 * know the LSN must be valid, and since we're local to the current backend
+	 * we don't have to worry if we're advancing or not.
+	 */
+	dbentry->last_commit_lsn = pgStatLastCommitLSN;
+
 	/*
 	 * Accumulate xact commit/rollback and I/O timings to stats entry of the
 	 * current database.
@@ -311,6 +324,7 @@ pgstat_update_dbstats(TimestampTz ts)
 	pgStatBlockWriteTime = 0;
 	pgStatActiveTime = 0;
 	pgStatTransactionIdleTime = 0;
+	/* No reason to zero out pgStatLastCommitLSN; it's not accumulative. */
 }
 
 /*
@@ -427,6 +441,10 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	PGSTAT_ACCUM_DBCOUNT(sessions_killed);
 #undef PGSTAT_ACCUM_DBCOUNT
 
+	/* Only update last_commit_lsn if our backend has the newest commit. */
+	if (pendingent->last_commit_lsn > sharedent->stats.last_commit_lsn)
+		sharedent->stats.last_commit_lsn = pendingent->last_commit_lsn;
+
 	pgstat_unlock_entry(entry_ref);
 
 	memset(pendingent, 0, sizeof(*pendingent));
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 85788ba795..876c788e37 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -38,11 +38,11 @@ static PgStat_SubXactStatus *pgStatXactStack = NULL;
  * Called from access/transam/xact.c at top-level transaction commit/abort.
  */
 void
-AtEOXact_PgStat(bool isCommit, bool parallel)
+AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel)
 {
 	PgStat_SubXactStatus *xact_state;
 
-	AtEOXact_PgStat_Database(isCommit, parallel);
+	AtEOXact_PgStat_Database(isCommit, commit_lsn, parallel);
 
 	/* handle transactional stats information */
 	xact_state = pgStatXactStack;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 30a2063505..a1e28050d2 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1062,6 +1062,9 @@ PG_STAT_GET_DBENTRY_INT64(tuples_returned)
 /* pg_stat_get_db_tuples_updated */
 PG_STAT_GET_DBENTRY_INT64(tuples_updated)
 
+/* pg_stat_get_db_last_commit_lsn */
+PG_STAT_GET_DBENTRY_INT64(last_commit_lsn)
+
 /* pg_stat_get_db_xact_commit */
 PG_STAT_GET_DBENTRY_INT64(xact_commit)
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index ad74e07dbb..29672fc160 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5567,6 +5567,10 @@
   proname => 'pg_stat_get_db_numbackends', provolatile => 's',
   proparallel => 'r', prorettype => 'int4', proargtypes => 'oid',
   prosrc => 'pg_stat_get_db_numbackends' },
+{ oid => '8000', descr => 'statistics: wal location of last committed transaction',
+  proname => 'pg_stat_get_db_last_commit_lsn', provolatile => 's',
+  proparallel => 'r', prorettype => 'pg_lsn', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_db_last_commit_lsn' },
 { oid => '1942', descr => 'statistics: transactions committed',
   proname => 'pg_stat_get_db_xact_commit', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => 'oid',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2136239710..21e50487cd 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -18,6 +18,7 @@
 #include "utils/backend_status.h"	/* for backward compatibility */
 #include "utils/relcache.h"
 #include "utils/wait_event.h"	/* for backward compatibility */
+#include "access/xlogdefs.h"
 
 
 /* ----------
@@ -322,6 +323,7 @@ typedef struct PgStat_IO
 
 typedef struct PgStat_StatDBEntry
 {
+	XLogRecPtr	last_commit_lsn;
 	PgStat_Counter xact_commit;
 	PgStat_Counter xact_rollback;
 	PgStat_Counter blocks_fetched;
@@ -704,7 +706,7 @@ extern PgStat_StatSubEntry *pgstat_fetch_stat_subscription(Oid subid);
  * Functions in pgstat_xact.c
  */
 
-extern void AtEOXact_PgStat(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 extern void AtEOSubXact_PgStat(bool isCommit, int nestDepth);
 extern void AtPrepare_PgStat(void);
 extern void PostPrepare_PgStat(void);
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 0cb8a58cba..e963247791 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -548,7 +548,7 @@ extern void pgstat_checkpointer_snapshot_cb(void);
 
 extern void pgstat_report_disconnect(Oid dboid);
 extern void pgstat_update_dbstats(TimestampTz ts);
-extern void AtEOXact_PgStat_Database(bool isCommit, bool parallel);
+extern void AtEOXact_PgStat_Database(bool isCommit, XLogRecPtr commit_lsn, bool parallel);
 
 extern PgStat_StatDBEntry *pgstat_prep_database_pending(Oid dboid);
 extern void pgstat_reset_database_timestamp(Oid dboid, TimestampTz ts);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 55f2e95352..952922d1fc 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1835,6 +1835,7 @@ pg_stat_database| SELECT oid AS datid,
             WHEN (oid = (0)::oid) THEN 0
             ELSE pg_stat_get_db_numbackends(oid)
         END AS numbackends,
+    pg_stat_get_db_last_commit_lsn(oid) AS last_commit_lsn,
     pg_stat_get_db_xact_commit(oid) AS xact_commit,
     pg_stat_get_db_xact_rollback(oid) AS xact_rollback,
     (pg_stat_get_db_blocks_fetched(oid) - pg_stat_get_db_blocks_hit(oid)) AS blks_read,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 346e10a3d2..eba0805659 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -828,6 +828,30 @@ SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
  t
 (1 row)
 
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+CREATE TABLE test_commit_increments(i int);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+ ?column? 
+----------
+ t
+(1 row)
+
+DROP TABLE test_commit_increments;
 -- Test pg_stat_checkpointer checkpointer-related stats, together with pg_stat_wal
 SELECT num_requested AS rqst_ckpts_before FROM pg_stat_checkpointer \gset
 -- Test pg_stat_wal (and make a temp table so our temp schema exists)
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index e3b4ca96e8..bd2ba05c53 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -417,6 +417,16 @@ SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
 SELECT pg_stat_force_next_flush();
 SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
 
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+CREATE TABLE test_commit_increments(i int);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+DROP TABLE test_commit_increments;
+
 -- Test pg_stat_checkpointer checkpointer-related stats, together with pg_stat_wal
 SELECT num_requested AS rqst_ckpts_before FROM pg_stat_checkpointer \gset
 
-- 
2.39.3 (Apple Git-145)

Tomas Vondra

tomas.vondra@enterprisedb.com

almost 2 years ago

In reply to: James Coleman (#7)

Re: Add last_commit_lsn to pg_stat_database

Hi James,

Thanks for the updated patch. I don't have a clear opinion on the
feature and whether this is the way to implement it, but I have two
simple questions.

1) Do we really need to modify RecordTransactionCommitPrepared and
XactLogCommitRecord to return the LSN of the commit record? Can't we
just use XactLastRecEnd? It's good enough for advancing
replorigin_session_origin_lsn, why wouldn't it be good enough for this?

2) Earlier in this thread it was claimed the function returning the
last_commit_lsn is STABLE, but I have to admit it's not clear to me why
that's the case :-( All the pg_stat_get_db_* functions are marked as
stable, so I guess it's thanks to the pgstat system ...

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Michael Paquier

michael@paquier.xyz

almost 2 years ago

In reply to: Tomas Vondra (#8)

Re: Add last_commit_lsn to pg_stat_database

On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote:

Thanks for the updated patch. I don't have a clear opinion on the
feature and whether this is the way to implement it, but I have two
simple questions.

Some users I know of would be really happy to be able to get this
information for each database in a single view, so that feels natural
to plug the information into pg_stat_database.

1) Do we really need to modify RecordTransactionCommitPrepared and
XactLogCommitRecord to return the LSN of the commit record? Can't we
just use XactLastRecEnd? It's good enough for advancing
replorigin_session_origin_lsn, why wouldn't it be good enough for this?

Or XactLastCommitEnd? The changes in AtEOXact_PgStat() are not really
attractive for what's a static variable in all the backends.

2) Earlier in this thread it was claimed the function returning the
last_commit_lsn is STABLE, but I have to admit it's not clear to me why
that's the case :-( All the pg_stat_get_db_* functions are marked as
stable, so I guess it's thanks to the pgstat system ...

The consistency of the shared stats data depends on
stats_fetch_consistency. The default of "cache" makes sure that the
values don't change across a scan, until the end of a transaction.

I have not paid much attention about that until now, but note that it
would not be the case of "none" where the data is retrieved each time
it is requested. So it seems to me that these functions should be
actually volatile, not stable, because they could deliver different
values across the same scan as an effect of the design behind
pgstat_fetch_entry() and a non-default stats_fetch_consistency. I may
be missing something, of course.
--
Michael

#10

Tomas Vondra

tomas.vondra@enterprisedb.com

almost 2 years ago

In reply to: Michael Paquier (#9)

Re: Add last_commit_lsn to pg_stat_database

On 2/19/24 07:57, Michael Paquier wrote:

On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote:

Thanks for the updated patch. I don't have a clear opinion on the
feature and whether this is the way to implement it, but I have two
simple questions.

Some users I know of would be really happy to be able to get this
information for each database in a single view, so that feels natural
to plug the information into pg_stat_database.

1) Do we really need to modify RecordTransactionCommitPrepared and
XactLogCommitRecord to return the LSN of the commit record? Can't we
just use XactLastRecEnd? It's good enough for advancing
replorigin_session_origin_lsn, why wouldn't it be good enough for this?

Or XactLastCommitEnd?

But that's not set in RecordTransactionCommitPrepared (or twophase.c in
general), and the patch seems to need that.

The changes in AtEOXact_PgStat() are not really
attractive for what's a static variable in all the backends.

I'm sorry, I'm not sure what "changes not being attractive" means :-(

2) Earlier in this thread it was claimed the function returning the
last_commit_lsn is STABLE, but I have to admit it's not clear to me why
that's the case :-( All the pg_stat_get_db_* functions are marked as
stable, so I guess it's thanks to the pgstat system ...

The consistency of the shared stats data depends on
stats_fetch_consistency. The default of "cache" makes sure that the
values don't change across a scan, until the end of a transaction.

I have not paid much attention about that until now, but note that it
would not be the case of "none" where the data is retrieved each time
it is requested. So it seems to me that these functions should be
actually volatile, not stable, because they could deliver different
values across the same scan as an effect of the design behind
pgstat_fetch_entry() and a non-default stats_fetch_consistency. I may
be missing something, of course.

Right. If I do this:

create or replace function get_db_lsn(oid) returns pg_lsn as $$
declare
v_lsn pg_lsn;
begin
select last_commit_lsn into v_lsn from pg_stat_database
where datid = $1;
return v_lsn;
end; $$ language plpgsql;

select min(l), max(l) from (select i, get_db_lsn(16384) AS l from
generate_series(1,100000) s(i)) foo;

and run this concurrently with a pgbench on the same database (with OID
16384), I get:

- stats_fetch_consistency=cache: always the same min/max value

- stats_fetch_consistency=none: min != max

Which would suggest you're right and this should be VOLATILE and not
STABLE. But then again - the existing pg_stat_get_db_* functions are all
marked as STABLE, so (a) is that correct and (b) why should this
function be marked different?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#11

Michael Paquier

michael@paquier.xyz

almost 2 years ago

In reply to: Tomas Vondra (#10)

Re: Add last_commit_lsn to pg_stat_database

On Mon, Feb 19, 2024 at 10:26:43AM +0100, Tomas Vondra wrote:

On 2/19/24 07:57, Michael Paquier wrote:

On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote:

1) Do we really need to modify RecordTransactionCommitPrepared and
XactLogCommitRecord to return the LSN of the commit record? Can't we
just use XactLastRecEnd? It's good enough for advancing
replorigin_session_origin_lsn, why wouldn't it be good enough for this?

Or XactLastCommitEnd?

But that's not set in RecordTransactionCommitPrepared (or twophase.c in
general), and the patch seems to need that.

Hmm. Okay.

The changes in AtEOXact_PgStat() are not really
attractive for what's a static variable in all the backends.

I'm sorry, I'm not sure what "changes not being attractive" means :-(

It just means that I am not much a fan of the signature changes done
for RecordTransactionCommit() and AtEOXact_PgStat_Database(), adding a
dependency to a specify LSN. Your suggestion to switching to
XactLastRecEnd should avoid that.

- stats_fetch_consistency=cache: always the same min/max value

- stats_fetch_consistency=none: min != max

Which would suggest you're right and this should be VOLATILE and not
STABLE. But then again - the existing pg_stat_get_db_* functions are all
marked as STABLE, so (a) is that correct and (b) why should this
function be marked different?

This can look like an oversight of 5891c7a8ed8f to me. I've skimmed
through the threads related to this commit and messages around [1]
explain why this GUC exists and why we have both behaviors, but I'm
not seeing a discussion about the volatibility of these functions.
The current situation is not bad either, the default makes these
functions stable, and I'd like to assume that users of this GUC know
what they do. Perhaps Andres or Horiguchi-san can comment on that.

/messages/by-id/382908.1616343275@sss.pgh.pa.us
--
Michael

#12

Heikki Linnakangas

hlinnaka@iki.fi

almost 2 years ago

In reply to: Michael Paquier (#11)

Re: Add last_commit_lsn to pg_stat_database

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

I'm not sure I fully understand the problem. What are you doing
currently to measure the lag? If you look at pg_replication_slots today,
confirmed_flush_lsn advances also when you do pg_switch_wal(), so
looking at the diff between confirmed_flush_lsn and pg_current_wal_lsn()
works, no?

And on the other hand, even if you expose the database's last commit
LSN, you can have an publication that includes only a subset of tables.
Or commits that don't write to any table at all. So I'm not sure why the
database's last commit LSN is relevant. Getting the last LSN that did
something that needs to be replicated through the publication might be
useful, but that's not what what this patch does.

--
Heikki Linnakangas
Neon (https://neon.tech)

#13

Cary Huang

cary.huang@highgo.ca

almost 2 years ago

In reply to: Heikki Linnakangas (#12)

Re: Add last_commit_lsn to pg_stat_database

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested

Hello

I think it is convenient to know the last commit LSN of a database for troubleshooting and monitoring purposes similar to the "pd_lsn" field in a page header that records the last LSN that modifies this page. I am not sure if it can help determine the WAL location to resume / catch up in logical replication as the "confirmed_flush_lsn" and "restart_lsn" in a logical replication slot are already there to figure out the resume / catchup point as I understand. Anyhow, I had a look at the patch, in general it looks good to me. Also ran it against the CI bot, which also turned out fine. Just one small comment though. The patch supports the recording of last commit lsn from 2 phase commit as well, but the test does not seem to have a test on 2 phase commit. In my opinion, it should test whether the last commit lsn increments when a prepared transaction is committed in addition to a regular transaction.

thank you
-----------------
Cary Huang
www.highgo.ca

#14

Kirill Reshke

reshkekirill@gmail.com

almost 2 years ago

In reply to: James Coleman (#1)

Re: Add last_commit_lsn to pg_stat_database

Hi James,

There are some review in the thread that need to be addressed.
it seems that we need to mark this entry "Waiting on Author" and move to
the next CF [0]https://commitfest.postgresql.org/47/4355/.

Thanks

[0]: https://commitfest.postgresql.org/47/4355/

On Sat, 10 Jun 2023 at 05:27, James Coleman <jtc331@gmail.com> wrote:

Show quoted text

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

In the aforementioned thread [1] I'd proposed a patch that added a SQL
function pg_last_commit_lsn() to expose the most recent commit's LSN.
Robert Haas didn't think the initial version's modifications to
commit_ts made sense, and a subsequent approach adding the value to
PGPROC didn't have strong objections, from what I can see, but it also
didn't generate any enthusiasm.

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.

Regards,
James Coleman

1:
/messages/by-id/CAAaqYe9QBiAu+j8rBun_JKBRe-3HeKLUhfVVsYfsxQG0VqLXsA@mail.gmail.com

#15

James Coleman

jtc331@gmail.com

over 1 year ago

In reply to: Heikki Linnakangas (#12)

Re: Add last_commit_lsn to pg_stat_database

On Thu, Mar 7, 2024 at 10:30 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

I've previously noted in "Add last commit LSN to
pg_last_committed_xact()" [1] that it's not possible to monitor how
many bytes of WAL behind a logical replication slot is (computing such
is obviously trivial for physical slots) because the slot doesn't need
to replicate beyond the last commit. In some cases it's possible for
the current WAL location to be far beyond the last commit. A few
examples:

- An idle server with checkout_timeout at a lower value (so lots of
empty WAL advance).
- Very large transactions: particularly insidious because committing a
1 GB transaction after a small transaction may show almost zero time
lag even though quite a bit of data needs to be processed and sent
over the wire (so time to replay is significantly different from
current lag).
- A cluster with multiple databases complicates matters further,
because while physical replication is cluster-wide, the LSNs that
matter for logical replication are database specific.

Since we don't expose the most recent commit's LSN there's no way to
say "the WAL is currently 1250, the last commit happened at 1000, the
slot has flushed up to 800, therefore there are at most 200 bytes
replication needs to read through to catch up.

I'm not sure I fully understand the problem. What are you doing
currently to measure the lag? If you look at pg_replication_slots today,
confirmed_flush_lsn advances also when you do pg_switch_wal(), so
looking at the diff between confirmed_flush_lsn and pg_current_wal_lsn()
works, no?

No, it's not that simple because of the "large, uncommitted
transaction" case I noted in the OP. Suppose I have a batch system,
and a job in that system has written 1 GB of data but not yet
committed (or rolled back). In this case confirmed_flush_lsn cannot
advance, correct?

And so having a "last commit LSN" allows me to know how far back the
"last possibly replicatable write"

And on the other hand, even if you expose the database's last commit
LSN, you can have an publication that includes only a subset of tables.
Or commits that don't write to any table at all. So I'm not sure why the
database's last commit LSN is relevant. Getting the last LSN that did
something that needs to be replicated through the publication might be
useful, but that's not what what this patch does.

I think that's fine, because as you noted earlier the
confirmed_flush_lsn could advance beyond that point anyway (if there's
nothing to replicate), but in the case where:

1. confirmed_flush_lsn is not advancing, and
2. last_commit_lsn is not advancing, and
3. pg_current_wal_lsn() has advanced a lot

then you can probably infer that there's a large amount of data that
simply cannot be completed by the subscriber, and so there's no "real"
delay. It also gives you an idea of how much data you will need to
churn through (even if not replicated) once the transaction commits.

Certainly understanding the data here will be simplest in the case
where 1.) there's a single database and 2.) all tables are in the
replication set, but I don't think the value is limited to that
situation either.

Regards,
James Coleman

#16

James Coleman

jtc331@gmail.com

over 1 year ago

In reply to: Michael Paquier (#11)

1 attachment(s)

Re: Add last_commit_lsn to pg_stat_database

On Mon, Feb 19, 2024 at 3:56 PM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Feb 19, 2024 at 10:26:43AM +0100, Tomas Vondra wrote:

On 2/19/24 07:57, Michael Paquier wrote:

On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote:

1) Do we really need to modify RecordTransactionCommitPrepared and
XactLogCommitRecord to return the LSN of the commit record? Can't we
just use XactLastRecEnd? It's good enough for advancing
replorigin_session_origin_lsn, why wouldn't it be good enough for this?

Or XactLastCommitEnd?

But that's not set in RecordTransactionCommitPrepared (or twophase.c in
general), and the patch seems to need that.

Hmm. Okay.

The changes in AtEOXact_PgStat() are not really
attractive for what's a static variable in all the backends.

I'm sorry, I'm not sure what "changes not being attractive" means :-(

It just means that I am not much a fan of the signature changes done
for RecordTransactionCommit() and AtEOXact_PgStat_Database(), adding a
dependency to a specify LSN. Your suggestion to switching to
XactLastRecEnd should avoid that.

Attached is an updated patch that uses XactLastCommitEnd and therefore
avoids all of those signature changes.

We can't use XactLastCommitEnd because it gets set to 0 immediately
after RecordTransactionCommit() sets XactLastCommitEnd to its value.

I added a test for two-phase commit, as well, and in so doing I
discovered something that I found curious: when I do "COMMIT PREPARED
't1'", not only does RecordTransactionCommitPrepared() get called, but
eventually RecordTransactionCommit() is called as well before the
command returns (despite the comments for those functions describing
them as two equivalents you need to change at the same time).

So it appears that we don't need to set XactLastCommitEnd in
RecordTransactionCommitPrepared() for this to work, and, indeed,
adding some logging to verify, the value of XactLastRecEnd we'd use to
update XactLastCommitEnd is the same at the end of both of those
functions during COMMIT PREPARED.

I'd like to have someone weigh in on whether relying on
RecordTransactionCommit() being called during COMMIT PREPARED is
correct or not (if perchance there are non-obvious reasons why we
shouldn't).

Regards,
James Coleman

Attachments:

v4-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchapplication/octet-stream; name=v4-0001-Add-last-commit-s-LSN-to-pg_stat_database.patchDownload

From 3d984d3881f3757b280c043043d365092c41b6a9 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Fri, 9 Jun 2023 20:12:31 -0500
Subject: [PATCH v4] Add last commit's LSN to pg_stat_database

---
 doc/src/sgml/monitoring.sgml                 |  9 ++++
 src/backend/catalog/system_views.sql         |  1 +
 src/backend/utils/activity/pgstat_database.c | 12 +++++
 src/backend/utils/adt/pgstatfuncs.c          |  3 ++
 src/include/catalog/pg_proc.dat              |  4 ++
 src/include/pgstat.h                         |  2 +
 src/test/regress/expected/rules.out          |  1 +
 src/test/regress/expected/stats.out          | 49 ++++++++++++++++++++
 src/test/regress/sql/stats.sql               | 24 ++++++++++
 9 files changed, 105 insertions(+)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 053da8d6e4..25190389a3 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3249,6 +3249,15 @@ description | Waiting for a newly initialized WAL file to reach durable storage
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_commit_lsn</structfield> <type>pg_lsn</type>
+      </para>
+      <para>
+       Write-ahead log location of the last commit in this database
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>xact_commit</structfield> <type>bigint</type>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 53047cab5f..81d3f415b8 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1052,6 +1052,7 @@ CREATE VIEW pg_stat_database AS
                     WHEN (D.oid = (0)::oid) THEN 0
                     ELSE pg_stat_get_db_numbackends(D.oid)
                 END AS numbackends,
+            pg_stat_get_db_last_commit_lsn(D.oid) AS last_commit_lsn,
             pg_stat_get_db_xact_commit(D.oid) AS xact_commit,
             pg_stat_get_db_xact_rollback(D.oid) AS xact_rollback,
             pg_stat_get_db_blocks_fetched(D.oid) -
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index 29bc090974..a01754ea85 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -20,6 +20,7 @@
 #include "storage/procsignal.h"
 #include "utils/pgstat_internal.h"
 #include "utils/timestamp.h"
+#include "access/xlog.h"
 
 
 static bool pgstat_should_report_connstat(void);
@@ -280,6 +281,13 @@ pgstat_update_dbstats(TimestampTz ts)
 
 	dbentry = pgstat_prep_database_pending(MyDatabaseId);
 
+	/*
+	 * Track the LSN of the most recent commit. Since we're local to the
+	 * current backend we don't have to worry if we're advancing or not.
+	 */
+	if (!XLogRecPtrIsInvalid(XactLastCommitEnd))
+		dbentry->last_commit_lsn = XactLastCommitEnd;
+
 	/*
 	 * Accumulate xact commit/rollback and I/O timings to stats entry of the
 	 * current database.
@@ -427,6 +435,10 @@ pgstat_database_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	PGSTAT_ACCUM_DBCOUNT(sessions_killed);
 #undef PGSTAT_ACCUM_DBCOUNT
 
+	/* Only update last_commit_lsn if our backend has the newest commit. */
+	if (pendingent->last_commit_lsn > sharedent->stats.last_commit_lsn)
+		sharedent->stats.last_commit_lsn = pendingent->last_commit_lsn;
+
 	pgstat_unlock_entry(entry_ref);
 
 	memset(pendingent, 0, sizeof(*pendingent));
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 3876339ee1..44194567f7 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1060,6 +1060,9 @@ PG_STAT_GET_DBENTRY_INT64(tuples_returned)
 /* pg_stat_get_db_tuples_updated */
 PG_STAT_GET_DBENTRY_INT64(tuples_updated)
 
+/* pg_stat_get_db_last_commit_lsn */
+PG_STAT_GET_DBENTRY_INT64(last_commit_lsn)
+
 /* pg_stat_get_db_xact_commit */
 PG_STAT_GET_DBENTRY_INT64(xact_commit)
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6a5476d3c4..9495a449b8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5581,6 +5581,10 @@
   proname => 'pg_stat_get_db_numbackends', provolatile => 's',
   proparallel => 'r', prorettype => 'int4', proargtypes => 'oid',
   prosrc => 'pg_stat_get_db_numbackends' },
+{ oid => '8000', descr => 'statistics: wal location of last committed transaction',
+  proname => 'pg_stat_get_db_last_commit_lsn', provolatile => 's',
+  proparallel => 'r', prorettype => 'pg_lsn', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_db_last_commit_lsn' },
 { oid => '1942', descr => 'statistics: transactions committed',
   proname => 'pg_stat_get_db_xact_commit', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => 'oid',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2136239710..19ab016904 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -18,6 +18,7 @@
 #include "utils/backend_status.h"	/* for backward compatibility */
 #include "utils/relcache.h"
 #include "utils/wait_event.h"	/* for backward compatibility */
+#include "access/xlogdefs.h"
 
 
 /* ----------
@@ -322,6 +323,7 @@ typedef struct PgStat_IO
 
 typedef struct PgStat_StatDBEntry
 {
+	XLogRecPtr	last_commit_lsn;
 	PgStat_Counter xact_commit;
 	PgStat_Counter xact_rollback;
 	PgStat_Counter blocks_fetched;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ef658ad740..2f650b1e23 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1839,6 +1839,7 @@ pg_stat_database| SELECT oid AS datid,
             WHEN (oid = (0)::oid) THEN 0
             ELSE pg_stat_get_db_numbackends(oid)
         END AS numbackends,
+    pg_stat_get_db_last_commit_lsn(oid) AS last_commit_lsn,
     pg_stat_get_db_xact_commit(oid) AS xact_commit,
     pg_stat_get_db_xact_rollback(oid) AS xact_rollback,
     (pg_stat_get_db_blocks_fetched(oid) - pg_stat_get_db_blocks_hit(oid)) AS blks_read,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 6e08898b18..359f2f18da 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -828,6 +828,55 @@ SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
  t
 (1 row)
 
+CREATE TABLE test_commit_increments(i int);
+-- Test that last_commit_lsn is incremented when a two-phase transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn_2pc FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+INSERT INTO test_commit_increments(i) VALUES (1);
+PREPARE TRANSACTION 't1';
+COMMIT PREPARED 't1';
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn_2pc'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :'db_stat_last_commit_lsn_2pc'::pg_lsn < pg_current_wal_insert_lsn();
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+INSERT INTO test_commit_increments(i) VALUES (1);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+ ?column? 
+----------
+ t
+(1 row)
+
+DROP TABLE test_commit_increments;
 -- Test pg_stat_checkpointer checkpointer-related stats, together with pg_stat_wal
 SELECT num_requested AS rqst_ckpts_before FROM pg_stat_checkpointer \gset
 -- Test pg_stat_wal (and make a temp table so our temp schema exists)
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index d8ac0d06f4..5c4509baf6 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -417,6 +417,30 @@ SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = (SELEC
 SELECT pg_stat_force_next_flush();
 SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
 
+CREATE TABLE test_commit_increments(i int);
+
+-- Test that last_commit_lsn is incremented when a two-phase transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn_2pc FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+INSERT INTO test_commit_increments(i) VALUES (1);
+PREPARE TRANSACTION 't1';
+COMMIT PREPARED 't1';
+SELECT pg_stat_force_next_flush();
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn_2pc'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT :'db_stat_last_commit_lsn_2pc'::pg_lsn < pg_current_wal_insert_lsn();
+
+
+-- Test that last_commit_lsn is incremented when a transaction commits
+SELECT last_commit_lsn AS db_stat_last_commit_lsn FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+BEGIN;
+INSERT INTO test_commit_increments(i) VALUES (1);
+COMMIT;
+SELECT pg_stat_force_next_flush();
+SELECT last_commit_lsn > :'db_stat_last_commit_lsn'::pg_lsn FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT :'db_stat_last_commit_lsn'::pg_lsn < pg_current_wal_insert_lsn();
+
+DROP TABLE test_commit_increments;
+
 -- Test pg_stat_checkpointer checkpointer-related stats, together with pg_stat_wal
 SELECT num_requested AS rqst_ckpts_before FROM pg_stat_checkpointer \gset
 
-- 
2.39.3 (Apple Git-146)

#17

Kyotaro Horiguchi

horikyota.ntt@gmail.com

over 1 year ago

In reply to: Michael Paquier (#11)

Re: Add last_commit_lsn to pg_stat_database

At Tue, 20 Feb 2024 07:56:28 +0900, Michael Paquier <michael@paquier.xyz> wrote in

On Mon, Feb 19, 2024 at 10:26:43AM +0100, Tomas Vondra wrote:
It just means that I am not much a fan of the signature changes done
for RecordTransactionCommit() and AtEOXact_PgStat_Database(), adding a
dependency to a specify LSN. Your suggestion to switching to
XactLastRecEnd should avoid that.

- stats_fetch_consistency=cache: always the same min/max value

- stats_fetch_consistency=none: min != max

Which would suggest you're right and this should be VOLATILE and not
STABLE. But then again - the existing pg_stat_get_db_* functions are all
marked as STABLE, so (a) is that correct and (b) why should this
function be marked different?

This can look like an oversight of 5891c7a8ed8f to me. I've skimmed
through the threads related to this commit and messages around [1]
explain why this GUC exists and why we have both behaviors, but I'm
not seeing a discussion about the volatibility of these functions.
The current situation is not bad either, the default makes these
functions stable, and I'd like to assume that users of this GUC know
what they do. Perhaps Andres or Horiguchi-san can comment on that.

/messages/by-id/382908.1616343275@sss.pgh.pa.us

I agree that we cannot achieve, nor do we need, perfect MVCC behavior,
and that completely volatile behavior is unusable. I believe the
functions are kept marked as stable, as this is the nearest and most
usable volatility for the default behavior, since function volatility
is not switchable on-the-fly. This approach seems least trouble-prone
to me.

The consistency of the functions are discussed here.

https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-STATS-VIEWS

This is a feature, not a bug, because ... Conversely, if it's known
that statistics are only accessed once, caching accessed statistics is
unnecessary and can be avoided by setting stats_fetch_consistency to
none.

It seems to me that this description already implies such an
incongruity in the functions' behavior from the "stable" behavior, but
we might want to explicitly mention that incongruity.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center