Re: Avoid LWLockWaitForVar() for currently held WAL insertion lock in WaitXLogInsertionsToFinish()

On Fri, Nov 25, 2022 at 12:16 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2022-11-24 18:13:10 +0530, Bharath Rupireddy wrote:
> > With that said, here's a small improvement I can think of, that is, to
> > avoid calling LWLockWaitForVar() for the WAL insertion lock the caller
> > of WaitXLogInsertionsToFinish() currently holds. Since
> > LWLockWaitForVar() does a bunch of things - holds interrupts, does
> > atomic reads, acquires and releases wait list lock and so on, avoiding
> > it may be a good idea IMO.
>
> That doesn't seem like a big win. We're still going to call LWLockWaitForVar()
> for all the other locks.
>
> I think we could improve this code more significantly by avoiding the call to
> LWLockWaitForVar() for all locks that aren't acquired or don't have a
> conflicting insertingAt, that'd require just a bit more work to handle systems
> without tear-free 64bit writes/reads.
>
> The easiest way would probably be to just make insertingAt a 64bit atomic,
> that transparently does the required work to make even non-atomic read/writes
> tear free. Then we could trivially avoid the spinlock in
> LWLockConflictsWithVar(), LWLockReleaseClearVar() and with just a bit more
> work add a fastpath to LWLockUpdateVar(). We don't need to acquire the wait
> list lock if there aren't any waiters.
>
> I'd bet that start to have visible effects in a workload with many small
> records.

Thanks Andres! I quickly came up with the attached patch. I also ran
an insert test [1], below are the results. I also attached the results
graph. The cirrus-ci is happy with the patch -
https://github.com/BRupireddy/postgres/tree/wal_insertion_lock_improvements_v1_2.
Please let me know if the direction of the patch seems right.
clients HEAD PATCHED
1 1354 1499
2 1451 1464
4 3069 3073
8 5712 5797
16 11331 11157
32 22020 22074
64 41742 42213
128 71300 76638
256 103652 118944
512 111250 161582
768 99544 161987
1024 96743 164161
2048 72711 156686
4096 54158 135713

[1]
./configure --prefix=$PWD/inst/ CFLAGS="-O3" > install.log && make -j
8 install > install.log 2>&1 &
cd inst/bin
./pg_ctl -D data -l logfile stop
rm -rf data logfile
free -m
sudo su -c 'sync; echo 3 > /proc/sys/vm/drop_caches'
free -m
./initdb -D data
./pg_ctl -D data -l logfile start
./psql -d postgres -c 'ALTER SYSTEM SET shared_buffers = "8GB";'
./psql -d postgres -c 'ALTER SYSTEM SET max_wal_size = "16GB";'
./psql -d postgres -c 'ALTER SYSTEM SET max_connections = "4096";'
./pg_ctl -D data -l logfile restart
./pgbench -i -s 1 -d postgres
./psql -d postgres -c "ALTER TABLE pgbench_accounts DROP CONSTRAINT
pgbench_accounts_pkey;"
cat << EOF >> insert.sql
\set aid random(1, 10 * :scale)
\set delta random(1, 100000 * :scale)
INSERT INTO pgbench_accounts (aid, bid, abalance) VALUES (:aid, :aid, :delta);
EOF
ulimit -S -n 5000
for c in 1 2 4 8 16 32 64 128 256 512 768 1024 2048 4096; do echo -n
"$c ";./pgbench -n -M prepared -U ubuntu postgres -f insert.sql -c$c
-j$c -T5 2>&1|grep '^tps'|awk '{print $3}';done

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com