Re: [RFC] Lock-free XLog Reservation from WAL

22.01.2025 15:37, Japin Li пишет:
> On Wed, 22 Jan 2025 at 16:49, Japin Li <japinli(at)hotmail(dot)com> wrote:
>> On Wed, 22 Jan 2025 at 11:22, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> wrote:
>>> 22.01.2025 10:54, Japin Li пишет:
>>>> On Wed, 22 Jan 2025 at 10:25, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> wrote:
>>>>> 22.01.2025 09:09, Japin Li пишет:
>>>>>> Hi, Yura Sokolov
>>>>>> Thanks for updating the patch.
>>>>>> I test the v2 patch using BenchmarkSQL 1000 warehouse, and here is the tpmC
>>>>>> result:
>>>>>> case | min | avg | max
>>>>>> --------------------+------------+------------+--------------
>>>>>> master (patched) | 988,461.89 | 994,916.50 | 1,000,362.40
>>>>>> master (44b61efb79) | 857,028.07 | 863,174.59 | 873,856.92
>>>>>> The patch provides a significant improvement.
>>>>>> I just looked through the patch, here are some comments.
>>>>>> 1.
>>>>>> The v2 patch can't be applied cleanly.
>>>>>> Applying: Lock-free XLog Reservation using lock-free hash-table
>>>>>> .git/rebase-apply/patch:33: trailing whitespace.
>>>>>> .git/rebase-apply/patch:37: space before tab in indent.
>>>>>> {
>>>>>> .git/rebase-apply/patch:38: space before tab in indent.
>>>>>> int i;
>>>>>> .git/rebase-apply/patch:39: trailing whitespace.
>>>>>> .git/rebase-apply/patch:46: space before tab in indent.
>>>>>> LWLockReleaseClearVar(&WALInsertLocks[i].l.lock,
>>>>>> warning: squelched 4 whitespace errors
>>>>>> warning: 9 lines add whitespace errors.
>>>>>> 2.
>>>>>> And there is a typo:
>>>>>> + * PrevLinksHash is a lock-free hash table based on Cuckoo
>>>>>> algorith. It is
>>>>>> + * mostly 4 way: for every element computed two positions h1, h2, and
>>>>>> s/algorith/algorithm/g
>>>>>
>>>>> Hi, Japin
>>>>>
>>>>> Thank you a lot for measuring and comments.
>>>>>
>>>>> May I ask you to compare not only against master, but against straight
>>>>> increase of NUM_XLOGINSERT_LOCKS to 128 as well?
>>>>> This way the profit from added complexity will be more obvious: does
>>>>> it pay for self or not.
>>>> The above test already increases NUM_XLOGINSERT_LOCKS to 64;
>>>
>>> Ok, that is good.
>>> Did you just increased number of locks, or applied
>>> "several-attempts-to-lock"
>>> from [1] as well? It will be interesting how it affects performance in this
>>> case. And it is orthogonal to "lock-free reservation", so they could
>>> applied simultaneously.
>>
>> I apply the following two patches:
>>
>> 1. Lock-free XLog Reservation using lock-free hash-table
>
> Hi, Yura Sokolov
>
> When I try to test the performance by only applying the Lock-free XLog
> Reservation patch, there is an error:
>
> 2025-01-22 20:06:49.976 CST [1271602] PANIC: stuck spinlock detected at LinkAndFindPrevPos, /home/postgres/postgres/build/../src/backend/access/transam/xlog.c:1425
> 2025-01-22 20:06:49.976 CST [1271602] STATEMENT: UPDATE bmsql_customer SET c_balance = c_balance - $1, c_ytd_payment = c_ytd_payment + $2, c_payment_cnt = c_payment_cnt + 1 WHERE c_w_id = $3 AND c_d_id = $4 AND c_id = $5
> 2025-01-22 20:06:50.078 CST [1271748] PANIC: stuck spinlock detected at LinkAndFindPrevPos, /home/postgres/postgres/build/../src/backend/access/transam/xlog.c:1425
>
> However, it does not always occur.

Oh, thank you!

I believe, I know why it happens: I was in hurry making v2 by
cherry-picking internal version. I reverted some changes in
CalcCuckooPositions manually and forgot to add modulo PREV_LINKS_HASH_CAPA.

Here's the fix:

pos->pos[0] = hash % PREV_LINKS_HASH_CAPA;
- pos->pos[1] = pos->pos[0] + 1;
+ pos->pos[1] = (pos->pos[0] + 1) % PREV_LINKS_HASH_CAPA;
pos->pos[2] = (hash >> 16) % PREV_LINKS_HASH_CAPA;
- pos->pos[3] = pos->pos[2] + 2;
+ pos->pos[3] = (pos->pos[2] + 2) % PREV_LINKS_HASH_CAPA;

Any way, here's v3:
- excess file "v0-0001-Increase..." removed. I believe it was source of
white-space apply warnings.
- this mistake fixed
- more clear slots strategies + "8 positions in two cache-lines" strategy.

You may play with switching PREV_LINKS_HASH_STRATEGY to 2 or 3 and see
if it affects measurably.

-------
regards
Yura Sokolov aka funny-falcon