Re: Latches vs lwlock contention

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Latches vs lwlock contention
Date: 2023-09-28 09:58:12
Message-ID: 53c844e1-a21a-e481-cca5-2184c8aa47b9@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 28/10/2022 06:56, Thomas Munro wrote:
> One example is heavyweight lock wakeups. If you run BEGIN; LOCK TABLE
> t; ... and then N other sessions wait in SELECT * FROM t;, and then
> you run ... COMMIT;, you'll see the first session wake all the others
> while it still holds the partition lock itself. They'll all wake up
> and begin to re-acquire the same partition lock in exclusive mode,
> immediately go back to sleep on*that* wait list, and then wake each
> other up one at a time in a chain. We could avoid the first
> double-bounce by not setting the latches until after we've released
> the partition lock. We could avoid the rest of them by not
> re-acquiring the partition lock at all, which ... if I'm reading right
> ... shouldn't actually be necessary in modern PostgreSQL? Or if there
> is another reason to re-acquire then maybe the comment should be
> updated.

ISTM that the change to not re-aqcuire the lock in ProcSleep is
independent from the other changes. Let's split that off to a separate
patch.

I agree it should be safe. Acquiring a lock just to hold off interrupts
is overkill anwyway, HOLD_INTERRUPTS() would be enough.
LockErrorCleanup() uses HOLD_INTERRUPTS() already.

There are no CHECK_FOR_INTERRUPTS() in GrantAwaitedLock(), so cancel/die
interrupts can't happen here. But could we add HOLD_INTERRUPTS(), just
pro forma, to document the assumption? It's a little awkward: you really
should hold interrupts until the caller has done "awaitedLock = NULL;".
So it's not quite enough to add a pair of HOLD_ and RESUME_INTERRUPTS()
at the end of ProcSleep(). You'd need to do the HOLD_INTERRUPTS() in
ProcSleep() and require the caller to do RESUME_INTERRUPTS(). In a
sense, ProcSleep downgrades the lock on the partition to just holding
off interrupts.

Overall +1 on this change to not re-acquire the partition lock.

--
Heikki Linnakangas
Neon (https://neon.tech)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jakub Wartak 2023-09-28 10:22:07 Re: trying again to get incremental backup
Previous Message Jim Jones 2023-09-28 09:55:58 Re: [PATCH] Add inline comments to the pg_hba_file_rules view