public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] cfe-03-scripts_over_cfe-02-internaldoc squash commit
16+ messages / 4 participants
[nested] [flat]
* [PATCH] cfe-03-scripts_over_cfe-02-internaldoc squash commit
@ 2021-04-06 18:23 Bruce Momjian <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Bruce Momjian @ 2021-04-06 18:23 UTC (permalink / raw)
---
src/backend/Makefile | 15 +++-
src/backend/crypto/ckey_aws.sh.sample (new) | 53 ++++++++++++
.../crypto/ckey_direct.sh.sample (new) | 39 +++++++++
.../crypto/ckey_passphrase.sh.sample (new) | 35 ++++++++
.../crypto/ckey_piv_nopin.sh.sample (new) | 68 ++++++++++++++++
.../crypto/ckey_piv_pin.sh.sample (new) | 81 +++++++++++++++++++
.../crypto/ssl_passphrase.sh.sample (new) | 35 ++++++++
7 files changed, 325 insertions(+), 1 deletion(-)
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 0da848b1fd..76eb44f632 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -211,6 +211,12 @@ endif
$(INSTALL_DATA) $(srcdir)/libpq/pg_hba.conf.sample '$(DESTDIR)$(datadir)/pg_hba.conf.sample'
$(INSTALL_DATA) $(srcdir)/libpq/pg_ident.conf.sample '$(DESTDIR)$(datadir)/pg_ident.conf.sample'
$(INSTALL_DATA) $(srcdir)/utils/misc/postgresql.conf.sample '$(DESTDIR)$(datadir)/postgresql.conf.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ckey_aws.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ckey_aws.sh.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ckey_direct.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ckey_direct.sh.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ckey_passphrase.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ckey_passphrase.sh.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ckey_piv_nopin.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ckey_piv_nopin.sh.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ckey_piv_pin.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ckey_piv_pin.sh.sample'
+ $(INSTALL_DATA) $(srcdir)/crypto/ssl_passphrase.sh.sample '$(DESTDIR)$(datadir)/auth_commands/ssl_passphrase.sh.sample'
ifeq ($(with_llvm), yes)
install-bin: install-postgres-bitcode
@@ -236,6 +242,7 @@ endif
installdirs:
$(MKDIR_P) '$(DESTDIR)$(bindir)' '$(DESTDIR)$(datadir)'
+ $(MKDIR_P) '$(DESTDIR)$(datadir)' '$(DESTDIR)$(datadir)/auth_commands'
ifeq ($(PORTNAME), cygwin)
ifeq ($(MAKE_DLL), true)
$(MKDIR_P) '$(DESTDIR)$(libdir)'
@@ -275,7 +282,13 @@ endif
$(MAKE) -C utils uninstall-data
rm -f '$(DESTDIR)$(datadir)/pg_hba.conf.sample' \
'$(DESTDIR)$(datadir)/pg_ident.conf.sample' \
- '$(DESTDIR)$(datadir)/postgresql.conf.sample'
+ '$(DESTDIR)$(datadir)/postgresql.conf.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ckey_aws.sh.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ckey_direct.sh.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ckey_passphrase.sh.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ckey_piv_nopin.sh.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ckey_piv_pin.sh.sample' \
+ '$(DESTDIR)$(datadir)/auth_commands/ssl_passphrase.sh.sample'
ifeq ($(with_llvm), yes)
$(call uninstall_llvm_module,postgres)
endif
diff --git a/src/backend/crypto/ckey_aws.sh.sample b/src/backend/crypto/ckey_aws.sh.sample
new file mode 100644
index 0000000000..d9bee53132
--- /dev/null
+++ b/src/backend/crypto/ckey_aws.sh.sample
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+# This uses the AWS Secrets Manager using the AWS CLI and OpenSSL.
+# This stores the AWS secret Id in $DIR.
+# Do not create any file with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -ne 1 ] && echo "cluster_key_command usage: $0 \"%d\"" 1>&2 && exit 1
+# No need for %R or -R since we are not prompting
+
+DIR="$1"
+[ ! -e "$DIR" ] && echo "$DIR does not exist" 1>&2 && exit 1
+[ ! -d "$DIR" ] && echo "$DIR is not a directory" 1>&2 && exit 1
+
+# File containing the id of the AWS secret
+AWS_ID_FILE="$DIR/aws-secret.id"
+
+
+# ----------------------------------------------------------------------
+
+
+# Create an AWS Secrets Manager secret?
+if [ ! -e "$AWS_ID_FILE" ]
+then # The 'postgres' operating system user must have permission to
+ # access the AWS CLI
+
+ # The epoch-time/directory/hostname combination is unique
+ HASH=$(echo -n "$(date '+%s')$DIR$(hostname)" | sha1sum | cut -d' ' -f1)
+ AWS_SECRET_ID="Postgres-cluster-key-$HASH"
+
+ # Use stdin to avoid passing the secret on the command line
+ openssl rand -hex 32 |
+ aws secretsmanager create-secret \
+ --name "$AWS_SECRET_ID" \
+ --description "Postgres cluster file encryption on $(hostname)" \
+ --secret-string 'file:///dev/stdin' \
+ --output text > /dev/null
+ if [ "$?" -ne 0 ]
+ then echo 'cluster key generation failed' 1>&2
+ exit 1
+ fi
+
+ echo "$AWS_SECRET_ID" > "$AWS_ID_FILE"
+fi
+
+if ! aws secretsmanager get-secret-value \
+ --secret-id "$(cat "$AWS_ID_FILE")" \
+ --output text
+then echo 'cluster key retrieval failed' 1>&2
+ exit 1
+fi | awk -F'\t' 'NR == 1 {print $4}'
+
+exit 0
diff --git a/src/backend/crypto/ckey_direct.sh.sample b/src/backend/crypto/ckey_direct.sh.sample
new file mode 100644
index 0000000000..492defcffe
--- /dev/null
+++ b/src/backend/crypto/ckey_direct.sh.sample
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+# This uses a 64-character hex key supplied by the user.
+# If OpenSSL is installed, you can generate a pseudo-random key by running:
+# openssl rand -hex 32
+# To get a true random key, run:
+# wget -q -O - 'https://www.random.org/cgi-bin/randbyte?nbytes=32&format=h'; | tr -d ' \n'; echo
+# Do not create any fie with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -lt 1 ] && echo "cluster_key_command usage: $0 %R [%p]" 1>&2 && exit 1
+# Supports environment variable PROMPT
+
+FD="$1"
+[ ! -t "$FD" ] && echo "file descriptor $FD does not refer to a terminal" 1>&2 && exit 1
+
+[ "$2" ] && PROMPT="$2"
+
+
+# ----------------------------------------------------------------------
+
+[ ! "$PROMPT" ] && PROMPT='Enter cluster key as 64 hexadecimal characters: '
+
+stty -echo <&"$FD"
+
+echo 1>&"$FD"
+echo -n "$PROMPT" 1>&"$FD"
+read KEY <&"$FD"
+
+stty echo <&"$FD"
+
+if [ "$(expr "$KEY" : '[0-9a-fA-F]*$')" -ne 64 ]
+then echo 'invalid; must be 64 hexadecimal characters' 1>&2
+ exit 1
+fi
+
+echo "$KEY"
+
+exit 0
diff --git a/src/backend/crypto/ckey_passphrase.sh.sample b/src/backend/crypto/ckey_passphrase.sh.sample
new file mode 100644
index 0000000000..a5d837b45e
--- /dev/null
+++ b/src/backend/crypto/ckey_passphrase.sh.sample
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+# This uses a passphrase supplied by the user.
+# Do not create any fie with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -lt 1 ] && echo "cluster_key_command usage: $0 %R [\"%p\"]" 1>&2 && exit 1
+
+FD="$1"
+[ ! -t "$FD" ] && echo "file descriptor $FD does not refer to a terminal" 1>&2 && exit 1
+# Supports environment variable PROMPT
+
+[ "$2" ] && PROMPT="$2"
+
+
+# ----------------------------------------------------------------------
+
+[ ! "$PROMPT" ] && PROMPT='Enter cluster passphrase: '
+
+stty -echo <&"$FD"
+
+echo 1>&"$FD"
+echo -n "$PROMPT" 1>&"$FD"
+read PASS <&"$FD"
+
+stty echo <&"$FD"
+
+if [ ! "$PASS" ]
+then echo 'invalid: empty passphrase' 1>&2
+ exit 1
+fi
+
+echo "$PASS" | sha256sum | cut -d' ' -f1
+
+exit 0
diff --git a/src/backend/crypto/ckey_piv_nopin.sh.sample b/src/backend/crypto/ckey_piv_nopin.sh.sample
new file mode 100644
index 0000000000..e90a579dea
--- /dev/null
+++ b/src/backend/crypto/ckey_piv_nopin.sh.sample
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+# This uses the public/private keys on a PIV device, like a CAC or Yubikey.
+# It uses a PIN stored in a file.
+# It uses OpenSSL with PKCS11 enabled via OpenSC.
+# This stores the cluster encryption key encrypted with the PIV public
+# key in $DIR. This is technically a three-level encryption
+# architecture, with the third level requiring the PIV and PIN.
+# Do not create any fie with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -ne 1 ] && echo "cluster_key_command usage: $0 \"%d\"" 1>&2 && exit 1
+# Supports environment variable PIV_PIN_FILE
+# No need for %R or -R since we are not prompting for a PIN
+
+DIR="$1"
+[ ! -e "$DIR" ] && echo "$DIR does not exist" 1>&2 && exit 1
+[ ! -d "$DIR" ] && echo "$DIR is not a directory" 1>&2 && exit 1
+
+# Set these here or pass in as environment variables.
+# File that stores the PIN to unlock the PIV
+#PIV_PIN_FILE=''
+# PIV slot 3 is the "Key Management" slot, so we use '0:3'
+PIV_SLOT='0:3'
+
+# File containing the cluster key encrypted with the PIV_SLOT's public key
+KEY_FILE="$DIR/pivpass.key"
+
+
+# ----------------------------------------------------------------------
+
+[ ! "$PIV_PIN_FILE" ] && echo 'PIV_PIN_FILE undefined' 1>&2 && exit 1
+[ ! -e "$PIV_PIN_FILE" ] && echo "$PIV_PIN_FILE does not exist" 1>&2 && exit 1
+[ -d "$PIV_PIN_FILE" ] && echo "$PIV_PIN_FILE is a directory" 1>&2 && exit 1
+
+[ ! "$KEY_FILE" ] && echo 'KEY_FILE undefined' 1>&2 && exit 1
+[ -d "$KEY_FILE" ] && echo "$KEY_FILE is a directory" 1>&2 && exit 1
+
+# Create a cluster key encrypted with the PIV_SLOT's public key?
+if [ ! -e "$KEY_FILE" ]
+then # The 'postgres' operating system user must have permission to
+ # access the PIV device.
+
+ openssl rand -hex 32 |
+ if ! openssl rsautl -engine pkcs11 -keyform engine -encrypt \
+ -inkey "$PIV_SLOT" -passin file:"$PIV_PIN_FILE" -out "$KEY_FILE"
+ then echo 'cluster key generation failed' 1>&2
+ exit 1
+ fi
+
+ # Warn the user to save the cluster key in a safe place
+ cat 1>&2 <<END
+
+WARNING: The PIV device can be locked and require a reset if too many PIN
+attempts fail. It is recommended to run this command manually and save
+the cluster key in a secure location for possible recovery.
+END
+
+fi
+
+# Decrypt the cluster key encrypted with the PIV_SLOT's public key
+if ! openssl rsautl -engine pkcs11 -keyform engine -decrypt \
+ -inkey "$PIV_SLOT" -passin file:"$PIV_PIN_FILE" -in "$KEY_FILE"
+then echo 'cluster key decryption failed' 1>&2
+ exit 1
+fi
+
+exit 0
diff --git a/src/backend/crypto/ckey_piv_pin.sh.sample b/src/backend/crypto/ckey_piv_pin.sh.sample
new file mode 100644
index 0000000000..e693ac31ba
--- /dev/null
+++ b/src/backend/crypto/ckey_piv_pin.sh.sample
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+# This uses the public/private keys on a PIV device, like a CAC or Yubikey.
+# It requires a user-entered PIN.
+# It uses OpenSSL with PKCS11 enabled via OpenSC.
+# This stores the cluster encryption key encrypted with the PIV public
+# key in $DIR. This is technically a three-level encryption
+# architecture, with the third level requiring the PIV and PIN.
+# Do not create any fie with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -lt 2 ] && echo "cluster_key_command usage: $0 \"%d\" %R [\"%p\"]" 1>&2 && exit 1
+# Supports environment variable PROMPT
+
+DIR="$1"
+[ ! -e "$DIR" ] && echo "$DIR does not exist" 1>&2 && exit 1
+[ ! -d "$DIR" ] && echo "$DIR is not a directory" 1>&2 && exit 1
+
+FD="$2"
+[ ! -t "$FD" ] && echo "file descriptor $FD does not refer to a terminal" 1>&2 && exit 1
+
+[ "$3" ] && PROMPT="$3"
+
+# PIV slot 3 is the "Key Management" slot, so we use '0:3'
+PIV_SLOT='0:3'
+
+# File containing the cluster key encrypted with the PIV_SLOT's public key
+KEY_FILE="$DIR/pivpass.key"
+
+
+# ----------------------------------------------------------------------
+
+[ ! "$PROMPT" ] && PROMPT='Enter PIV PIN: '
+
+stty -echo <&"$FD"
+
+# Create a cluster key encrypted with the PIV_SLOT's public key?
+if [ ! -e "$KEY_FILE" ]
+then echo 1>&"$FD"
+ echo -n "$PROMPT" 1>&"$FD"
+
+ # The 'postgres' operating system user must have permission to
+ # access the PIV device.
+
+ openssl rand -hex 32 |
+ # 'engine "pkcs11" set.' message confuses prompting
+ if ! openssl rsautl -engine pkcs11 -keyform engine -encrypt \
+ -inkey "$PIV_SLOT" -passin fd:"$FD" -out "$KEY_FILE" 2>&1
+ then stty echo <&"$FD"
+ echo 'cluster key generation failed' 1>&2
+ exit 1
+ fi | grep -v 'engine "pkcs11" set\.'
+
+ echo 1>&"$FD"
+
+ # Warn the user to save the cluster key in a safe place
+ cat 1>&"$FD" <<END
+
+WARNING: The PIV can be locked and require a reset if too many PIN
+attempts fail. It is recommended to run this command manually and save
+the cluster key in a secure location for possible recovery.
+END
+
+fi
+
+echo 1>&"$FD"
+echo -n "$PROMPT" 1>&"$FD"
+
+# Decrypt the cluster key encrypted with the PIV_SLOT's public key
+if ! openssl rsautl -engine pkcs11 -keyform engine -decrypt \
+ -inkey "$PIV_SLOT" -passin fd:"$FD" -in "$KEY_FILE" 2>&1
+then stty echo <&"$FD"
+ echo 'cluster key retrieval failed' 1>&2
+ exit 1
+fi | grep -v 'engine "pkcs11" set\.'
+
+echo 1>&"$FD"
+
+stty echo <&"$FD"
+
+exit 0
diff --git a/src/backend/crypto/ssl_passphrase.sh.sample b/src/backend/crypto/ssl_passphrase.sh.sample
new file mode 100644
index 0000000000..efbf5c0720
--- /dev/null
+++ b/src/backend/crypto/ssl_passphrase.sh.sample
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+# This uses a passphrase supplied by the user.
+# Do not create any fie with extension "wkey" in $DIR; these are
+# reserved for wrapped data key files.
+
+[ "$#" -lt 1 ] && echo "ssl_passphrase_command usage: $0 %R [\"%p\"]" 1>&2 && exit 1
+
+FD="$1"
+[ ! -t "$FD" ] && echo "file descriptor $FD does not refer to a terminal" 1>&2 && exit 1
+# Supports environment variable PROMPT
+
+[ "$2" ] && PROMPT="$2"
+
+
+# ----------------------------------------------------------------------
+
+[ ! "$PROMPT" ] && PROMPT='Enter cluster passphrase: '
+
+stty -echo <&"$FD"
+
+echo 1>&"$FD"
+echo -n "$PROMPT" 1>&"$FD"
+read PASS <&"$FD"
+
+stty echo <&"$FD"
+
+if [ ! "$PASS" ]
+then echo 'invalid: empty passphrase' 1>&2
+ exit 1
+fi
+
+echo "$PASS"
+
+exit 0
--
2.20.1
--gatW/ieO32f1wygP
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="cfe-04-common_over_cfe-03-scripts.diff"
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
@ 2023-02-02 13:30 Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Bharath Rupireddy @ 2023-02-02 13:30 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Nathan Bossart <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Jan 24, 2023 at 7:00 PM Bharath Rupireddy
<[email protected]> wrote:
>
> I'm attaching the v3 patch with the above review comments addressed.
> Hopefully, no memory ordering issues now. FWIW, I've added it to CF
> https://commitfest.postgresql.org/42/4141/.
>
> Test results with the v3 patch and insert workload are the same as
> that of the earlier run - TPS starts to scale at higher clients as
> expected after 512 clients and peaks at 2X with 2048 and 4096 clients.
>
> HEAD:
> 1 1380.411086
> 2 1358.378988
> 4 2701.974332
> 8 5925.380744
> 16 10956.501237
> 32 20877.513953
> 64 40838.046774
> 128 70251.744161
> 256 108114.321299
> 512 120478.988268
> 768 99140.425209
> 1024 93645.984364
> 2048 70111.159909
> 4096 55541.804826
>
> v3 PATCHED:
> 1 1493.800209
> 2 1569.414953
> 4 3154.186605
> 8 5965.578904
> 16 11912.587645
> 32 22720.964908
> 64 42001.094528
> 128 78361.158983
> 256 110457.926232
> 512 148941.378393
> 768 167256.590308
> 1024 155510.675372
> 2048 147499.376882
> 4096 119375.457779
I slightly modified the comments and attached the v4 patch for further
review. I also took perf report - there's a clear reduction in the
functions that are affected by the patch - LWLockWaitListLock,
WaitXLogInsertionsToFinish, LWLockWaitForVar and
LWLockConflictsWithVar. Note that I compiled the source code with
-ggdb for capturing symbols for perf, still the benefit stands at > 2X
for a higher number of clients.
HEAD:
+ 16.87% 0.01% postgres [.] CommitTransactionCommand
+ 16.86% 0.00% postgres [.] finish_xact_command
+ 16.81% 0.01% postgres [.] CommitTransaction
+ 15.09% 0.20% postgres [.] LWLockWaitListLock
+ 14.53% 0.01% postgres [.] WaitXLogInsertionsToFinish
+ 14.51% 0.02% postgres [.] LWLockWaitForVar
+ 11.70% 11.63% postgres [.] pg_atomic_read_u32_impl
+ 11.66% 0.08% postgres [.] pg_atomic_read_u32
+ 9.96% 0.03% postgres [.] LWLockConflictsWithVar
+ 4.78% 0.00% postgres [.] LWLockQueueSelf
+ 1.91% 0.01% postgres [.] pg_atomic_fetch_or_u32
+ 1.91% 1.89% postgres [.] pg_atomic_fetch_or_u32_impl
+ 1.73% 0.00% postgres [.] XLogInsert
+ 1.69% 0.01% postgres [.] XLogInsertRecord
+ 1.41% 0.01% postgres [.] LWLockRelease
+ 1.37% 0.47% postgres [.] perform_spin_delay
+ 1.11% 1.11% postgres [.] spin_delay
+ 1.10% 0.03% postgres [.] exec_bind_message
+ 0.91% 0.00% postgres [.] WALInsertLockRelease
+ 0.91% 0.00% postgres [.] LWLockReleaseClearVar
+ 0.72% 0.02% postgres [.] LWLockAcquire
+ 0.60% 0.00% postgres [.] LWLockDequeueSelf
+ 0.58% 0.00% postgres [.] GetTransactionSnapshot
0.58% 0.49% postgres [.] GetSnapshotData
+ 0.58% 0.00% postgres [.] WALInsertLockAcquire
+ 0.55% 0.00% postgres [.] XactLogCommitRecord
TPS (compiled with -ggdb for capturing symbols for perf)
1 1392.512967
2 1435.899119
4 3104.091923
8 6159.305522
16 11477.641780
32 22701.000718
64 41662.425880
128 23743.426209
256 89837.651619
512 65164.221500
768 66015.733370
1024 56421.223080
2048 52909.018072
4096 40071.146985
PATCHED:
+ 2.19% 0.05% postgres [.] LWLockWaitListLock
+ 2.10% 0.01% postgres [.] LWLockQueueSelf
+ 1.73% 1.71% postgres [.] pg_atomic_read_u32_impl
+ 1.73% 0.02% postgres [.] pg_atomic_read_u32
+ 1.72% 0.02% postgres [.] LWLockRelease
+ 1.65% 0.04% postgres [.] exec_bind_message
+ 1.43% 0.00% postgres [.] XLogInsert
+ 1.42% 0.01% postgres [.] WaitXLogInsertionsToFinish
+ 1.40% 0.03% postgres [.] LWLockWaitForVar
+ 1.38% 0.02% postgres [.] XLogInsertRecord
+ 0.93% 0.03% postgres [.] LWLockAcquireOrWait
+ 0.91% 0.00% postgres [.] GetTransactionSnapshot
+ 0.91% 0.79% postgres [.] GetSnapshotData
+ 0.91% 0.00% postgres [.] WALInsertLockRelease
+ 0.91% 0.00% postgres [.] LWLockReleaseClearVar
+ 0.53% 0.02% postgres [.] ExecInitModifyTable
TPS (compiled with -ggdb for capturing symbols for perf)
1 1295.296611
2 1459.079162
4 2865.688987
8 5533.724983
16 10771.697842
32 20557.499312
64 39436.423783
128 42555.639048
256 73139.060227
512 124649.665196
768 131162.826976
1024 132185.160007
2048 117377.586644
4096 88240.336940
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/x-patch] v4-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch (9.5K, ../../CALj2ACUt-YdzxdGZPcr0LcERf0Hv2sTbz4ZvUftQ8Dgk4aVRfA@mail.gmail.com/2-v4-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch)
download | inline diff:
From 74c5bd8cc4f1497aa7f2fa02c6487039dc91e847 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 2 Feb 2023 03:42:27 +0000
Subject: [PATCH v4] Optimize WAL insertion lock acquisition and release
This commit optimizes WAL insertion lock acquisition and release
in the following way:
1. WAL insertion lock's variable insertingAt is currently read and
written with the help of lwlock's wait list lock to avoid
torn-free reads/writes. This wait list lock can become a point of
contention on a highly concurrent write workloads. Therefore, make
insertingAt a 64-bit atomic which inherently provides torn-free
reads/writes.
2. LWLockUpdateVar currently acquires lwlock's wait list lock even
when there are no waiters at all. Add a fastpath exit to
LWLockUpdateVar when there are no waiters to avoid unnecessary
locking.
Note that atomic exchange operation (which is a full barrier) is
used when necessary, instead of atomic write to ensure the memory
ordering is preserved.
It also adds a note in WaitXLogInsertionsToFinish regarding how the
use of spinlock there can avoid explicit memory barrier in some
subsequently called functions.
Suggested-by: Andres Freund
Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart
Discussion: https://www.postgresql.org/message-id/20221124184619.xit4sfi52bcz2tva%40awork3.anarazel.de
---
src/backend/access/transam/xlog.c | 14 +++++--
src/backend/storage/lmgr/lwlock.c | 66 +++++++++++++++++++------------
src/include/storage/lwlock.h | 6 +--
3 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fb4c860bde..95aed0e97f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -350,7 +350,8 @@ typedef struct XLogwrtResult
* wait for all currently in-progress insertions to finish, but the
* insertingAt indicator allows you to ignore insertions to later in the WAL,
* so that you only wait for the insertions that are modifying the buffers
- * you're about to write out.
+ * you're about to write out. Using an atomic variable for insertingAt avoids
+ * taking any explicit lock for reads and writes.
*
* This isn't just an optimization. If all the WAL buffers are dirty, an
* inserter that's holding a WAL insert lock might need to evict an old WAL
@@ -376,7 +377,7 @@ typedef struct XLogwrtResult
typedef struct
{
LWLock lock;
- XLogRecPtr insertingAt;
+ pg_atomic_uint64 insertingAt;
XLogRecPtr lastImportantAt;
} WALInsertLock;
@@ -1496,6 +1497,13 @@ WaitXLogInsertionsToFinish(XLogRecPtr upto)
* calling LWLockUpdateVar. But if it has to sleep, it will
* advertise the insertion point with LWLockUpdateVar before
* sleeping.
+ *
+ * XXX: Use of a spinlock at the beginning of this function to read
+ * current insert position implies memory ordering. That means that
+ * the immediate loads and stores to shared memory (for instance,
+ * in LWLockUpdateVar called via LWLockWaitForVar) don't need an
+ * explicit memory barrier as far as the current usage is
+ * concerned. But that might not be safe in general.
*/
if (LWLockWaitForVar(&WALInsertLocks[i].l.lock,
&WALInsertLocks[i].l.insertingAt,
@@ -4596,7 +4604,7 @@ XLOGShmemInit(void)
for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
{
LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
- WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr;
+ pg_atomic_init_u64(&WALInsertLocks[i].l.insertingAt, InvalidXLogRecPtr);
WALInsertLocks[i].l.lastImportantAt = InvalidXLogRecPtr;
}
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index d2ec396045..27c3b63c68 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1547,9 +1547,8 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
* *result is set to true if the lock was free, and false otherwise.
*/
static bool
-LWLockConflictsWithVar(LWLock *lock,
- uint64 *valptr, uint64 oldval, uint64 *newval,
- bool *result)
+LWLockConflictsWithVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval, bool *result)
{
bool mustwait;
uint64 value;
@@ -1572,13 +1571,11 @@ LWLockConflictsWithVar(LWLock *lock,
*result = false;
/*
- * Read value using the lwlock's wait list lock, as we can't generally
- * rely on atomic 64 bit reads/stores. TODO: On platforms with a way to
- * do atomic 64 bit reads/writes the spinlock should be optimized away.
+ * Read value atomically without any explicit lock. We rely on 64-bit
+ * atomic reads/writes that transparently does the required work to make
+ * even non-atomic reads/writes tear free.
*/
- LWLockWaitListLock(lock);
- value = *valptr;
- LWLockWaitListUnlock(lock);
+ value = pg_atomic_read_u64(valptr);
if (value != oldval)
{
@@ -1607,7 +1604,8 @@ LWLockConflictsWithVar(LWLock *lock,
* in shared mode, returns 'true'.
*/
bool
-LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
+LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval)
{
PGPROC *proc = MyProc;
int extraWaits = 0;
@@ -1735,29 +1733,47 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
* LWLockUpdateVar - Update a variable and wake up waiters atomically
*
* Sets *valptr to 'val', and wakes up all processes waiting for us with
- * LWLockWaitForVar(). Setting the value and waking up the processes happen
- * atomically so that any process calling LWLockWaitForVar() on the same lock
- * is guaranteed to see the new value, and act accordingly.
+ * LWLockWaitForVar(). It first sets the value atomically and then wakes up
+ * the waiting processes so that any process calling LWLockWaitForVar() on the
+ * same lock is guaranteed to see the new value, and act accordingly.
*
* The caller must be holding the lock in exclusive mode.
*/
void
-LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
proclist_head wakeup;
proclist_mutable_iter iter;
PRINT_LWDEBUG("LWLockUpdateVar", lock, LW_EXCLUSIVE);
+ /*
+ * Update the lock variable atomically first without having to acquire wait
+ * list lock, so that if anyone looking for the lock will have chance to
+ * grab it a bit quickly.
+ *
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent atomic read of lock
+ * state to check if it has any waiters happens after we set the lock
+ * variable to new value here. Without a barrier, we could end up missing
+ * waiters that otherwise should have been woken up.
+ */
+ pg_atomic_exchange_u64(valptr, val);
+
+ /*
+ * Quick exit when there are no waiters. This avoids unnecessary lwlock's
+ * wait list lock acquisition and release.
+ */
+ if ((pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) == 0)
+ return;
+
proclist_init(&wakeup);
LWLockWaitListLock(lock);
Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
- /* Update the lock's value */
- *valptr = val;
-
/*
* See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken
* up. They are always in the front of the queue.
@@ -1873,17 +1889,17 @@ LWLockRelease(LWLock *lock)
* LWLockReleaseClearVar - release a previously acquired lock, reset variable
*/
void
-LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
- LWLockWaitListLock(lock);
-
/*
- * Set the variable's value before releasing the lock, that prevents race
- * a race condition wherein a new locker acquires the lock, but hasn't yet
- * set the variables value.
+ * Update the lock variable atomically first.
+ *
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent shared memory
+ * reads/writes, if any, happen after we reset the lock variable.
*/
- *valptr = val;
- LWLockWaitListUnlock(lock);
+ pg_atomic_exchange_u64(valptr, val);
LWLockRelease(lock);
}
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index d2c7afb8f4..f19bc49193 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -128,14 +128,14 @@ extern bool LWLockAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockConditionalAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockAcquireOrWait(LWLock *lock, LWLockMode mode);
extern void LWLockRelease(LWLock *lock);
-extern void LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern void LWLockReleaseAll(void);
extern bool LWLockHeldByMe(LWLock *lock);
extern bool LWLockAnyHeldByMe(LWLock *lock, int nlocks, size_t stride);
extern bool LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode);
-extern bool LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval);
-extern void LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern bool LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, uint64 *newval);
+extern void LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern Size LWLockShmemSize(void);
extern void CreateLWLocks(void);
--
2.34.1
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-02-08 22:06 ` Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Nathan Bossart @ 2023-02-08 22:06 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
+ pg_atomic_exchange_u64(valptr, val);
nitpick: I'd add a (void) at the beginning of these calls to
pg_atomic_exchange_u64() so that it's clear that we are discarding the
return value.
+ /*
+ * Update the lock variable atomically first without having to acquire wait
+ * list lock, so that if anyone looking for the lock will have chance to
+ * grab it a bit quickly.
+ *
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent atomic read of lock
+ * state to check if it has any waiters happens after we set the lock
+ * variable to new value here. Without a barrier, we could end up missing
+ * waiters that otherwise should have been woken up.
+ */
+ pg_atomic_exchange_u64(valptr, val);
+
+ /*
+ * Quick exit when there are no waiters. This avoids unnecessary lwlock's
+ * wait list lock acquisition and release.
+ */
+ if ((pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) == 0)
+ return;
I think this makes sense. A waiter could queue itself after the exchange,
but it'll recheck after queueing. IIUC this is basically how this works
today. We update the value and release the lock before waking up any
waiters, so the same principle applies.
Overall, I think this patch is in reasonable shape.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
@ 2023-02-09 06:21 ` Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Bharath Rupireddy @ 2023-02-09 06:21 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Feb 9, 2023 at 3:36 AM Nathan Bossart <[email protected]> wrote:
>
> + pg_atomic_exchange_u64(valptr, val);
>
> nitpick: I'd add a (void) at the beginning of these calls to
> pg_atomic_exchange_u64() so that it's clear that we are discarding the
> return value.
I did that in the attached v5 patch although it's a mix elsewhere;
some doing explicit return value cast with (void) and some not.
> + /*
> + * Update the lock variable atomically first without having to acquire wait
> + * list lock, so that if anyone looking for the lock will have chance to
> + * grab it a bit quickly.
> + *
> + * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
> + * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
> + * a full barrier, we're guaranteed that the subsequent atomic read of lock
> + * state to check if it has any waiters happens after we set the lock
> + * variable to new value here. Without a barrier, we could end up missing
> + * waiters that otherwise should have been woken up.
> + */
> + pg_atomic_exchange_u64(valptr, val);
> +
> + /*
> + * Quick exit when there are no waiters. This avoids unnecessary lwlock's
> + * wait list lock acquisition and release.
> + */
> + if ((pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) == 0)
> + return;
>
> I think this makes sense. A waiter could queue itself after the exchange,
> but it'll recheck after queueing. IIUC this is basically how this works
> today. We update the value and release the lock before waking up any
> waiters, so the same principle applies.
Yes, a waiter right after self-queuing (LWLockQueueSelf) checks for
the value (LWLockConflictsWithVar) before it goes and waits until
awakened in LWLockWaitForVar. A waiter added to the queue is
guaranteed to be woken up by the
LWLockUpdateVar but before that the lock value is set and we have
pg_atomic_exchange_u64 as a memory barrier, so no memory reordering.
Essentially, the order of these operations aren't changed. The benefit
that we're seeing is from avoiding LWLock's waitlist lock for reading
and updating the lock value relying on 64-bit atomics.
> Overall, I think this patch is in reasonable shape.
Thanks for reviewing. Please see the attached v5 patch.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v5-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch (9.5K, ../../CALj2ACULfpGBpXY3UPLegFbk0zFUCYvHVom09gwAbQ+NsJTHaw@mail.gmail.com/2-v5-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch)
download | inline diff:
From d13f00494e7a74caa41dd2d79b8b05add6dc5c28 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 9 Feb 2023 02:42:58 +0000
Subject: [PATCH v5] Optimize WAL insertion lock acquisition and release
This commit optimizes WAL insertion lock acquisition and release
in the following way:
1. WAL insertion lock's variable insertingAt is currently read and
written with the help of lwlock's wait list lock to avoid
torn-free reads/writes. This wait list lock can become a point of
contention on a highly concurrent write workloads. Therefore, make
insertingAt a 64-bit atomic which inherently provides torn-free
reads/writes.
2. LWLockUpdateVar currently acquires lwlock's wait list lock even
when there are no waiters at all. Add a fastpath exit to
LWLockUpdateVar when there are no waiters to avoid unnecessary
locking.
Note that atomic exchange operation (which is a full barrier) is
used when necessary, instead of atomic write to ensure the memory
ordering is preserved.
It also adds a note in WaitXLogInsertionsToFinish regarding how the
use of spinlock there can avoid explicit memory barrier in some
subsequently called functions.
Suggested-by: Andres Freund
Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart
Discussion: https://www.postgresql.org/message-id/20221124184619.xit4sfi52bcz2tva%40awork3.anarazel.de
---
src/backend/access/transam/xlog.c | 14 +++++--
src/backend/storage/lmgr/lwlock.c | 66 +++++++++++++++++++------------
src/include/storage/lwlock.h | 6 +--
3 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f9f0f6db8d..affb8c77c5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -350,7 +350,8 @@ typedef struct XLogwrtResult
* wait for all currently in-progress insertions to finish, but the
* insertingAt indicator allows you to ignore insertions to later in the WAL,
* so that you only wait for the insertions that are modifying the buffers
- * you're about to write out.
+ * you're about to write out. Using an atomic variable for insertingAt avoids
+ * taking any explicit lock for reads and writes.
*
* This isn't just an optimization. If all the WAL buffers are dirty, an
* inserter that's holding a WAL insert lock might need to evict an old WAL
@@ -376,7 +377,7 @@ typedef struct XLogwrtResult
typedef struct
{
LWLock lock;
- XLogRecPtr insertingAt;
+ pg_atomic_uint64 insertingAt;
XLogRecPtr lastImportantAt;
} WALInsertLock;
@@ -1495,6 +1496,13 @@ WaitXLogInsertionsToFinish(XLogRecPtr upto)
* calling LWLockUpdateVar. But if it has to sleep, it will
* advertise the insertion point with LWLockUpdateVar before
* sleeping.
+ *
+ * XXX: Use of a spinlock at the beginning of this function to read
+ * current insert position implies memory ordering. That means that
+ * the immediate loads and stores to shared memory (for instance,
+ * in LWLockUpdateVar called via LWLockWaitForVar) don't need an
+ * explicit memory barrier as far as the current usage is
+ * concerned. But that might not be safe in general.
*/
if (LWLockWaitForVar(&WALInsertLocks[i].l.lock,
&WALInsertLocks[i].l.insertingAt,
@@ -4595,7 +4603,7 @@ XLOGShmemInit(void)
for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
{
LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
- WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr;
+ pg_atomic_init_u64(&WALInsertLocks[i].l.insertingAt, InvalidXLogRecPtr);
WALInsertLocks[i].l.lastImportantAt = InvalidXLogRecPtr;
}
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index d2ec396045..3a7e3e11ba 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1547,9 +1547,8 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
* *result is set to true if the lock was free, and false otherwise.
*/
static bool
-LWLockConflictsWithVar(LWLock *lock,
- uint64 *valptr, uint64 oldval, uint64 *newval,
- bool *result)
+LWLockConflictsWithVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval, bool *result)
{
bool mustwait;
uint64 value;
@@ -1572,13 +1571,11 @@ LWLockConflictsWithVar(LWLock *lock,
*result = false;
/*
- * Read value using the lwlock's wait list lock, as we can't generally
- * rely on atomic 64 bit reads/stores. TODO: On platforms with a way to
- * do atomic 64 bit reads/writes the spinlock should be optimized away.
+ * Read value atomically without any explicit lock. We rely on 64-bit
+ * atomic reads/writes that transparently does the required work to make
+ * even non-atomic reads/writes tear free.
*/
- LWLockWaitListLock(lock);
- value = *valptr;
- LWLockWaitListUnlock(lock);
+ value = pg_atomic_read_u64(valptr);
if (value != oldval)
{
@@ -1607,7 +1604,8 @@ LWLockConflictsWithVar(LWLock *lock,
* in shared mode, returns 'true'.
*/
bool
-LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
+LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval)
{
PGPROC *proc = MyProc;
int extraWaits = 0;
@@ -1735,29 +1733,47 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
* LWLockUpdateVar - Update a variable and wake up waiters atomically
*
* Sets *valptr to 'val', and wakes up all processes waiting for us with
- * LWLockWaitForVar(). Setting the value and waking up the processes happen
- * atomically so that any process calling LWLockWaitForVar() on the same lock
- * is guaranteed to see the new value, and act accordingly.
+ * LWLockWaitForVar(). It first sets the value atomically and then wakes up
+ * the waiting processes so that any process calling LWLockWaitForVar() on the
+ * same lock is guaranteed to see the new value, and act accordingly.
*
* The caller must be holding the lock in exclusive mode.
*/
void
-LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
proclist_head wakeup;
proclist_mutable_iter iter;
PRINT_LWDEBUG("LWLockUpdateVar", lock, LW_EXCLUSIVE);
+ /*
+ * Update the lock variable atomically first without having to acquire wait
+ * list lock, so that if anyone looking for the lock will have chance to
+ * grab it a bit quickly.
+ *
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent atomic read of lock
+ * state to check if it has any waiters happens after we set the lock
+ * variable to new value here. Without a barrier, we could end up missing
+ * waiters that otherwise should have been woken up.
+ */
+ (void) pg_atomic_exchange_u64(valptr, val);
+
+ /*
+ * Quick exit when there are no waiters. This avoids unnecessary lwlock's
+ * wait list lock acquisition and release.
+ */
+ if ((pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) == 0)
+ return;
+
proclist_init(&wakeup);
LWLockWaitListLock(lock);
Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
- /* Update the lock's value */
- *valptr = val;
-
/*
* See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken
* up. They are always in the front of the queue.
@@ -1873,17 +1889,17 @@ LWLockRelease(LWLock *lock)
* LWLockReleaseClearVar - release a previously acquired lock, reset variable
*/
void
-LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
- LWLockWaitListLock(lock);
-
/*
- * Set the variable's value before releasing the lock, that prevents race
- * a race condition wherein a new locker acquires the lock, but hasn't yet
- * set the variables value.
+ * Update the lock variable atomically first.
+ *
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent shared memory
+ * reads/writes, if any, happen after we reset the lock variable.
*/
- *valptr = val;
- LWLockWaitListUnlock(lock);
+ (void) pg_atomic_exchange_u64(valptr, val);
LWLockRelease(lock);
}
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index d2c7afb8f4..f19bc49193 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -128,14 +128,14 @@ extern bool LWLockAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockConditionalAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockAcquireOrWait(LWLock *lock, LWLockMode mode);
extern void LWLockRelease(LWLock *lock);
-extern void LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern void LWLockReleaseAll(void);
extern bool LWLockHeldByMe(LWLock *lock);
extern bool LWLockAnyHeldByMe(LWLock *lock, int nlocks, size_t stride);
extern bool LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode);
-extern bool LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval);
-extern void LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern bool LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, uint64 *newval);
+extern void LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern Size LWLockShmemSize(void);
extern void CreateLWLocks(void);
--
2.34.1
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-02-21 05:49 ` Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Nathan Bossart @ 2023-02-21 05:49 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Feb 09, 2023 at 11:51:28AM +0530, Bharath Rupireddy wrote:
> On Thu, Feb 9, 2023 at 3:36 AM Nathan Bossart <[email protected]> wrote:
>> Overall, I think this patch is in reasonable shape.
>
> Thanks for reviewing. Please see the attached v5 patch.
I'm marking this as ready-for-committer. I think a couple of the comments
could use some small adjustments, but that probably doesn't need to hold up
this patch.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
@ 2023-04-10 04:08 ` Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Michael Paquier @ 2023-04-10 04:08 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Feb 20, 2023 at 09:49:48PM -0800, Nathan Bossart wrote:
> I'm marking this as ready-for-committer. I think a couple of the comments
> could use some small adjustments, but that probably doesn't need to hold up
> this patch.
Apologies. I was planning to have a thorough look at this patch but
life got in the way and I have not been able to study what's happening
on this thread this close to the feature freeze.
Anyway, I am attaching two modules I have written for the sake of this
thread while beginning my lookup of the patch:
- lwlock_test.tar.gz, validation module for LWLocks with variable
waits. This module can be loaded with shared_preload_libraries to
have two LWLocks and two variables in shmem, then have 2 backends play
ping-pong with each other's locks. An isolation test may be possible,
though I have not thought hard about it. Just use a SQL sequence like
that, for example, with N > 1 (see README):
Backend 1: SELECT lwlock_test_acquire();
Backend 2: SELECT lwlock_test_wait(N);
Backend 1: SELECT lwlock_test_update(N);
Backend 1: SELECT lwlock_test_release();
- custom_wal.tar.gz, thin wrapper for LogLogicalMessage() able to
generate N records of size M bytes in a single SQL call. This can be
used to generate records of various sizes for benchmarking, limiting
the overhead of individual calls to pg_logical_emit_message_bytea().
I have begun gathering numbers with WAL records of various size and
length, using pgbench like:
$ cat script.sql
\set record_size 1
\set record_number 5000
SELECT custom_wal(:record_size, :record_number);
$ pgbench -n -c 500 -t 100 -f script.sql
So this limits most the overhead of behind parsing, planning, and most
of the INSERT logic.
I have been trying to get some reproducible numbers, but I think that
I am going to need a bigger maching than what I have been using for
the last few days, up to 400 connections. It is worth noting that
00d1e02b may influence a bit the results, so we may want to have more
numbers with that in place particularly with INSERTs, and one of the
tests used upthread uses single row INSERTs.
Another question I had: would it be worth having some tests with
pg_wal/ mounted to a tmpfs so as I/O would not be a bottleneck? It
should be instructive to get more measurement with a fixed number of
transactions and a rather high amount of concurrent connections (1k at
least?), where the contention would be on the variable waits. My
first impression is that records should not be too small if you want
to see more the effects of this patch, either.
Looking at the patch.. LWLockConflictsWithVar() and
LWLockReleaseClearVar() are the trivial bits. These are OK.
+ * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
+ * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
+ * a full barrier, we're guaranteed that the subsequent shared memory
+ * reads/writes, if any, happen after we reset the lock variable.
This mentions that the subsequent read/write operations are safe, so
this refers to anything happening after the variable is reset. As
a full barrier, should be also mention that this is also ordered with
respect to anything that the caller did before clearing the variable?
From this perspective using pg_atomic_exchange_u64() makes sense to me
in LWLockReleaseClearVar().
+ * XXX: Use of a spinlock at the beginning of this function to read
+ * current insert position implies memory ordering. That means that
+ * the immediate loads and stores to shared memory (for instance,
+ * in LWLockUpdateVar called via LWLockWaitForVar) don't need an
+ * explicit memory barrier as far as the current usage is
+ * concerned. But that might not be safe in general.
*/
What's the part where this is not safe? Based on what I see, this
code path is safe because of the previous spinlock. This is the same
comment as at the beginning of LWLockConflictsWithVar(). Is that
something that we ought to document at the top of LWLockWaitForVar()
as well? We have one caller of this function currently, but there may
be more in the future.
- * you're about to write out.
+ * you're about to write out. Using an atomic variable for insertingAt avoids
+ * taking any explicit lock for reads and writes.
Hmm. Not sure that we need to comment at all.
-LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
[...]
Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
- /* Update the lock's value */
- *valptr = val;
The sensitive change is in LWLockUpdateVar(). I am not completely
sure to understand this removal, though. Does that influence the case
where there are waiters?
Another thing I was wondering about: how much does the fast-path used
in LWLockUpdateVar() influence the performance numbers? Am I right to
guess that it counts for most of the gain seen? Or could it be that
the removal of the spin lock in
LWLockConflictsWithVar()/LWLockWaitForVar() the point that has the
highest effect?
--
Michael
Attachments:
[application/gzip] lwlock_test.tar.gz (3.1K, ../../[email protected]/2-lwlock_test.tar.gz)
download
[application/gzip] custom_wal.tar.gz (1.1K, ../../[email protected]/3-custom_wal.tar.gz)
download
[application/pgp-signature] signature.asc (833B, ../../[email protected]/4-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
@ 2023-05-08 12:27 ` Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-05-10 12:04 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 2 replies; 16+ messages in thread
From: Bharath Rupireddy @ 2023-05-08 12:27 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Apr 10, 2023 at 9:38 AM Michael Paquier <[email protected]> wrote:
>
> I have been trying to get some reproducible numbers, but I think that
> I am going to need a bigger maching than what I have been using for
> the last few days, up to 400 connections. It is worth noting that
> 00d1e02b may influence a bit the results, so we may want to have more
> numbers with that in place particularly with INSERTs, and one of the
> tests used upthread uses single row INSERTs.
I ran performance tests on the patch with different use-cases. Clearly
the patch reduces burden on LWLock's waitlist lock (evident from perf
reports [1]). However, to see visible impact in the output, the txns
must be generating small (between 16 bytes to 2 KB) amounts of WAL in
a highly concurrent manner, check the results below (FWIW, I've zipped
and attached perf images for better illustration along with test
setup).
When the txns are generating a small amount of WAL i.e. between 16
bytes to 2 KB in a highly concurrent manner, the benefit is clearly
visible in the TPS more than 2.3X improvement. When the txns are
generating more WAL i.e. more than 2 KB, the gain from reduced burden
on waitlist lock is offset by increase in the wait/release for WAL
insertion locks and no visible benefit is seen.
As the amount of WAL each txn generates increases, it looks like the
benefit gained from reduced burden on waitlist lock is offset by
increase in the wait for WAL insertion locks.
Note that I've used pg_logical_emit_message() for ease of
understanding about the txns generating various amounts of WAL, but
the pattern is the same if txns are generating various amounts of WAL
say with inserts.
test-case 1: -T5, WAL ~16 bytes
clients HEAD PATCHED
1 1437 1352
2 1376 1419
4 2919 2774
8 5875 6371
16 11148 12242
32 22108 23532
64 41414 46478
128 85304 85235
256 83771 152901
512 61970 141021
768 56514 118899
1024 51784 110960
2048 39141 84150
4096 16901 45759
test-case 1: -t1000, WAL ~16 bytes
clients HEAD PATCHED
1 1417 1333
2 1363 1791
4 2978 2970
8 5954 6198
16 11179 11164
32 23742 24043
64 45537 44103
128 84683 91762
256 80369 146293
512 61080 132079
768 57236 118046
1024 53497 114574
2048 46423 93588
4096 42067 85790
test-case 2: -T5, WAL ~256 bytes
clients HEAD PATCHED
1 1521 1386
2 1647 1637
4 3088 3270
8 6011 5631
16 12778 10317
32 24117 20006
64 43966 38199
128 72660 67936
256 93096 121261
512 57247 142418
768 53782 126218
1024 50279 109153
2048 35109 91602
4096 21184 39848
test-case 2: -t1000, WAL ~256 bytes
clients HEAD PATCHED
1 1265 1389
2 1522 1258
4 2802 2775
8 5875 5422
16 11664 10853
32 21961 22145
64 44304 40851
128 73278 80494
256 91172 122287
512 60966 136734
768 56590 125050
1024 52481 124341
2048 47878 104760
4096 42838 94121
test-case 3: -T5, WAL 512 bytes
clients HEAD PATCHED
1 1464 1284
2 1520 1381
4 2985 2877
8 6237 5261
16 11296 10621
32 22257 20789
64 40548 37243
128 66507 59891
256 92516 97506
512 56404 119716
768 51127 112482
1024 48463 103484
2048 38079 81424
4096 18977 40942
test-case 3: -t1000, WAL 512 bytes
clients HEAD PATCHED
1 1452 1434
2 1604 1649
4 3051 2971
8 5967 5650
16 10471 10702
32 20257 20899
64 39412 36750
128 62767 61110
256 81050 89768
512 56888 122786
768 51238 114444
1024 48972 106867
2048 43451 98847
4096 40018 111079
test-case 4: -T5, WAL 1024 bytes
clients HEAD PATCHED
1 1405 1395
2 1638 1607
4 3176 3207
8 6271 6024
16 11653 11103
32 20530 20260
64 34313 32367
128 55939 52079
256 74355 76420
512 56506 90983
768 50088 100410
1024 44589 99025
2048 39640 90931
4096 20942 36035
test-case 4: -t1000, WAL 1024 bytes
clients HEAD PATCHED
1 1330 1304
2 1615 1366
4 3117 2667
8 6179 5390
16 10524 10426
32 19819 18620
64 34844 29731
128 52180 48869
256 73284 71396
512 55714 96014
768 49336 108100
1024 46113 102789
2048 44627 104721
4096 44979 106189
test-case 5: -T5, WAL 2048 bytes
clients HEAD PATCHED
1 1407 1377
2 1518 1559
4 2589 2870
8 4883 5493
16 9075 9201
32 15957 16295
64 27471 25029
128 37493 38642
256 46369 45787
512 61755 62836
768 59144 68419
1024 52495 68933
2048 48608 72500
4096 26463 61252
test-case 5: -t1000, WAL 2048 bytes
clients HEAD PATCHED
1 1289 1366
2 1489 1628
4 2960 3036
8 5536 5965
16 9248 10399
32 15770 18140
64 27626 27800
128 36817 39483
256 48533 52105
512 64453 64007
768 59146 64160
1024 57637 61756
2048 59063 62109
4096 58268 61206
test-case 6: -T5, WAL 4096 bytes
clients HEAD PATCHED
1 1322 1325
2 1504 1551
4 2811 2880
8 5330 5159
16 8625 8315
32 12820 13534
64 19737 19965
128 26298 24633
256 34630 29939
512 34382 36669
768 33421 33316
1024 33525 32821
2048 37053 37752
4096 37334 39114
test-case 6: -t1000, WAL 4096 bytes
clients HEAD PATCHED
1 1212 1371
2 1383 1566
4 2858 2967
8 5092 5035
16 8233 8486
32 13353 13678
64 19052 20072
128 24803 24726
256 34065 33139
512 31590 32029
768 31432 31404
1024 31357 31366
2048 31465 31508
4096 32157 32180
test-case 7: -T5, WAL 8192 bytes
clients HEAD PATCHED
1 1287 1233
2 1552 1521
4 2658 2617
8 4680 4532
16 6732 7110
32 9649 9198
64 13276 12042
128 17100 17187
256 17408 17448
512 16595 16358
768 16599 16500
1024 16975 17300
2048 19073 19137
4096 21368 21735
test-case 7: -t1000, WAL 8192 bytes
clients HEAD PATCHED
1 1144 1190
2 1414 1395
4 2618 2438
8 4645 4485
16 6766 7001
32 9620 9804
64 12943 13023
128 15904 17148
256 16645 16035
512 15800 15796
768 15788 15810
1024 15814 15817
2048 17775 17771
4096 31715 31682
> Looking at the patch.. LWLockConflictsWithVar() and
> LWLockReleaseClearVar() are the trivial bits. These are OK.
Hm, the crux of the patch is avoiding LWLock's waitlist lock for
reading/writing the lock variable. Essentially, they are important
bits.
> + * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
> + * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
> + * a full barrier, we're guaranteed that the subsequent shared memory
> + * reads/writes, if any, happen after we reset the lock variable.
>
> This mentions that the subsequent read/write operations are safe, so
> this refers to anything happening after the variable is reset. As
> a full barrier, should be also mention that this is also ordered with
> respect to anything that the caller did before clearing the variable?
> From this perspective using pg_atomic_exchange_u64() makes sense to me
> in LWLockReleaseClearVar().
Wordsmithed that comment a bit.
> + * XXX: Use of a spinlock at the beginning of this function to read
> + * current insert position implies memory ordering. That means that
> + * the immediate loads and stores to shared memory (for instance,
> + * in LWLockUpdateVar called via LWLockWaitForVar) don't need an
> + * explicit memory barrier as far as the current usage is
> + * concerned. But that might not be safe in general.
> */
> What's the part where this is not safe? Based on what I see, this
> code path is safe because of the previous spinlock. This is the same
> comment as at the beginning of LWLockConflictsWithVar(). Is that
> something that we ought to document at the top of LWLockWaitForVar()
> as well? We have one caller of this function currently, but there may
> be more in the future.
'But that might not be safe in general' applies only for
LWLockWaitForVar not for WaitXLogInsertionsToFinish for sure. My bad.
If there's another caller for LWLockWaitForVar without any spinlock,
that's when the LWLockWaitForVar needs to have an explicit memory
barrier.
Per a comment upthread
https://www.postgresql.org/message-id/20221205183007.s72oygp63s43dqyz%40awork3.anarazel.de,
I had a note in WaitXLogInsertionsToFinish before LWLockWaitForVar. I
now have modified that comment.
> - * you're about to write out.
> + * you're about to write out. Using an atomic variable for insertingAt avoids
> + * taking any explicit lock for reads and writes.
>
> Hmm. Not sure that we need to comment at all.
Removed. I was being verbose. One who understands pg_atomic_uint64 can
get to that point easily.
> -LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
> +LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
> [...]
> Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
>
> - /* Update the lock's value */
> - *valptr = val;
>
> The sensitive change is in LWLockUpdateVar(). I am not completely
> sure to understand this removal, though. Does that influence the case
> where there are waiters?
I'll send about this in a follow-up email to not overload this
response with too much data.
> Another thing I was wondering about: how much does the fast-path used
> in LWLockUpdateVar() influence the performance numbers? Am I right to
> guess that it counts for most of the gain seen?
I'll send about this in a follow-up email to not overload this
response with too much data.
> Or could it be that
> the removal of the spin lock in
> LWLockConflictsWithVar()/LWLockWaitForVar() the point that has the
> highest effect?
I'll send about this in a follow-up email to not overload this
response with too much data.
I've addressed the above review comments and attached the v6 patch.
[1]
test-case 1: -T5, WAL ~16 bytes HEAD:
+ 81.52% 0.03% postgres [.] __vstrfmon_l_internal
+ 81.52% 0.00% postgres [.] startup_hacks
+ 81.52% 0.00% postgres [.] PostmasterMain
+ 63.95% 1.01% postgres [.] LWLockWaitListLock
+ 61.93% 0.02% postgres [.] WaitXLogInsertionsToFinish
+ 61.89% 0.05% postgres [.] LWLockWaitForVar
+ 48.83% 48.33% postgres [.] pg_atomic_read_u32_impl
+ 48.78% 0.40% postgres [.] pg_atomic_read_u32
+ 43.19% 0.12% postgres [.] LWLockConflictsWithVar
+ 19.81% 0.01% postgres [.] LWLockQueueSelf
+ 7.86% 2.46% postgres [.] perform_spin_delay
+ 6.14% 6.06% postgres [.] spin_delay
+ 5.82% 0.01% postgres [.] pg_atomic_fetch_or_u32
+ 5.81% 5.76% postgres [.] pg_atomic_fetch_or_u32_impl
+ 4.00% 0.01% postgres [.] XLogInsert
+ 3.93% 0.03% postgres [.] XLogInsertRecord
+ 2.13% 0.02% postgres [.] LWLockRelease
+ 2.10% 0.03% postgres [.] LWLockAcquire
+ 1.92% 0.00% postgres [.] LWLockDequeueSelf
+ 1.87% 0.01% postgres [.] WALInsertLockAcquire
+ 1.68% 0.04% postgres [.] LWLockAcquireOrWait
+ 1.64% 0.01% postgres [.] pg_analyze_and_rewrite_fixedparams
+ 1.62% 0.00% postgres [.] WALInsertLockRelease
+ 1.62% 0.00% postgres [.] LWLockReleaseClearVar
+ 1.55% 0.01% postgres [.] parse_analyze_fixedparams
+ 1.51% 0.00% postgres [.] transformTopLevelStmt
+ 1.50% 0.00% postgres [.] transformOptionalSelectInto
+ 1.50% 0.01% postgres [.] transformStmt
+ 1.47% 0.02% postgres [.] transformSelectStmt
+ 1.29% 0.01% postgres [.] XactLogCommitRecord
test-case 1: -T5, WAL ~16 bytes PATCHED:
+ 74.49% 0.04% postgres [.] __vstrfmon_l_internal
+ 74.49% 0.00% postgres [.] startup_hacks
+ 74.49% 0.00% postgres [.] PostmasterMain
+ 51.60% 0.01% postgres [.] finish_xact_command
+ 51.60% 0.02% postgres [.] CommitTransactionCommand
+ 51.37% 0.03% postgres [.] CommitTransaction
+ 49.43% 0.05% postgres [.] RecordTransactionCommit
+ 46.55% 0.05% postgres [.] XLogFlush
+ 46.37% 0.85% postgres [.] LWLockWaitListLock
+ 43.79% 0.02% postgres [.] LWLockQueueSelf
+ 38.87% 0.03% postgres [.] WaitXLogInsertionsToFinish
+ 38.79% 0.11% postgres [.] LWLockWaitForVar
+ 34.99% 34.49% postgres [.] pg_atomic_read_u32_impl
+ 34.93% 0.35% postgres [.] pg_atomic_read_u32
+ 6.99% 2.12% postgres [.] perform_spin_delay
+ 6.64% 0.01% postgres [.] XLogInsert
+ 6.54% 0.06% postgres [.] XLogInsertRecord
+ 6.26% 0.08% postgres [.] LWLockAcquireOrWait
+ 5.31% 5.22% postgres [.] spin_delay
+ 4.23% 0.04% postgres [.] LWLockRelease
+ 3.74% 0.01% postgres [.] pg_atomic_fetch_or_u32
+ 3.73% 3.68% postgres [.] pg_atomic_fetch_or_u32_impl
+ 3.33% 0.06% postgres [.] LWLockAcquire
+ 2.97% 0.01% postgres [.] pg_plan_queries
+ 2.95% 0.01% postgres [.] WALInsertLockAcquire
+ 2.94% 0.02% postgres [.] planner
+ 2.94% 0.01% postgres [.] pg_plan_query
+ 2.92% 0.01% postgres [.] LWLockDequeueSelf
+ 2.89% 0.04% postgres [.] standard_planner
+ 2.81% 0.00% postgres [.] WALInsertLockRelease
+ 2.80% 0.00% postgres [.] LWLockReleaseClearVar
+ 2.38% 0.07% postgres [.] subquery_planner
+ 2.35% 0.01% postgres [.] XactLogCommitRecord
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/zip] TestArtifacts.zip (234.7K, ../../CALj2ACWGHGJ0pCsuVqJ4QkEfv8D_69rZL0FUK7TFfukaOcf7YQ@mail.gmail.com/2-TestArtifacts.zip)
download
[application/x-patch] v6-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch (8.7K, ../../CALj2ACWGHGJ0pCsuVqJ4QkEfv8D_69rZL0FUK7TFfukaOcf7YQ@mail.gmail.com/3-v6-0001-Optimize-WAL-insertion-lock-acquisition-and-relea.patch)
download | inline diff:
From 4835d9f1c990940e7c73cbae16cc2cfdb855add6 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Mon, 8 May 2023 11:31:08 +0000
Subject: [PATCH v6] Optimize WAL insertion lock acquisition and release
This commit optimizes WAL insertion lock acquisition and release
in the following way:
1. WAL insertion lock's variable insertingAt is currently read and
written with the help of lwlock's wait list lock to avoid
torn-free reads/writes. This wait list lock can become a point of
contention on a highly concurrent write workloads. Therefore, make
insertingAt a 64-bit atomic which inherently provides torn-free
reads/writes.
2. LWLockUpdateVar currently acquires lwlock's wait list lock even
when there are no waiters at all. Add a fastpath exit to
LWLockUpdateVar when there are no waiters to avoid unnecessary
locking.
Note that atomic exchange operation (which is a full barrier) is
used when necessary, instead of atomic write to ensure the memory
ordering is preserved.
It also adds a note in WaitXLogInsertionsToFinish regarding how the
use of spinlock there can avoid explicit memory barrier in some
subsequently called functions.
Suggested-by: Andres Freund
Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart
Discussion: https://www.postgresql.org/message-id/20221124184619.xit4sfi52bcz2tva%40awork3.anarazel.de
---
src/backend/access/transam/xlog.c | 8 +++-
src/backend/storage/lmgr/lwlock.c | 67 +++++++++++++++++++------------
src/include/storage/lwlock.h | 6 +--
3 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index bc5a8e0569..92b0b87d1e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -376,7 +376,7 @@ typedef struct XLogwrtResult
typedef struct
{
LWLock lock;
- XLogRecPtr insertingAt;
+ pg_atomic_uint64 insertingAt;
XLogRecPtr lastImportantAt;
} WALInsertLock;
@@ -1495,6 +1495,10 @@ WaitXLogInsertionsToFinish(XLogRecPtr upto)
* calling LWLockUpdateVar. But if it has to sleep, it will
* advertise the insertion point with LWLockUpdateVar before
* sleeping.
+ *
+ * NB: LWLockConflictsWithVar (which is called from
+ * LWLockWaitForVar) relies on the spinlock used above in this
+ * function and doesn't use a memory barrier.
*/
if (LWLockWaitForVar(&WALInsertLocks[i].l.lock,
&WALInsertLocks[i].l.insertingAt,
@@ -4611,7 +4615,7 @@ XLOGShmemInit(void)
for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
{
LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
- WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr;
+ pg_atomic_init_u64(&WALInsertLocks[i].l.insertingAt, InvalidXLogRecPtr);
WALInsertLocks[i].l.lastImportantAt = InvalidXLogRecPtr;
}
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 59347ab951..3cca22d2b9 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1547,9 +1547,8 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
* *result is set to true if the lock was free, and false otherwise.
*/
static bool
-LWLockConflictsWithVar(LWLock *lock,
- uint64 *valptr, uint64 oldval, uint64 *newval,
- bool *result)
+LWLockConflictsWithVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval, bool *result)
{
bool mustwait;
uint64 value;
@@ -1572,13 +1571,11 @@ LWLockConflictsWithVar(LWLock *lock,
*result = false;
/*
- * Read value using the lwlock's wait list lock, as we can't generally
- * rely on atomic 64 bit reads/stores. TODO: On platforms with a way to
- * do atomic 64 bit reads/writes the spinlock should be optimized away.
+ * Reading the value atomically ensures that we don't need any explicit
+ * locking. Note that in general, 64 bit atomic APIs in postgres inherently
+ * provide explicit locking for the platforms without atomics support.
*/
- LWLockWaitListLock(lock);
- value = *valptr;
- LWLockWaitListUnlock(lock);
+ value = pg_atomic_read_u64(valptr);
if (value != oldval)
{
@@ -1607,7 +1604,8 @@ LWLockConflictsWithVar(LWLock *lock,
* in shared mode, returns 'true'.
*/
bool
-LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
+LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval,
+ uint64 *newval)
{
PGPROC *proc = MyProc;
int extraWaits = 0;
@@ -1735,29 +1733,47 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
* LWLockUpdateVar - Update a variable and wake up waiters atomically
*
* Sets *valptr to 'val', and wakes up all processes waiting for us with
- * LWLockWaitForVar(). Setting the value and waking up the processes happen
- * atomically so that any process calling LWLockWaitForVar() on the same lock
- * is guaranteed to see the new value, and act accordingly.
+ * LWLockWaitForVar(). It first sets the value atomically and then wakes up
+ * the waiting processes so that any process calling LWLockWaitForVar() on the
+ * same lock is guaranteed to see the new value, and act accordingly.
*
* The caller must be holding the lock in exclusive mode.
*/
void
-LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
proclist_head wakeup;
proclist_mutable_iter iter;
PRINT_LWDEBUG("LWLockUpdateVar", lock, LW_EXCLUSIVE);
+ /*
+ * Update the variable atomically first without having to acquire wait
+ * list lock, so that if anyone looking for the lock will have chance to
+ * grab it a bit quickly.
+ *
+ * NB: pg_atomic_exchange_u64 is used here as opposed to just
+ * pg_atomic_write_u64 to update the variable. Since pg_atomic_exchange_u64
+ * is a full barrier, we're guaranteed that all loads and stores issued
+ * prior to setting the variable are completed before any loads or stores
+ * issued after setting the variable. In other words, a barrier here
+ * ensures the variable is updated before waking up waiters.
+ */
+ pg_atomic_exchange_u64(valptr, val);
+
+ /*
+ * Quick exit when there are no waiters. This avoids unnecessary lwlock's
+ * wait list lock acquisition and release.
+ */
+ if ((pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) == 0)
+ return;
+
proclist_init(&wakeup);
LWLockWaitListLock(lock);
Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE);
- /* Update the lock's value */
- *valptr = val;
-
/*
* See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken
* up. They are always in the front of the queue.
@@ -1873,17 +1889,18 @@ LWLockRelease(LWLock *lock)
* LWLockReleaseClearVar - release a previously acquired lock, reset variable
*/
void
-LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val)
+LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val)
{
- LWLockWaitListLock(lock);
-
/*
- * Set the variable's value before releasing the lock, that prevents race
- * a race condition wherein a new locker acquires the lock, but hasn't yet
- * set the variables value.
+ * Update the variable atomically first.
+ *
+ * NB: pg_atomic_exchange_u64 is used here as opposed to just
+ * pg_atomic_write_u64 to update the variable. Since pg_atomic_exchange_u64
+ * is a full barrier, we're guaranteed that all loads and stores issued
+ * prior to setting the variable are completed before any loads or stores
+ * issued after setting the variable.
*/
- *valptr = val;
- LWLockWaitListUnlock(lock);
+ pg_atomic_exchange_u64(valptr, val);
LWLockRelease(lock);
}
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index d2c7afb8f4..f19bc49193 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -128,14 +128,14 @@ extern bool LWLockAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockConditionalAcquire(LWLock *lock, LWLockMode mode);
extern bool LWLockAcquireOrWait(LWLock *lock, LWLockMode mode);
extern void LWLockRelease(LWLock *lock);
-extern void LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern void LWLockReleaseAll(void);
extern bool LWLockHeldByMe(LWLock *lock);
extern bool LWLockAnyHeldByMe(LWLock *lock, int nlocks, size_t stride);
extern bool LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode);
-extern bool LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval);
-extern void LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val);
+extern bool LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, uint64 *newval);
+extern void LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val);
extern Size LWLockShmemSize(void);
extern void CreateLWLocks(void);
--
2.34.1
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-08 23:04 ` Nathan Bossart <[email protected]>
2023-05-09 03:32 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Nathan Bossart @ 2023-05-08 23:04 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Michael Paquier <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote:
> I ran performance tests on the patch with different use-cases. Clearly
> the patch reduces burden on LWLock's waitlist lock (evident from perf
> reports [1]). However, to see visible impact in the output, the txns
> must be generating small (between 16 bytes to 2 KB) amounts of WAL in
> a highly concurrent manner, check the results below (FWIW, I've zipped
> and attached perf images for better illustration along with test
> setup).
>
> When the txns are generating a small amount of WAL i.e. between 16
> bytes to 2 KB in a highly concurrent manner, the benefit is clearly
> visible in the TPS more than 2.3X improvement. When the txns are
> generating more WAL i.e. more than 2 KB, the gain from reduced burden
> on waitlist lock is offset by increase in the wait/release for WAL
> insertion locks and no visible benefit is seen.
>
> As the amount of WAL each txn generates increases, it looks like the
> benefit gained from reduced burden on waitlist lock is offset by
> increase in the wait for WAL insertion locks.
Nice.
> test-case 1: -T5, WAL ~16 bytes
> test-case 1: -t1000, WAL ~16 bytes
I wonder if it's worth doing a couple of long-running tests, too.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
@ 2023-05-09 03:32 ` Michael Paquier <[email protected]>
2023-05-09 03:54 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Michael Paquier @ 2023-05-09 03:32 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 08, 2023 at 04:04:10PM -0700, Nathan Bossart wrote:
> On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote:
>> test-case 1: -T5, WAL ~16 bytes
>> test-case 1: -t1000, WAL ~16 bytes
>
> I wonder if it's worth doing a couple of long-running tests, too.
Yes, 5s or 1000 transactions per client is too small, though it shows
that things are going in the right direction.
(Will reply to the rest in a bit..)
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-05-09 03:32 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
@ 2023-05-09 03:54 ` Bharath Rupireddy <[email protected]>
2023-05-10 17:10 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Bharath Rupireddy @ 2023-05-09 03:54 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, May 9, 2023 at 9:02 AM Michael Paquier <[email protected]> wrote:
>
> On Mon, May 08, 2023 at 04:04:10PM -0700, Nathan Bossart wrote:
> > On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote:
> >> test-case 1: -T5, WAL ~16 bytes
> >> test-case 1: -t1000, WAL ~16 bytes
> >
> > I wonder if it's worth doing a couple of long-running tests, too.
>
> Yes, 5s or 1000 transactions per client is too small, though it shows
> that things are going in the right direction.
I'll pick a test case that generates a reasonable amount of WAL 256
bytes. What do you think of the following?
test-case 2: -T900, WAL ~256 bytes (for c in 1 2 4 8 16 32 64 128 256
512 768 1024 2048 4096 - takes 3.5hrs)
test-case 2: -t1000000, WAL ~256 bytes
If okay, I'll fire the tests.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-05-09 03:32 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-09 03:54 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-10 17:10 ` Bharath Rupireddy <[email protected]>
2023-05-10 23:31 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Bharath Rupireddy @ 2023-05-10 17:10 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, May 9, 2023 at 9:24 AM Bharath Rupireddy
<[email protected]> wrote:
>
> On Tue, May 9, 2023 at 9:02 AM Michael Paquier <[email protected]> wrote:
> >
> > On Mon, May 08, 2023 at 04:04:10PM -0700, Nathan Bossart wrote:
> > > On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote:
> > >> test-case 1: -T5, WAL ~16 bytes
> > >> test-case 1: -t1000, WAL ~16 bytes
> > >
> > > I wonder if it's worth doing a couple of long-running tests, too.
> >
> > Yes, 5s or 1000 transactions per client is too small, though it shows
> > that things are going in the right direction.
>
> I'll pick a test case that generates a reasonable amount of WAL 256
> bytes. What do you think of the following?
>
> test-case 2: -T900, WAL ~256 bytes (for c in 1 2 4 8 16 32 64 128 256
> 512 768 1024 2048 4096 - takes 3.5hrs)
> test-case 2: -t1000000, WAL ~256 bytes
>
> If okay, I'll fire the tests.
test-case 2: -T900, WAL ~256 bytes - ran for about 3.5 hours and the
more than 3X improvement in TPS is seen - 3.11X @ 512 3.79 @ 768, 3.47
@ 1024, 2.27 @ 2048, 2.77 @ 4096
test-case 2: -T900, WAL ~256 bytes
clients HEAD PATCHED
1 1394 1351
2 1551 1445
4 3104 2881
8 5974 5774
16 12154 11319
32 22438 21606
64 43689 40567
128 80726 77993
256 139987 141638
512 60108 187126
768 51188 194406
1024 48766 169353
2048 46617 105961
4096 44163 122697
test-case 2: -t1000000, WAL ~256 bytes - ran for more than 12 hours
and the maximum improvement is 1.84X @ 1024 client.
test-case 2: -t1000000, WAL ~256 bytes
clients HEAD PATCHED
1 1454 1500
2 1657 1612
4 3223 3224
8 6305 6295
16 12447 12260
32 24855 24335
64 45229 44386
128 80752 79518
256 120663 119083
512 149546 159396
768 118298 181732
1024 101829 187492
2048 107506 191378
4096 125130 186728
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[image/png] test-case 2 -T900 WAL ~256 bytes.png (15.9K, ../../CALj2ACWddeHz0E6uetxJdi=2xJrBdkMErw6eV9ykQ=bsNZ_qwQ@mail.gmail.com/2-test-case%202%20-T900%20WAL%20~256%20bytes.png)
download | view image
[image/png] test-case 2 -t1000000 WAL ~256 bytes.png (14.7K, ../../CALj2ACWddeHz0E6uetxJdi=2xJrBdkMErw6eV9ykQ=bsNZ_qwQ@mail.gmail.com/3-test-case%202%20-t1000000%20WAL%20~256%20bytes.png)
download | view image
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-05-09 03:32 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-09 03:54 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 17:10 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-10 23:31 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Michael Paquier @ 2023-05-10 23:31 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, May 10, 2023 at 10:40:20PM +0530, Bharath Rupireddy wrote:
> test-case 2: -T900, WAL ~256 bytes - ran for about 3.5 hours and the
> more than 3X improvement in TPS is seen - 3.11X @ 512 3.79 @ 768, 3.47
> @ 1024, 2.27 @ 2048, 2.77 @ 4096
>
> [...]
>
> test-case 2: -t1000000, WAL ~256 bytes - ran for more than 12 hours
> and the maximum improvement is 1.84X @ 1024 client.
Thanks. So that's pretty close to what I was seeing when it comes to
this message size where you see much more effects under a number of
clients of at least 512~. Any of these tests have been using fsync =
on, I assume. I think that disabling fsync or just mounting pg_wal to
a tmpfs should show the same pattern for larger record sizes (after 1k
of message size the curve begins to go down with 512~ clients).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-10 12:04 ` Michael Paquier <[email protected]>
2023-05-11 06:26 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
1 sibling, 1 reply; 16+ messages in thread
From: Michael Paquier @ 2023-05-10 12:04 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 08, 2023 at 05:57:09PM +0530, Bharath Rupireddy wrote:
> Note that I've used pg_logical_emit_message() for ease of
> understanding about the txns generating various amounts of WAL, but
> the pattern is the same if txns are generating various amounts of WAL
> say with inserts.
Sounds good to me to just rely on that for some comparison numbers.
+ * NB: LWLockConflictsWithVar (which is called from
+ * LWLockWaitForVar) relies on the spinlock used above in this
+ * function and doesn't use a memory barrier.
This patch adds the following comment in WaitXLogInsertionsToFinish()
because lwlock.c on HEAD mentions that:
/*
* Test first to see if it the slot is free right now.
*
* XXX: the caller uses a spinlock before this, so we don't need a memory
* barrier here as far as the current usage is concerned. But that might
* not be safe in general.
*/
Should it be something where we'd better be noisy about at the top of
LWLockWaitForVar()? We don't want to add a memory barrier at the
beginning of LWLockConflictsWithVar(), still it strikes me that
somebody that aims at using LWLockWaitForVar() may miss this point
because LWLockWaitForVar() is the routine published in lwlock.h, not
LWLockConflictsWithVar(). This does not need to be really
complicated, say a note at the top of LWLockWaitForVar() among the
lines of (?):
"Be careful that LWLockConflictsWithVar() does not include a memory
barrier, hence the caller of this function may want to rely on an
explicit barrier or a spinlock to avoid memory ordering issues."
>> + * NB: Note the use of pg_atomic_exchange_u64 as opposed to just
>> + * pg_atomic_write_u64 to update the value. Since pg_atomic_exchange_u64 is
>> + * a full barrier, we're guaranteed that the subsequent shared memory
>> + * reads/writes, if any, happen after we reset the lock variable.
>>
>> This mentions that the subsequent read/write operations are safe, so
>> this refers to anything happening after the variable is reset. As
>> a full barrier, should be also mention that this is also ordered with
>> respect to anything that the caller did before clearing the variable?
>> From this perspective using pg_atomic_exchange_u64() makes sense to me
>> in LWLockReleaseClearVar().
>
> Wordsmithed that comment a bit.
- * Set the variable's value before releasing the lock, that prevents race
- * a race condition wherein a new locker acquires the lock, but hasn't yet
- * set the variables value.
[...]
+ * NB: pg_atomic_exchange_u64 is used here as opposed to just
+ * pg_atomic_write_u64 to update the variable. Since pg_atomic_exchange_u64
+ * is a full barrier, we're guaranteed that all loads and stores issued
+ * prior to setting the variable are completed before any loads or stores
+ * issued after setting the variable.
This is the same explanation as LWLockUpdateVar(), except that we
lose the details explaining why we are doing the update before
releasing the lock.
It took me some time, but I have been able to deploy a big box to see
the effect of this patch at a rather large scale (64 vCPU, 512G of
memory), with the following test characteristics for HEAD and v6:
- TPS comparison with pgbench and pg_logical_emit_message().
- Record sizes of 16, 64, 256, 1k, 4k and 16k.
- Clients and jobs equal at 4, 16, 64, 256, 512, 1024, 2048, 4096.
- Runs of 3 mins for each of the 48 combinations, meaning 96 runs in
total.
And here are the results I got:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16k
------------------|--------|--------|--------|--------|-------|-------
head_4_clients | 3026 | 2965 | 2846 | 2880 | 2778 | 2412
head_16_clients | 12087 | 11287 | 11670 | 11100 | 9003 | 5608
head_64_clients | 42995 | 44005 | 43592 | 35437 | 21533 | 11273
head_256_clients | 106775 | 109233 | 104201 | 80759 | 42118 | 16254
head_512_clients | 153849 | 156950 | 142915 | 99288 | 57714 | 16198
head_1024_clients | 122102 | 123895 | 114248 | 117317 | 62270 | 16261
head_2048_clients | 126730 | 115594 | 109671 | 119564 | 62454 | 16298
head_4096_clients | 111564 | 111697 | 119164 | 123483 | 62430 | 16140
v6_4_clients | 2893 | 2917 | 3087 | 2904 | 2786 | 2262
v6_16_clients | 12097 | 11387 | 11579 | 11242 | 9228 | 5661
v6_64_clients | 45124 | 46533 | 42275 | 36124 | 21696 | 11386
v6_256_clients | 121500 | 125732 | 104328 | 78989 | 41949 | 16254
v6_512_clients | 164120 | 174743 | 146677 | 98110 | 60228 | 16171
v6_1024_clients | 168990 | 180710 | 149894 | 117431 | 62271 | 16259
v6_2048_clients | 165426 | 162893 | 146322 | 132476 | 62468 | 16274
v6_4096_clients | 161283 | 158732 | 162474 | 135636 | 62461 | 16030
These tests are not showing me any degradation, and a correlation
between the record size and the number of clients where the TPS begins
to show a difference between HEAD and v6 of the patch. In short the
shorter the record, the better performance gets at a lower client
number, still this required at least 256~512 clients with even
messages of 16bytes. At the end I'm cool with that.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 12:04 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
@ 2023-05-11 06:26 ` Michael Paquier <[email protected]>
2023-05-12 02:05 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Michael Paquier @ 2023-05-11 06:26 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, May 10, 2023 at 09:04:47PM +0900, Michael Paquier wrote:
> It took me some time, but I have been able to deploy a big box to see
> the effect of this patch at a rather large scale (64 vCPU, 512G of
> memory), with the following test characteristics for HEAD and v6:
> - TPS comparison with pgbench and pg_logical_emit_message().
> - Record sizes of 16, 64, 256, 1k, 4k and 16k.
> - Clients and jobs equal at 4, 16, 64, 256, 512, 1024, 2048, 4096.
> - Runs of 3 mins for each of the 48 combinations, meaning 96 runs in
> total.
>
> And here are the results I got:
> message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16k
> ------------------|--------|--------|--------|--------|-------|-------
> head_4_clients | 3026 | 2965 | 2846 | 2880 | 2778 | 2412
> head_16_clients | 12087 | 11287 | 11670 | 11100 | 9003 | 5608
> head_64_clients | 42995 | 44005 | 43592 | 35437 | 21533 | 11273
> head_256_clients | 106775 | 109233 | 104201 | 80759 | 42118 | 16254
> head_512_clients | 153849 | 156950 | 142915 | 99288 | 57714 | 16198
> head_1024_clients | 122102 | 123895 | 114248 | 117317 | 62270 | 16261
> head_2048_clients | 126730 | 115594 | 109671 | 119564 | 62454 | 16298
> head_4096_clients | 111564 | 111697 | 119164 | 123483 | 62430 | 16140
> v6_4_clients | 2893 | 2917 | 3087 | 2904 | 2786 | 2262
> v6_16_clients | 12097 | 11387 | 11579 | 11242 | 9228 | 5661
> v6_64_clients | 45124 | 46533 | 42275 | 36124 | 21696 | 11386
> v6_256_clients | 121500 | 125732 | 104328 | 78989 | 41949 | 16254
> v6_512_clients | 164120 | 174743 | 146677 | 98110 | 60228 | 16171
> v6_1024_clients | 168990 | 180710 | 149894 | 117431 | 62271 | 16259
> v6_2048_clients | 165426 | 162893 | 146322 | 132476 | 62468 | 16274
> v6_4096_clients | 161283 | 158732 | 162474 | 135636 | 62461 | 16030
Another thing I was wondering is if it would be able to see a
difference by reducing the I/O pressure. After mounting pg_wal to a
tmpfs, I am getting the following table:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16000
-------------------+--------+--------+--------+--------+--------+-------
head_4_clients | 86476 | 86592 | 84645 | 76784 | 57887 | 30199
head_16_clients | 277006 | 278431 | 263238 | 228614 | 143880 | 67237
head_64_clients | 373972 | 370082 | 352217 | 297377 | 190974 | 96843
head_256_clients | 144510 | 147077 | 146281 | 189059 | 156294 | 88345
head_512_clients | 122863 | 119054 | 127790 | 162187 | 142771 | 84109
head_1024_clients | 140802 | 138728 | 147200 | 172449 | 138022 | 81054
head_2048_clients | 175950 | 164143 | 154070 | 161432 | 128205 | 76732
head_4096_clients | 161438 | 158666 | 152057 | 139520 | 113955 | 69335
v6_4_clients | 87356 | 86985 | 83933 | 76397 | 57352 | 30084
v6_16_clients | 277466 | 280125 | 259733 | 224916 | 143832 | 66589
v6_64_clients | 388352 | 386188 | 362358 | 302719 | 190353 | 96687
v6_256_clients | 365797 | 360114 | 337135 | 266851 | 172252 | 88898
v6_512_clients | 339751 | 332384 | 308182 | 249624 | 158868 | 84258
v6_1024_clients | 301294 | 295140 | 276769 | 226034 | 148392 | 80909
v6_2048_clients | 268846 | 261001 | 247110 | 205332 | 137271 | 76299
v6_4096_clients | 229322 | 227049 | 217271 | 183708 | 124888 | 69263
This shows more difference from 64 clients up to 4k records, without
degradation noticed across the board.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 12:04 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-11 06:26 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
@ 2023-05-12 02:05 ` Bharath Rupireddy <[email protected]>
2023-05-12 22:56 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
0 siblings, 1 reply; 16+ messages in thread
From: Bharath Rupireddy @ 2023-05-12 02:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, May 11, 2023 at 11:56 AM Michael Paquier <[email protected]> wrote:
>
> On Wed, May 10, 2023 at 09:04:47PM +0900, Michael Paquier wrote:
> > It took me some time, but I have been able to deploy a big box to see
> > the effect of this patch at a rather large scale (64 vCPU, 512G of
> > memory), with the following test characteristics for HEAD and v6:
> > - TPS comparison with pgbench and pg_logical_emit_message().
> > - Record sizes of 16, 64, 256, 1k, 4k and 16k.
> > - Clients and jobs equal at 4, 16, 64, 256, 512, 1024, 2048, 4096.
> > - Runs of 3 mins for each of the 48 combinations, meaning 96 runs in
> > total.
> >
> > And here are the results I got:
> > message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16k
> > ------------------|--------|--------|--------|--------|-------|-------
> > head_4_clients | 3026 | 2965 | 2846 | 2880 | 2778 | 2412
> > head_16_clients | 12087 | 11287 | 11670 | 11100 | 9003 | 5608
> > head_64_clients | 42995 | 44005 | 43592 | 35437 | 21533 | 11273
> > head_256_clients | 106775 | 109233 | 104201 | 80759 | 42118 | 16254
> > head_512_clients | 153849 | 156950 | 142915 | 99288 | 57714 | 16198
> > head_1024_clients | 122102 | 123895 | 114248 | 117317 | 62270 | 16261
> > head_2048_clients | 126730 | 115594 | 109671 | 119564 | 62454 | 16298
> > head_4096_clients | 111564 | 111697 | 119164 | 123483 | 62430 | 16140
> > v6_4_clients | 2893 | 2917 | 3087 | 2904 | 2786 | 2262
> > v6_16_clients | 12097 | 11387 | 11579 | 11242 | 9228 | 5661
> > v6_64_clients | 45124 | 46533 | 42275 | 36124 | 21696 | 11386
> > v6_256_clients | 121500 | 125732 | 104328 | 78989 | 41949 | 16254
> > v6_512_clients | 164120 | 174743 | 146677 | 98110 | 60228 | 16171
> > v6_1024_clients | 168990 | 180710 | 149894 | 117431 | 62271 | 16259
> > v6_2048_clients | 165426 | 162893 | 146322 | 132476 | 62468 | 16274
> > v6_4096_clients | 161283 | 158732 | 162474 | 135636 | 62461 | 16030
>
> Another thing I was wondering is if it would be able to see a
> difference by reducing the I/O pressure. After mounting pg_wal to a
> tmpfs, I am getting the following table:
> message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16000
> -------------------+--------+--------+--------+--------+--------+-------
> head_4_clients | 86476 | 86592 | 84645 | 76784 | 57887 | 30199
> head_16_clients | 277006 | 278431 | 263238 | 228614 | 143880 | 67237
> head_64_clients | 373972 | 370082 | 352217 | 297377 | 190974 | 96843
> head_256_clients | 144510 | 147077 | 146281 | 189059 | 156294 | 88345
> head_512_clients | 122863 | 119054 | 127790 | 162187 | 142771 | 84109
> head_1024_clients | 140802 | 138728 | 147200 | 172449 | 138022 | 81054
> head_2048_clients | 175950 | 164143 | 154070 | 161432 | 128205 | 76732
> head_4096_clients | 161438 | 158666 | 152057 | 139520 | 113955 | 69335
> v6_4_clients | 87356 | 86985 | 83933 | 76397 | 57352 | 30084
> v6_16_clients | 277466 | 280125 | 259733 | 224916 | 143832 | 66589
> v6_64_clients | 388352 | 386188 | 362358 | 302719 | 190353 | 96687
> v6_256_clients | 365797 | 360114 | 337135 | 266851 | 172252 | 88898
> v6_512_clients | 339751 | 332384 | 308182 | 249624 | 158868 | 84258
> v6_1024_clients | 301294 | 295140 | 276769 | 226034 | 148392 | 80909
> v6_2048_clients | 268846 | 261001 | 247110 | 205332 | 137271 | 76299
> v6_4096_clients | 229322 | 227049 | 217271 | 183708 | 124888 | 69263
>
> This shows more difference from 64 clients up to 4k records, without
> degradation noticed across the board.
Impressive. I further covered the following test cases. There's a
clear gain with the patch i.e. reducing burden on LWLock's waitlist
lock is helping out.
fsync=off, -T120:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16384
-------------------+--------+--------+--------+--------+--------+--------
head_1_clients | 33609 | 33862 | 32975 | 29722 | 21842 | 10606
head_2_clients | 60583 | 60524 | 57833 | 53582 | 38583 | 20120
head_4_clients | 115209 | 114012 | 114077 | 102991 | 73452 | 39179
head_8_clients | 181786 | 177592 | 174404 | 155350 | 98642 | 41406
head_16_clients | 313750 | 309024 | 295375 | 253101 | 159328 | 73617
head_32_clients | 406456 | 416809 | 400527 | 344573 | 213756 | 96322
head_64_clients | 199619 | 197948 | 198871 | 208606 | 221751 | 107762
head_128_clients | 108576 | 108727 | 107606 | 112137 | 173998 | 106976
head_256_clients | 75303 | 74983 | 73986 | 76100 | 148209 | 98080
head_512_clients | 62559 | 60189 | 59588 | 61102 | 131803 | 90534
head_768_clients | 55650 | 54486 | 54813 | 55515 | 120707 | 88009
head_1024_clients | 54709 | 52395 | 51672 | 52910 | 113904 | 86116
head_2048_clients | 48640 | 47098 | 46787 | 47582 | 98394 | 80766
head_4096_clients | 43205 | 42709 | 42591 | 43649 | 88903 | 72362
v6_1_clients | 33337 | 32877 | 31880 | 29372 | 21695 | 10596
v6_2_clients | 60125 | 60682 | 58770 | 53709 | 38390 | 20266
v6_4_clients | 115338 | 114053 | 114232 | 93527 | 74409 | 40437
v6_8_clients | 179472 | 183899 | 175474 | 154547 | 101807 | 43508
v6_16_clients | 318181 | 318580 | 296591 | 258094 | 159351 | 74758
v6_32_clients | 439681 | 447005 | 428459 | 367307 | 218511 | 97635
v6_64_clients | 473440 | 478388 | 464287 | 394825 | 244365 | 109194
v6_128_clients | 384433 | 412694 | 405916 | 366046 | 232421 | 110274
v6_256_clients | 312480 | 303635 | 291900 | 307573 | 206784 | 104171
v6_512_clients | 218560 | 189207 | 216267 | 252513 | 186762 | 97918
v6_768_clients | 168432 | 155493 | 145941 | 226616 | 178178 | 95435
v6_1024_clients | 150300 | 132078 | 134657 | 224515 | 172950 | 94356
v6_2048_clients | 126941 | 120189 | 120702 | 195684 | 158683 | 88055
v6_4096_clients | 163993 | 140795 | 139702 | 170149 | 139740 | 78907
pg_wal on tmpfs, -T180:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16384
-------------------+--------+--------+--------+--------+--------+--------
head_1_clients | 32956 | 32766 | 32244 | 29772 | 22094 | 11212
head_2_clients | 60093 | 60382 | 58825 | 53812 | 39764 | 20953
head_4_clients | 117178 | 104986 | 112060 | 103416 | 75588 | 39753
head_8_clients | 177556 | 179926 | 173413 | 156684 | 107727 | 42001
head_16_clients | 311033 | 313842 | 298362 | 261298 | 165293 | 76183
head_32_clients | 425750 | 433988 | 419193 | 370925 | 227392 | 101638
head_64_clients | 227463 | 219832 | 221421 | 235603 | 236601 | 113677
head_128_clients | 117188 | 116847 | 118414 | 123605 | 194533 | 111480
head_256_clients | 80596 | 80541 | 79130 | 83949 | 167529 | 102401
head_512_clients | 64912 | 63610 | 63209 | 65554 | 146882 | 94936
head_768_clients | 59050 | 57082 | 57061 | 58966 | 133336 | 92389
head_1024_clients | 56880 | 54951 | 54864 | 56554 | 125270 | 90893
head_2048_clients | 52148 | 49603 | 50422 | 50692 | 110789 | 86659
head_4096_clients | 47001 | 46992 | 46075 | 47793 | 99617 | 77762
v6_1_clients | 32915 | 32854 | 31676 | 29341 | 21956 | 11220
v6_2_clients | 59592 | 59146 | 58106 | 53235 | 38973 | 20943
v6_4_clients | 113947 | 114897 | 97349 | 104630 | 73628 | 40719
v6_8_clients | 177996 | 179673 | 176190 | 156831 | 104183 | 42884
v6_16_clients | 312284 | 317065 | 300130 | 268788 | 165765 | 77299
v6_32_clients | 443101 | 450025 | 436774 | 380398 | 229081 | 101916
v6_64_clients | 450794 | 469633 | 470252 | 411374 | 253232 | 113722
v6_128_clients | 413357 | 399514 | 386713 | 364070 | 236133 | 112780
v6_256_clients | 264674 | 252701 | 268273 | 296090 | 208050 | 105477
v6_512_clients | 196481 | 154815 | 158316 | 238805 | 188363 | 99507
v6_768_clients | 139839 | 132645 | 131391 | 219846 | 179226 | 97808
v6_1024_clients | 124540 | 119543 | 120140 | 206740 | 174657 | 96629
v6_2048_clients | 118793 | 113033 | 113881 | 190997 | 161421 | 91888
v6_4096_clients | 156341 | 156971 | 131391 | 177024 | 146564 | 84096
--enable-atomics=no, -T60:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16384
-------------------+-------+-------+-------+-------+-------+-------
head_1_clients | 1701 | 1686 | 1636 | 1693 | 1523 | 1331
head_2_clients | 1751 | 1712 | 1698 | 1769 | 1690 | 1579
head_4_clients | 3328 | 3370 | 3405 | 3495 | 3107 | 2713
head_8_clients | 6580 | 6521 | 6459 | 6370 | 5470 | 4253
head_16_clients | 13433 | 13476 | 12986 | 11461 | 9249 | 6313
head_32_clients | 25697 | 26729 | 24879 | 20862 | 14344 | 9454
head_64_clients | 51499 | 48322 | 46297 | 35224 | 20970 | 13241
head_128_clients | 56777 | 57177 | 59129 | 47687 | 27591 | 16007
head_256_clients | 9555 | 10041 | 9526 | 9830 | 13179 | 15776
head_512_clients | 5795 | 5871 | 5809 | 5954 | 5828 | 15647
head_768_clients | 4322 | 4366 | 4782 | 4624 | 4853 | 12959
head_1024_clients | 4003 | 3789 | 3647 | 3865 | 4160 | 7991
head_2048_clients | 2687 | 2573 | 2569 | 2829 | 2918 | 5462
head_4096_clients | 1694 | 1802 | 1813 | 1948 | 2256 | 5862
v6_1_clients | 1560 | 1595 | 1690 | 1621 | 1526 | 1374
v6_2_clients | 1737 | 1736 | 1738 | 1663 | 1601 | 1568
v6_4_clients | 3575 | 3583 | 3449 | 3137 | 3157 | 2788
v6_8_clients | 6660 | 6900 | 6802 | 6158 | 5605 | 4521
v6_16_clients | 14084 | 12991 | 13485 | 12628 | 10025 | 6211
v6_32_clients | 26408 | 24652 | 24672 | 21441 | 14966 | 9753
v6_64_clients | 49537 | 47703 | 45583 | 33524 | 21476 | 13259
v6_128_clients | 86938 | 79745 | 73740 | 53007 | 34863 | 15901
v6_256_clients | 20391 | 21433 | 21730 | 30836 | 43821 | 15891
v6_512_clients | 13128 | 12181 | 12309 | 11596 | 14744 | 15851
v6_768_clients | 10511 | 9942 | 9713 | 9373 | 10181 | 15964
v6_1024_clients | 9264 | 8745 | 8031 | 7500 | 8762 | 15198
v6_2048_clients | 6070 | 5724 | 5939 | 5987 | 5513 | 10828
v6_4096_clients | 4322 | 4035 | 3616 | 3637 | 5628 | 10970
--enable-spinlocks=no, -T60:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16384
-------------------+--------+--------+--------+--------+-------+-------
head_1_clients | 1644 | 1716 | 1701 | 1636 | 1569 | 1368
head_2_clients | 1779 | 1875 | 1728 | 1728 | 1770 | 1568
head_4_clients | 3448 | 3569 | 3330 | 3324 | 3319 | 2780
head_8_clients | 6159 | 6996 | 6893 | 6401 | 6308 | 4423
head_16_clients | 13195 | 13810 | 13139 | 12892 | 10744 | 6714
head_32_clients | 26752 | 26834 | 25749 | 21739 | 18071 | 9706
head_64_clients | 52303 | 49759 | 47785 | 36625 | 26993 | 13685
head_128_clients | 98325 | 89753 | 83276 | 62302 | 38515 | 16005
head_256_clients | 128075 | 124396 | 111059 | 97165 | 56941 | 15779
head_512_clients | 140908 | 132622 | 126363 | 119113 | 62572 | 15919
head_768_clients | 118694 | 111764 | 109464 | 120368 | 62129 | 15905
head_1024_clients | 102542 | 99007 | 94291 | 109485 | 62680 | 16039
head_2048_clients | 57994 | 57003 | 57410 | 60350 | 62487 | 16091
head_4096_clients | 33995 | 32944 | 34174 | 33483 | 61071 | 15655
v6_1_clients | 1743 | 1711 | 1722 | 1655 | 1588 | 1378
v6_2_clients | 1714 | 1830 | 1767 | 1667 | 1725 | 1518
v6_4_clients | 3638 | 3602 | 3594 | 3452 | 3216 | 2713
v6_8_clients | 7047 | 6671 | 7148 | 6342 | 5577 | 4573
v6_16_clients | 13885 | 13247 | 13951 | 13037 | 10570 | 6391
v6_32_clients | 27766 | 27230 | 27079 | 22911 | 17152 | 9700
v6_64_clients | 50748 | 51548 | 47852 | 36479 | 27232 | 13290
v6_128_clients | 97611 | 89554 | 85009 | 67349 | 37046 | 16005
v6_256_clients | 124475 | 128603 | 108888 | 95277 | 55021 | 15785
v6_512_clients | 181639 | 176544 | 152852 | 120914 | 62674 | 15921
v6_768_clients | 188600 | 180691 | 158997 | 128740 | 62402 | 15979
v6_1024_clients | 191845 | 180830 | 161597 | 143032 | 62426 | 15985
v6_2048_clients | 179227 | 168906 | 173510 | 149689 | 62721 | 16090
v6_4096_clients | 156613 | 152795 | 154231 | 134587 | 62245 | 15781
--enable-atomics=no --enable-spinlocks=no, -T60:
message_size_b | 16 | 64 | 256 | 1024 | 4096 | 16384
-------------------+-------+-------+-------+-------+-------+-------
head_1_clients | 1644 | 1768 | 1726 | 1698 | 1544 | 1344
head_2_clients | 1805 | 1829 | 1746 | 1869 | 1730 | 1565
head_4_clients | 3562 | 3606 | 3571 | 3656 | 3145 | 2704
head_8_clients | 6921 | 7051 | 6774 | 6676 | 5999 | 4425
head_16_clients | 13418 | 13998 | 13634 | 12640 | 9782 | 6440
head_32_clients | 21716 | 21690 | 21124 | 18977 | 14050 | 9168
head_64_clients | 27085 | 26498 | 26108 | 23048 | 17843 | 13278
head_128_clients | 26704 | 26373 | 25845 | 24056 | 19777 | 15922
head_256_clients | 24694 | 24586 | 24148 | 22525 | 23523 | 15852
head_512_clients | 21364 | 21143 | 20697 | 20334 | 21770 | 15870
head_768_clients | 16985 | 16618 | 16544 | 16511 | 17360 | 15945
head_1024_clients | 13133 | 13640 | 13521 | 13716 | 14202 | 16020
head_2048_clients | 8051 | 8140 | 7711 | 8673 | 9027 | 15091
head_4096_clients | 4692 | 4549 | 4924 | 4908 | 6853 | 14752
v6_1_clients | 1676 | 1722 | 1781 | 1681 | 1527 | 1394
v6_2_clients | 1868 | 1706 | 1868 | 1842 | 1762 | 1573
v6_4_clients | 3668 | 3591 | 3449 | 3556 | 3309 | 2707
v6_8_clients | 7279 | 6818 | 6842 | 6846 | 5888 | 4283
v6_16_clients | 13604 | 13364 | 14099 | 12851 | 9959 | 6271
v6_32_clients | 22899 | 22453 | 22488 | 20127 | 15970 | 8915
v6_64_clients | 33289 | 32943 | 32280 | 28683 | 22885 | 13215
v6_128_clients | 43614 | 42954 | 41336 | 36660 | 29107 | 15928
v6_256_clients | 46542 | 46593 | 45673 | 41064 | 38759 | 15850
v6_512_clients | 36303 | 35923 | 34640 | 32828 | 38359 | 15913
v6_768_clients | 29654 | 29822 | 29317 | 28703 | 34194 | 15903
v6_1024_clients | 25871 | 25219 | 25801 | 25099 | 29323 | 16015
v6_2048_clients | 16497 | 17041 | 16401 | 17128 | 19656 | 15962
v6_4096_clients | 10067 | 10873 | 10702 | 10540 | 12909 | 16041
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: WAL Insertion Lock Improvements
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 12:04 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-11 06:26 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-12 02:05 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
@ 2023-05-12 22:56 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 16+ messages in thread
From: Michael Paquier @ 2023-05-12 22:56 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, May 12, 2023 at 07:35:20AM +0530, Bharath Rupireddy wrote:
> --enable-atomics=no, -T60:
> --enable-spinlocks=no, -T60:
> --enable-atomics=no --enable-spinlocks=no, -T60:
Thanks for these extra tests, I have not done these specific cases but
the profiles look similar to what I've seen myself. If I recall
correctly the fallback implementation of atomics just uses spinlocks
internally to force the barriers required.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2023-05-12 22:56 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06 18:23 [PATCH] cfe-03-scripts_over_cfe-02-internaldoc squash commit Bruce Momjian <[email protected]>
2023-02-02 13:30 Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-08 22:06 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-02-09 06:21 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-02-21 05:49 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-04-10 04:08 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-08 12:27 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-08 23:04 ` Re: WAL Insertion Lock Improvements Nathan Bossart <[email protected]>
2023-05-09 03:32 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-09 03:54 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 17:10 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-10 23:31 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-10 12:04 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-11 06:26 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
2023-05-12 02:05 ` Re: WAL Insertion Lock Improvements Bharath Rupireddy <[email protected]>
2023-05-12 22:56 ` Re: WAL Insertion Lock Improvements Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox