Allow processes to reset procArrayGroupNext themselves instead of leader resetting for all the followers
Hi,
While working on something else, I noticed that the proc array group
XID clearing leader resets procArrayGroupNext of all the followers
atomically along with procArrayGroupMember. ISTM that it's enough for
the followers to exit the wait loop and continue if the leader resets
just procArrayGroupMember, the followers can reset procArrayGroupNext
by themselves. This relieves the leader a bit, especially when there
are many followers, as it avoids a bunch of atomic writes and
pg_write_barrier() for the leader .
I'm attaching a small patch with the above change. It doesn't seem to
break anything, the cirrus-ci members are happy
https://github.com/BRupireddy/postgres/tree/allow_processes_to_reset_proc_array_v1.
It doesn't seem to show visible benefit on my system, nor hurt anyone
in my testing [1]# of clients HEAD PATCHED 1 31661 31512 2 67134 66789 4 135084 132372 8 254174 255384 16 418598 420903 32 491922 494183 64 509824 509451 128 513298 512838 256 505191 496266 512 465208 453588 768 431304 425736 1024 398110 397352 2048 308732 308901 4096 200355 219480.
Thoughts?
[1]: # of clients HEAD PATCHED 1 31661 31512 2 67134 66789 4 135084 132372 8 254174 255384 16 418598 420903 32 491922 494183 64 509824 509451 128 513298 512838 256 505191 496266 512 465208 453588 768 431304 425736 1024 398110 397352 2048 308732 308901 4096 200355 219480
# of clients HEAD PATCHED
1 31661 31512
2 67134 66789
4 135084 132372
8 254174 255384
16 418598 420903
32 491922 494183
64 509824 509451
128 513298 512838
256 505191 496266
512 465208 453588
768 431304 425736
1024 398110 397352
2048 308732 308901
4096 200355 219480
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Allow-processes-to-reset-procArrayGroupNext-thems.patchapplication/x-patch; name=v1-0001-Allow-processes-to-reset-procArrayGroupNext-thems.patchDownload
From 097e74e5d1477e6670cf1bcb9316a4a850c960ba Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 23 Nov 2022 14:08:19 +0000
Subject: [PATCH v1] Allow processes to reset procArrayGroupNext themselves
instead of leader
---
src/backend/storage/ipc/procarray.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 06918759e7..459ac8ead9 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -826,7 +826,9 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
}
pgstat_report_wait_end();
- Assert(pg_atomic_read_u32(&proc->procArrayGroupNext) == INVALID_PGPROCNO);
+ Assert(pg_atomic_read_u32(&proc->procArrayGroupNext) != INVALID_PGPROCNO);
+
+ pg_atomic_write_u32(&proc->procArrayGroupNext, INVALID_PGPROCNO);
/* Fix semaphore count for any absorbed wakeups */
while (extraWaits-- > 0)
@@ -874,10 +876,6 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
PGPROC *nextproc = &allProcs[wakeidx];
wakeidx = pg_atomic_read_u32(&nextproc->procArrayGroupNext);
- pg_atomic_write_u32(&nextproc->procArrayGroupNext, INVALID_PGPROCNO);
-
- /* ensure all previous writes are visible before follower continues. */
- pg_write_barrier();
nextproc->procArrayGroupMember = false;
--
2.34.1
Hi,
On 2022-11-24 10:43:46 +0530, Bharath Rupireddy wrote:
While working on something else, I noticed that the proc array group
XID clearing leader resets procArrayGroupNext of all the followers
atomically along with procArrayGroupMember. ISTM that it's enough for
the followers to exit the wait loop and continue if the leader resets
just procArrayGroupMember, the followers can reset procArrayGroupNext
by themselves. This relieves the leader a bit, especially when there
are many followers, as it avoids a bunch of atomic writes and
pg_write_barrier() for the leader .
I doubt this is a useful change - the leader already has to modify the
relevant cacheline (for procArrayGroupMember). That makes it pretty much free
to modify another field in the same cacheline.
Greetings,
Andres Freund
On Sun, Nov 27, 2022 at 2:48 AM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2022-11-24 10:43:46 +0530, Bharath Rupireddy wrote:
While working on something else, I noticed that the proc array group
XID clearing leader resets procArrayGroupNext of all the followers
atomically along with procArrayGroupMember. ISTM that it's enough for
the followers to exit the wait loop and continue if the leader resets
just procArrayGroupMember, the followers can reset procArrayGroupNext
by themselves. This relieves the leader a bit, especially when there
are many followers, as it avoids a bunch of atomic writes and
pg_write_barrier() for the leader .I doubt this is a useful change - the leader already has to modify the
relevant cacheline (for procArrayGroupMember). That makes it pretty much free
to modify another field in the same cacheline.
Agreed. Thanks for the response.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com