Add new wait event to XactLockTableWait

Started by Xuneng Zhou7 months ago12 messages
#1Xuneng Zhou
xunengzhou@gmail.com
1 attachment(s)

Hi hackers,

Currently, when XactLockTableWait() and ConditionalXactLockTableWait()
sleep waiting for transactions to complete, they don't report any
specific wait event to the statistics system. This means that backends
stuck in these waits show up in pg_stat_activity with NULL
wait_event_type and wait_event columns, making it difficult for users
to understand what's actually happening.

This is more problematic in logical replication scenarios where these
waits can be very long - for example, when creating a logical
replication slot on a busy system. Without a specific wait event, it's
hard to distinguish legitimate wait from other issues.

Based on suggestions from Fujii and Kevin [1]/messages/by-id/CAM45KeELdjhS-rGuvN=ZLJ_asvZACucZ9LZWVzH7bGcD12DDwg@mail.gmail.com, the patch introduces
WAIT_EVENT_XACT_DONE ("Waiting for a transaction to commit or abort")
and instructs both functions to report this event during their
pg_usleep() calls With patch applied, when backends are waiting in
these functions, pg_stat_activity will show what they're waiting for.

Head:
postgres=# SELECT pg_is_in_recovery();
pg_is_in_recovery
-------------------
t
(1 row)
postgres=# SELECT pid,wait_event_type,wait_event,state,query FROM
pg_stat_activity WHERE pid=5074;
pid | wait_event_type | wait_event | state |
query
------+-----------------+------------+--------+----------------------------------------------------------------
5074 | | | active | SELECT
pg_create_logical_replication_slot('wow1', 'pgoutput');

With patch applied:
testdb=# SELECT pid, wait_event_type, wait_event, state, query FROM
pg_stat_activity WHERE pid = 62774;
pid | wait_event_type | wait_event | state |
query

-------+-----------------+------------+--------+------------------------------------------------------------------

62774 | IPC | XactDone | active | SELECT *

| | | | FROM
pg_create_logical_replication_slot('my_slot','pgoutput');

(1 row)

[1]: /messages/by-id/CAM45KeELdjhS-rGuvN=ZLJ_asvZACucZ9LZWVzH7bGcD12DDwg@mail.gmail.com

Best regards,
Xuneng

Attachments:

0001-Add-new-wait-event-to-XactLockTableWait.patchapplication/octet-stream; name=0001-Add-new-wait-event-to-XactLockTableWait.patchDownload
From a99ad22010d5b779b7c24c981ca1a025cc1cad3b Mon Sep 17 00:00:00 2001
From: alterego665 <824662526@qq.com>
Date: Wed, 4 Jun 2025 16:37:31 +0800
Subject: [PATCH] Add WAIT_EVENT_XACT_DONE

XactLockTableWait and ConditionalXactLockTableWait previously lacked a
specific wait event, making backend states less transparent. This commit
introduces WAIT_EVENT_XACT_DONE, which both functions now report to
pg_stat_activity during their sleep phases, enhancing visibility.
---
 src/backend/storage/lmgr/lmgr.c                 | 4 ++++
 src/backend/utils/activity/wait_event_names.txt | 1 +
 2 files changed, 5 insertions(+)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..a35de2c497a 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -719,7 +719,9 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
@@ -762,7 +764,9 @@ ConditionalXactLockTableWait(TransactionId xid, bool logLockFailure)
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 4da68312b5f..3da08b853c3 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -161,6 +161,7 @@ WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at transaction end."
+XACT_DONE	"Waiting for a transaction to commit or abort."
 
 ABI_compatibility:
 
-- 
2.48.1

#2Michael Paquier
michael@paquier.xyz
In reply to: Xuneng Zhou (#1)
Re: Add new wait event to XactLockTableWait

On Sun, Jun 08, 2025 at 10:30:45PM +0800, Xuneng Zhou wrote:

This is more problematic in logical replication scenarios where these
waits can be very long - for example, when creating a logical
replication slot on a busy system. Without a specific wait event, it's
hard to distinguish legitimate wait from other issues.

Gotcha.

Based on suggestions from Fujii and Kevin [1], the patch introduces
WAIT_EVENT_XACT_DONE ("Waiting for a transaction to commit or abort")
and instructs both functions to report this event during their
pg_usleep() calls With patch applied, when backends are waiting in
these functions, pg_stat_activity will show what they're waiting for.

+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
[...]
+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE); 

Wouldn't it be better to use two wait events named differently to be
able to make the difference between the two code paths?
--
Michael

#3Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: Michael Paquier (#2)
Re: Add new wait event to XactLockTableWait

On 2025/06/09 7:41, Michael Paquier wrote:

On Sun, Jun 08, 2025 at 10:30:45PM +0800, Xuneng Zhou wrote:

This is more problematic in logical replication scenarios where these
waits can be very long - for example, when creating a logical
replication slot on a busy system. Without a specific wait event, it's
hard to distinguish legitimate wait from other issues.

Gotcha.

Based on suggestions from Fujii and Kevin [1], the patch introduces
WAIT_EVENT_XACT_DONE ("Waiting for a transaction to commit or abort")
and instructs both functions to report this event during their
pg_usleep() calls With patch applied, when backends are waiting in
these functions, pg_stat_activity will show what they're waiting for.

+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
[...]
+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);

Wouldn't it be better to use two wait events named differently to be
able to make the difference between the two code paths?

I think it's fine to use the same wait event, since both code paths
are waiting for the same thing, even though they're in different places.

Here are a few review comments on the patch:

WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for streaming replication."
WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated."
XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at transaction end."
+XACT_DONE "Waiting for a transaction to commit or abort."

XACT_DONE should be listed before XACT_GROUP_UPDATE to maintain
alphabetical order.

Also, for the existing description:
transactionid "Waiting for a transaction to finish."

This could be confusing alongside XACT_DONE. Maybe update it to
something like:

"Waiting to acquire a transaction ID lock; see <xref linkend='transaction-id'/>."

This would help users better understand the difference between
the two wait events.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

#4Xuneng Zhou
xunengzhou@gmail.com
In reply to: Xuneng Zhou (#1)
Re: Add new wait event to XactLockTableWait

Just CC.

Show quoted text

On Mon, Jun 9, 2025 at 10:57 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:

Hi Michael,

Thanks for reviewing.

On Mon, Jun 9, 2025 at 6:41 AM Michael Paquier <michael@paquier.xyz> wrote:

+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
[...]
+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);

Wouldn't it be better to use two wait events named differently to be
able to make the difference between the two code paths?
--

Both `XactLockTableWait()` and its conditional sibling ultimately
block on the same thing: “other transaction must commit or abort
before I can proceed.” I think that using one identifier might keep
the catalog simple.

Best regards,
Xuneng

#5Xuneng Zhou
xunengzhou@gmail.com
In reply to: Fujii Masao (#3)
1 attachment(s)
Re: Add new wait event to XactLockTableWait

Hi Fujii-san,

Thanks for reviewing.

On 2025/06/09 7:41, Michael Paquier wrote:

On Sun, Jun 08, 2025 at 10:30:45PM +0800, Xuneng Zhou wrote:

This is more problematic in logical replication scenarios where these
waits can be very long - for example, when creating a logical
replication slot on a busy system. Without a specific wait event, it's
hard to distinguish legitimate wait from other issues.

Gotcha.

Based on suggestions from Fujii and Kevin [1], the patch introduces
WAIT_EVENT_XACT_DONE ("Waiting for a transaction to commit or abort")
and instructs both functions to report this event during their
pg_usleep() calls With patch applied, when backends are waiting in
these functions, pg_stat_activity will show what they're waiting for.

+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
[...]
+     pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);

Wouldn't it be better to use two wait events named differently to be
able to make the difference between the two code paths?

I think it's fine to use the same wait event, since both code paths
are waiting for the same thing, even though they're in different places.

+1

Here are a few review comments on the patch:

WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for streaming replication."
WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated."
XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at transaction end."
+XACT_DONE "Waiting for a transaction to commit or abort."

XACT_DONE should be listed before XACT_GROUP_UPDATE to maintain
alphabetical order.

Done.

Also, for the existing description:
transactionid "Waiting for a transaction to finish."

This could be confusing alongside XACT_DONE. Maybe update it to
something like:

"Waiting to acquire a transaction ID lock; see <xref linkend='transaction-id'/>."

This would help users better understand the difference between
the two wait events.

I think this is clearer.

Please find attached Version 2, incorporating the suggested changes.

Attachments:

0002-Add-new-wait-event-to-XactLockTableWait functions.patchapplication/octet-stream; name="0002-Add-new-wait-event-to-XactLockTableWait functions.patch"Download
From 3ef2f72689d30bf78c32c297f277c8370a3b0a94 Mon Sep 17 00:00:00 2001
From: alterego665 <824662526@qq.com>
Date: Mon, 9 Jun 2025 11:24:08 +0800
Subject: [PATCH] Add WAIT_EVENT_XACT_DONE to XactLockTableWait functions

XactLockTableWait and ConditionalXactLockTableWait previously lacked a
specific wait event, making backend states less transparent. This commit
introduces WAIT_EVENT_XACT_DONE, which both functions now report to
pg_stat_activity during their sleep phases, enhancing visibility.
---
 src/backend/storage/lmgr/lmgr.c                 | 4 ++++
 src/backend/utils/activity/wait_event_names.txt | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..a35de2c497a 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -719,7 +719,9 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
@@ -762,7 +764,9 @@ ConditionalXactLockTableWait(TransactionId xid, bool logLockFailure)
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 4da68312b5f..9be14973535 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -160,6 +160,7 @@ WAL_BUFFER_INIT	"Waiting on WAL buffer to be initialized."
 WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
+XACT_DONE	"Waiting for a transaction to commit or abort."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at transaction end."
 
 ABI_compatibility:
@@ -419,7 +420,7 @@ extend	"Waiting to extend a relation."
 frozenid	"Waiting to update <structname>pg_database</structname>.<structfield>datfrozenxid</structfield> and <structname>pg_database</structname>.<structfield>datminmxid</structfield>."
 page	"Waiting to acquire a lock on a page of a relation."
 tuple	"Waiting to acquire a lock on a tuple."
-transactionid	"Waiting for a transaction to finish."
+transactionid	"Waiting to acquire a transaction ID lock; see <xref linkend='transaction-id'/>."
 virtualxid	"Waiting to acquire a virtual transaction ID lock; see <xref linkend="transaction-id"/>."
 spectoken	"Waiting to acquire a speculative insertion lock."
 object	"Waiting to acquire a lock on a non-relation database object."
-- 
2.48.1

#6Xuneng Zhou
xunengzhou@gmail.com
In reply to: Xuneng Zhou (#5)
1 attachment(s)
Re: Add new wait event to XactLockTableWait

Hi,

Please find attached Version 2, incorporating the suggested changes.

Apologies for the confusion — in the previous attempt, I mistakenly
named the patch file with a `0002-` prefix, thinking it reflected the
patch version rather than the patch series number. I've corrected the
filename to follow the proper convention.

Best regards,
Xuneng

Attachments:

v2-0001-Add-new-wait-event-to-XactLockTableWait functions.patchapplication/octet-stream; name="v2-0001-Add-new-wait-event-to-XactLockTableWait functions.patch"Download
From 3ef2f72689d30bf78c32c297f277c8370a3b0a94 Mon Sep 17 00:00:00 2001
From: alterego665 <824662526@qq.com>
Date: Mon, 9 Jun 2025 11:24:08 +0800
Subject: [PATCH v2] Add WAIT_EVENT_XACT_DONE to XactLockTableWait functions

XactLockTableWait and ConditionalXactLockTableWait previously lacked a
specific wait event, making backend states less transparent. This commit
introduces WAIT_EVENT_XACT_DONE, which both functions now report to
pg_stat_activity during their sleep phases, enhancing visibility.
---
 src/backend/storage/lmgr/lmgr.c                 | 4 ++++
 src/backend/utils/activity/wait_event_names.txt | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..a35de2c497a 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -719,7 +719,9 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
@@ -762,7 +764,9 @@ ConditionalXactLockTableWait(TransactionId xid, bool logLockFailure)
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 4da68312b5f..9be14973535 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -160,6 +160,7 @@ WAL_BUFFER_INIT	"Waiting on WAL buffer to be initialized."
 WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
+XACT_DONE	"Waiting for a transaction to commit or abort."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at transaction end."
 
 ABI_compatibility:
@@ -419,7 +420,7 @@ extend	"Waiting to extend a relation."
 frozenid	"Waiting to update <structname>pg_database</structname>.<structfield>datfrozenxid</structfield> and <structname>pg_database</structname>.<structfield>datminmxid</structfield>."
 page	"Waiting to acquire a lock on a page of a relation."
 tuple	"Waiting to acquire a lock on a tuple."
-transactionid	"Waiting for a transaction to finish."
+transactionid	"Waiting to acquire a transaction ID lock; see <xref linkend='transaction-id'/>."
 virtualxid	"Waiting to acquire a virtual transaction ID lock; see <xref linkend="transaction-id"/>."
 spectoken	"Waiting to acquire a speculative insertion lock."
 object	"Waiting to acquire a lock on a non-relation database object."
-- 
2.48.1

#7Xuneng Zhou
xunengzhou@gmail.com
In reply to: Xuneng Zhou (#6)
1 attachment(s)
Re: Add new wait event to XactLockTableWait

Hi,

On Mon, Jun 9, 2025 at 12:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

Hi,

Please find attached Version 2, incorporating the suggested changes.

Apologies for the confusion — in the previous attempt, I mistakenly
named the patch file with a `0002-` prefix, thinking it reflected the
patch version rather than the patch series number. I've corrected the
filename to follow the proper convention.

Sorry for the missing hyphen in the patch name. Things should be in good
shape now. I need to be more cautious and double-check everything before
submitting patches and sending emails.😂

Best regards,
Xuneng

Attachments:

v2-0001-Add-new-wait-event-to-XactLockTableWait-functions.patchapplication/octet-stream; name=v2-0001-Add-new-wait-event-to-XactLockTableWait-functions.patchDownload
From 3ef2f72689d30bf78c32c297f277c8370a3b0a94 Mon Sep 17 00:00:00 2001
From: alterego665 <824662526@qq.com>
Date: Mon, 9 Jun 2025 11:24:08 +0800
Subject: [PATCH v2] Add WAIT_EVENT_XACT_DONE to XactLockTableWait functions

XactLockTableWait and ConditionalXactLockTableWait previously lacked a
specific wait event, making backend states less transparent. This commit
introduces WAIT_EVENT_XACT_DONE, which both functions now report to
pg_stat_activity during their sleep phases, enhancing visibility.
---
 src/backend/storage/lmgr/lmgr.c                 | 4 ++++
 src/backend/utils/activity/wait_event_names.txt | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..a35de2c497a 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -719,7 +719,9 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
@@ -762,7 +764,9 @@ ConditionalXactLockTableWait(TransactionId xid, bool logLockFailure)
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_DONE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 4da68312b5f..9be14973535 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -160,6 +160,7 @@ WAL_BUFFER_INIT	"Waiting on WAL buffer to be initialized."
 WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
+XACT_DONE	"Waiting for a transaction to commit or abort."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at transaction end."
 
 ABI_compatibility:
@@ -419,7 +420,7 @@ extend	"Waiting to extend a relation."
 frozenid	"Waiting to update <structname>pg_database</structname>.<structfield>datfrozenxid</structfield> and <structname>pg_database</structname>.<structfield>datminmxid</structfield>."
 page	"Waiting to acquire a lock on a page of a relation."
 tuple	"Waiting to acquire a lock on a tuple."
-transactionid	"Waiting for a transaction to finish."
+transactionid	"Waiting to acquire a transaction ID lock; see <xref linkend='transaction-id'/>."
 virtualxid	"Waiting to acquire a virtual transaction ID lock; see <xref linkend="transaction-id"/>."
 spectoken	"Waiting to acquire a speculative insertion lock."
 object	"Waiting to acquire a lock on a non-relation database object."
-- 
2.48.1

#8Fujii Masao
masao.fujii@oss.nttdata.com
In reply to: Xuneng Zhou (#7)
Re: Add new wait event to XactLockTableWait

On 2025/06/09 14:19, Xuneng Zhou wrote:

Hi,

On Mon, Jun 9, 2025 at 12:52 PM Xuneng Zhou <xunengzhou@gmail.com <mailto:xunengzhou@gmail.com>> wrote:

Hi,

Please find attached Version 2, incorporating the suggested changes.

Apologies for the confusion — in the previous attempt, I mistakenly
named the patch file with a `0002-` prefix, thinking it reflected the
patch version rather than the patch series number.  I've corrected the
filename to follow the proper convention.

Sorry for the missing hyphen in the patch name. Things should be in good shape now. I need to be more cautious and double-check everything before submitting patches and sending emails.😂

Thanks for updating the patch! It looks good to me.

I think we can mark it as "Ready for Committer" in the CommitFest.
Unless there are any objections, I'll commit it once v19 development opens.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

#9Dilip Kumar
dilipbalaut@gmail.com
In reply to: Fujii Masao (#8)
Re: Add new wait event to XactLockTableWait

On Mon, Jun 9, 2025 at 2:18 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/06/09 14:19, Xuneng Zhou wrote:

Hi,

On Mon, Jun 9, 2025 at 12:52 PM Xuneng Zhou <xunengzhou@gmail.com <mailto:xunengzhou@gmail.com>> wrote:

Hi,

Please find attached Version 2, incorporating the suggested changes.

Apologies for the confusion — in the previous attempt, I mistakenly
named the patch file with a `0002-` prefix, thinking it reflected the
patch version rather than the patch series number. I've corrected the
filename to follow the proper convention.

Sorry for the missing hyphen in the patch name. Things should be in good shape now. I need to be more cautious and double-check everything before submitting patches and sending emails.😂

Thanks for updating the patch! It looks good to me.

I think we can mark it as "Ready for Committer" in the CommitFest.
Unless there are any objections, I'll commit it once v19 development opens.

LGTM, except I suggest using WAIT_EVENT_XACT_COMPLETE instead of
WAIT_EVENT_XACT_DONE. I think it sounds better.

--
Regards,
Dilip Kumar
Google

#10Xuneng Zhou
xunengzhou@gmail.com
In reply to: Dilip Kumar (#9)
1 attachment(s)
Re: Add new wait event to XactLockTableWait

Hi Dilip,

Thanks for looking into this!

On Mon, Jun 9, 2025 at 6:56 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

Thanks for updating the patch! It looks good to me.

I think we can mark it as "Ready for Committer" in the CommitFest.
Unless there are any objections, I'll commit it once v19 development

opens.

LGTM, except I suggest using WAIT_EVENT_XACT_COMPLETE instead of
WAIT_EVENT_XACT_DONE. I think it sounds better.

I have renamed it in v3.

Attachments:

v3-0001-Add-new-wait-event-to-XactLockTableWait-functions.patchapplication/octet-stream; name=v3-0001-Add-new-wait-event-to-XactLockTableWait-functions.patchDownload
From 3ef2f72689d30bf78c32c297f277c8370a3b0a94 Mon Sep 17 00:00:00 2001
From: alterego665 <824662526@qq.com>
Date: Mon, 9 Jun 2025 11:24:08 +0800
Subject: [PATCH v3] Add WAIT_EVENT_XACT_COMPLETE to XactLockTableWait functions

XactLockTableWait and ConditionalXactLockTableWait previously lacked a
specific wait event, making backend states less transparent. This commit
introduces WAIT_EVENT_XACT_COMPLETE, which both functions now report to
pg_stat_activity during their sleep phases, enhancing visibility.
---
 src/backend/storage/lmgr/lmgr.c                 | 4 ++++
 src/backend/utils/activity/wait_event_names.txt | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 3f6bf70bd3c..a35de2c497a 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -719,7 +719,9 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_COMPLETE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
@@ -762,7 +764,9 @@ ConditionalXactLockTableWait(TransactionId xid, bool logLockFailure)
 		if (!first)
 		{
 			CHECK_FOR_INTERRUPTS();
+			pgstat_report_wait_start(WAIT_EVENT_XACT_COMPLETE);
 			pg_usleep(1000L);
+			pgstat_report_wait_end();
 		}
 		first = false;
 		xid = SubTransGetTopmostTransaction(xid);
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index 4da68312b5f..9be14973535 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -160,6 +160,7 @@ WAL_BUFFER_INIT	"Waiting on WAL buffer to be initialized."
 WAL_RECEIVER_EXIT	"Waiting for the WAL receiver to exit."
 WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for streaming replication."
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
+XACT_COMPLETE	"Waiting for a transaction to complete."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at transaction end."
 
 ABI_compatibility:
@@ -419,7 +420,7 @@ extend	"Waiting to extend a relation."
 frozenid	"Waiting to update <structname>pg_database</structname>.<structfield>datfrozenxid</structfield> and <structname>pg_database</structname>.<structfield>datminmxid</structfield>."
 page	"Waiting to acquire a lock on a page of a relation."
 tuple	"Waiting to acquire a lock on a tuple."
-transactionid	"Waiting for a transaction to finish."
+transactionid	"Waiting to acquire a transaction ID lock; see <xref linkend="transaction-id"/>."
 virtualxid	"Waiting to acquire a virtual transaction ID lock; see <xref linkend="transaction-id"/>."
 spectoken	"Waiting to acquire a speculative insertion lock."
 object	"Waiting to acquire a lock on a non-relation database object."
-- 
2.48.1

#11Xuneng Zhou
xunengzhou@gmail.com
In reply to: Fujii Masao (#8)
Re: Add new wait event to XactLockTableWait

Hi Fujii-san,

On Mon, Jun 9, 2025 at 4:48 PM Fujii Masao <masao.fujii@oss.nttdata.com>
wrote:

Thanks for updating the patch! It looks good to me.

I think we can mark it as "Ready for Committer" in the CommitFest.
Unless there are any objections, I'll commit it once v19 development opens.

I've marked it as "Ready for Committer" in the CommitFest.

#12Dilip Kumar
dilipbalaut@gmail.com
In reply to: Xuneng Zhou (#10)
Re: Add new wait event to XactLockTableWait

On Mon, Jun 9, 2025 at 6:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

Hi Dilip,

Thanks for looking into this!

On Mon, Jun 9, 2025 at 6:56 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

Thanks for updating the patch! It looks good to me.

I think we can mark it as "Ready for Committer" in the CommitFest.
Unless there are any objections, I'll commit it once v19 development opens.

LGTM, except I suggest using WAIT_EVENT_XACT_COMPLETE instead of
WAIT_EVENT_XACT_DONE. I think it sounds better.

I have renamed it in v3.

Thanks LGTM.

--
Regards,
Dilip Kumar
Google