Re: Read-Write optimistic lock (Re: sinvaladt.c: remove msgnumLock, use atomic operations on maxMsgNum) - Mailing list pgsql-hackers

16.07.2025 17:58, Andres Freund пишет:
> Hi,
>
> On 2025-06-25 16:41:46 +0300, Sergey Shinderuk wrote:
>> On 16.06.2025 17:41, Andres Freund wrote:
>>> TBH, I don't see a point in continuing with this thread without something that
>>> others can test.  I rather doubt that the right fix here is to just change the
>>> lock model over, but without a repro I can't evaluate that.
>
>> I think I can reproduce the issue with pgbench on a muti-core server. I
>> start a regular select-only test with 64 clients, and while it's running, I
>> start a plpgsql loop creating and dropping temporary tables from a single
>> psql session. I observe ~25% drop in tps reported by pgbench until I cancel
>> the query in psql.
>
>>
>> $ pgbench -n -S -c64 -j64 -T300 -P1
>>
>> progress: 10.0 s, 1249724.7 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 11.0 s, 1248289.0 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 12.0 s, 1246001.0 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 13.0 s, 1247832.5 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 14.0 s, 1248205.8 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 15.0 s, 1247737.3 tps, lat 0.051 ms stddev 0.002, 0 failed
>> progress: 16.0 s, 1219444.3 tps, lat 0.052 ms stddev 0.039, 0 failed
>> progress: 17.0 s, 893943.4 tps, lat 0.071 ms stddev 0.159, 0 failed
>> progress: 18.0 s, 927861.3 tps, lat 0.069 ms stddev 0.150, 0 failed
>> progress: 19.0 s, 886317.1 tps, lat 0.072 ms stddev 0.163, 0 failed
>> progress: 20.0 s, 877200.1 tps, lat 0.073 ms stddev 0.164, 0 failed
>> progress: 21.0 s, 875424.4 tps, lat 0.073 ms stddev 0.163, 0 failed
>> progress: 22.0 s, 877693.0 tps, lat 0.073 ms stddev 0.165, 0 failed
>> progress: 23.0 s, 897202.8 tps, lat 0.071 ms stddev 0.158, 0 failed
>> progress: 24.0 s, 917853.4 tps, lat 0.070 ms stddev 0.153, 0 failed
>> progress: 25.0 s, 907865.1 tps, lat 0.070 ms stddev 0.154, 0 failed
>>
>> Here I started the following loop in psql around 17s and tps dropped by
>> ~25%:
>>
>> do $$
>> begin
>>   for i in 1..1000000 loop
>>     create temp table tt1 (a bigserial primary key, b text);
>>     drop table tt1;
>>     commit;
>>   end loop;
>> end;
>> $$;
>
> Not a particularly interesting use-case, but still good to have.
>
>
>
>> Now, if I simply remove the spinlock in SIGetDataEntries, I see a drop of
>> just ~6% under concurrent DDL. I think this strongly suggests that the
>> spinlock is the bottleneck.
>
> This can be trivially optimized by just using atomics in a slightly more
> heavyweight way than Yura's patch upthread - my criticisim in [1] was purely
> that the reduced barriers make the patch incorrect.  I don't understand why
> Yura didn't just move to full memory barriers, instead choosing to write a RW
> optimized spinlock.
>
> [1] https://postgr.es/m/5ccgykypol3azijw2chqnpg3rhuwjtwsmbfs3pgcqm7fu6laus%40wppo6zcfszay

Excuse me, but I did at [1]. Not all barriers were changed to full, but one
necessary at the place you'd pointed to.

But you just answered then [2]:
> I don't believe we have the whole story here.

[1]
https://www.postgresql.org/message-id/b798eb5e-35b7-40b5-a245-4170deab56f8%40postgrespro.ru
[2]
https://www.postgresql.org/message-id/4yfgfeu3f6f7fayl5h3kggtd5bkvm4gj3a3ryjs2qhlo6g74bt%40g3cu2u4pgaiw

I moved to RW optimized spinlock because I found second place where it
helps. And patch includes both those places.
I think it is better to have good reusable tool applied in two places
instead of two custom pieces of code. And RWOptSpin is almost as good as
direct use of atomic operations.

--
regards
Yura Sokolov aka funny-falcon