public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: WAL Insertion Lock Improvements
Date: Wed, 8 Feb 2023 14:06:03 -0800
Message-ID: <20230208220603.GA546776@nathanxps13> (raw)
In-Reply-To: <CALj2ACUt-YdzxdGZPcr0LcERf0Hv2sTbz4ZvUftQ8Dgk4aVRfA@mail.gmail.com>
References: <CALj2ACVF+6jLvqKe6xhDzCCkr=rfd6upaGc3477Pji1Ke9G7Bg@mail.gmail.com>
	<[email protected]>
	<CALj2ACWkWbheFhkPwMw83CUpzHFGXSV_HXTBxG9+-PZ3ufHE=Q@mail.gmail.com>
	<[email protected]>
	<CALj2ACXYpYDboe4i+yXDYoNQj9-G0584hP5x_b=W6hj693qcaA@mail.gmail.com>
	<20221203003158.GA2518778@nathanxps13>
	<[email protected]>
	<CALj2ACUc=Hcd3X+mBNAF2o9Y3r=cxVoz4JBayBTz=jw74KopHQ@mail.gmail.com>
	<CALj2ACUt-YdzxdGZPcr0LcERf0Hv2sTbz4ZvUftQ8Dgk4aVRfA@mail.gmail.com>

+	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






view thread (16+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: WAL Insertion Lock Improvements
  In-Reply-To: <20230208220603.GA546776@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox