Request a Demo
Home   >   mailing lists

Re: Throwing away unnecessary spin-locks - Mailing list pgsql-hackers

From Merlin Moncure
Subject Re: Throwing away unnecessary spin-locks
Date
Msg-id CAHyXU0yjziQUN1FJTtxQf3AD6hkUG2W8OS0Omm4uE=uBzfYs4Q@mail.gmail.com
Whole thread
In response to Re: Throwing away unnecessary spin-locks  (Andres Freund <andres@anarazel.de>)
Responses Re: Throwing away unnecessary spin-locks
List pgsql-hackers
Tree view
On Fri, Oct 2, 2026 at 7:20 AM Andres Freund <andres@anarazel.de> wrote:

Hi,

I'm sorry to be blunt, but this dangerous stuff.  You're just doing mechanical
replacements, without any analysis. That's a recipe for hard to encounter and
find bugs.

It also doesn't make any sort of sense to me to use slock_* or whatnot. This
has nothing to do with spinlocks.


> @@ -2748,9 +2746,7 @@ XLogSetAsyncXactLSN(XLogRecPtr asyncXactLSN)
>  void
>  XLogSetReplicationSlotMinimumLSN(XLogRecPtr lsn)
>  {
> -     SpinLockAcquire(&XLogCtl->info_lck);
> -     XLogCtl->replicationSlotMinLSN = lsn;
> -     SpinLockRelease(&XLogCtl->info_lck);
> +     slock_write_uint64(&XLogCtl->info_lck, &XLogCtl->replicationSlotMinLSN, lsn);
>  }

Just about all the write side ones are completely wrong.  This allows the
field to be set while someone else holds the spinlock.  As I said before, this
needs careful, documented, analysis FOR EACH AND EVERY SINGLE CHANGE.

Here is one that looks really broken:
- SpinLockAcquire(&Insert->insertpos_lck);
- current_bytepos = Insert->CurrBytePos;
- SpinLockRelease(&Insert->insertpos_lck);
+ current_bytepos = slock_read_uint64(&Insert->insertpos_lck, &Insert->CurrBytePos);

The insert itself is guarded via:
        SpinLockAcquire(&Insert->insertpos_lck);

        startbytepos = Insert->C
        startbytepos = Insert->CurrBytePos;
        endbytepos = startbytepos + size;
        prevbytepos = Insert->PrevBytePos;
        Insert->CurrBytePos = endbytepos;
        Insert->PrevBytePos = startbytepos;
urrBytePos;
        endbytepos = startbytepos + size;
        prevbytepos = Insert->PrevBytePos;
        Insert->CurrBytePos = endbytepos;
        Insert->PrevBytePos = startbytepos;

        SpinLockRelease(&Insert->insertpos_lck);

You can't sneak under the lock in this case.   The insert lock seems very dangerous in general.  You might have better luck with the info_lock.  Have you measured contention?

merlin
 

pgsql-hackers by date:

Previous
From: Manu
Date:
Subject: Re: BUG #19686: Rolling back SET TABLESPACE
Next
From: David Christensen
Date:
Subject: Re: Fix GROUP BY ALL handling of ORDER BY operator semantics