Re: scalability bottlenecks with (many) partitions (and more)

Tomas Vondra <tomas@vondra.me>

From: Tomas Vondra <tomas@vondra.me>
To: Andres Freund <andres@anarazel.de>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-03-04T13:05:22Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Make FP_LOCK_SLOTS_PER_BACKEND look like a function

  2. Fix asserts in fast-path locking code

  3. Increase the number of fast-path lock slots

Attachments


On 3/3/25 21:52, Andres Freund wrote:
> Hi,
> 
> On 2025-03-03 21:31:42 +0100, Tomas Vondra wrote:
>> On 3/3/25 19:10, Andres Freund wrote:
>>> On 2024-09-21 20:33:49 +0200, Tomas Vondra wrote:
>>>> I've finally pushed this, after many rounds of careful testing to ensure
>>>> no regressions, and polishing.
>>>
>>> One minor nit: I don't like that FP_LOCK_SLOTS_PER_BACKEND is now non-constant
>>> while looking like a constant:
>>>
>>> #define		FP_LOCK_SLOTS_PER_BACKEND	(FP_LOCK_SLOTS_PER_GROUP * FastPathLockGroupsPerBackend)
>>>
>>> I don't think it's a good idea to have non-function-like #defines that
>>> reference variables that can change from run to run.
>>>
>>
>> Fair point, although it can't change "run to run" - not without a
>> restart.
> 
> That's what I meant with "run to run".
> 

OK.

> 
>> It's not a proper constant, of course, but it seemed close
>> enough. Yes, it might confuse people into thinking it's a constant, or
>> is there some additional impact?
> 
> That seems plenty. I just looked at the shem sizing function and was confused
> because I didn't see where the max_locks_per_transaction affects the
> allocation size.
> 

But the shmem sizing doesn't use FP_LOCK_SLOTS_PER_BACKEND at all, both
proc.c and postinit.c use the "full" formula, not the macro

  FastPathLockGroupsPerBackend * FP_LOCK_SLOTS_PER_GROUP

so why would the macro make this bit less obvious?

> 
>> The one fix I can think of is making it look more like a function,
>> possibly just like this:
>>
>> #define	FastPathLockSlotsPerBackend() \
>>   (FP_LOCK_SLOTS_PER_GROUP * FastPathLockGroupsPerBackend)
>>
>> Or do you have another suggestion?
> 
> That'd work for me.
> 

Attached is a patch doing this, but considering it has nothing to do
with the shmem sizing, I wonder if it's worth it.

regards

-- 
Tomas Vondra