Re: Get rid of WALBufMappingLock
Yura Sokolov <y.sokolov@postgrespro.ru>
From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org"
<pgsql-hackers@lists.postgresql.org>, Andres Freund <andres@anarazel.de>,
"Zhou, Zhiguo" <zhiguo.zhou@intel.com>
Date: 2025-02-12T18:16:12Z
Lists: pgsql-hackers
Attachments
- v4-0001-Get-rid-of-WALBufMappingLock.patch (text/x-patch) patch v4-0001
- v4-0002-several-attempts-to-lock-WALInsertLocks.patch (text/x-patch) patch v4-0002
08.02.2025 13:07, Alexander Korotkov пишет: > On Fri, Feb 7, 2025 at 1:39 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: >> Good, thank you. I think 0001 patch is generally good, but needs some >> further polishing, e.g. more comments explaining how does it work. I tried to add more comments. I'm not good at, so recommendations are welcome. > Two things comes to my mind worth rechecking about 0001. > 1) Are XLogCtl->InitializeReserved, XLogCtl->InitializedUpTo and > XLogCtl->xlblocks always page-aligned? Because algorithm seems to be > sensitive to that. If so, I would propose to explicitly comment that > and add corresponding asserts. They're certainly page aligned, since they are page borders. I added assert on alignment of InitializeReserved for the sanity. > 2) Check if there are concurrency issues between > AdvanceXLInsertBuffer() and switching to the new WAL file. There are no issues: 1. CopyXLogRecordToWAL for isLogSwitch follows same protocol, ie uses GetXLogBuffer to zero-out WAL page. 2. WALINSERT_SPECIAL_SWITCH forces exclusive lock on all insertion locks, so switching wal is not concurrent. (Although, there is no need in this exclusiveness, imho.) > Regarding 0002 patch, it looks generally reasonable. But are 2 > attempts always optimal? Are there cases of regression, or cases when > more attempts are even better? Could we have there some > self-adjusting mechanism like what we have for spinlocks? Well, I chose to perform 3 probes (2 conditional attempts + 1 unconditional) based on intuition. I have some experience in building hash tables, and cuckoo-hashing theory tells 2 probes is usually enough to reach 50% fill-rate, and 3 probes enough for ~75% fill rate. Since each probe is cache miss, it is hardly sensible to do more probes. 3 probes did better than 2 in other benchmark [1], although there were NUM_XLOGINSERT_LOCK increased. Excuse me for not bencmarking different choices here. I'll try to do measurements in next days. [1] https://postgr.es/m/3b11fdc2-9793-403d-b3d4-67ff9a00d447%40postgrespro.ru ------- regards Yura Sokolov aka funny-falcon