Re: Get rid of WALBufMappingLock

Hi!

On Sun, Jan 19, 2025 at 2:11 AM Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> wrote:
>
> During discussion of Increasing NUM_XLOGINSERT_LOCKS [1], Andres Freund
> used benchmark which creates WAL records very intensively. While I this
> it is not completely fair (1MB log records are really rare), it pushed
> me to analyze write-side waiting of XLog machinery.
>
> First I tried to optimize WaitXLogInsertionsToFinish, but without great
> success (yet).
>
> While profiling, I found a lot of time is spend in the memory clearing
> under global WALBufMappingLock:
>
> MemSet((char *) NewPage, 0, XLOG_BLCKSZ);
>
> It is obvious scalability bottleneck.
>
> So "challenge was accepted".
>
> Certainly, backend should initialize pages without exclusive lock. But
> which way to ensure pages were initialized? In other words, how to
> ensure XLogCtl->InitializedUpTo is correct.
>
> I've tried to play around WALBufMappingLock with holding it for a short
> time and spinning on XLogCtl->xlblocks[nextidx]. But in the end I found
> WALBufMappingLock is useless at all.
>
> Instead of holding lock, it is better to allow backends to cooperate:
> - I bound ConditionVariable to each xlblocks entry,
> - every backend now checks every required block pointed by
> InitializedUpto was successfully initialized or sleeps on its condvar,
> - when backend sure block is initialized, it tries to update
> InitializedUpTo with conditional variable.

Looks reasonable for me, but having ConditionVariable per xlog buffer
seems overkill for me. Find an attached revision, where I've
implemented advancing InitializedUpTo without ConditionVariable.
After initialization of each buffer there is attempt to do CAS for
InitializedUpTo in a loop. So, multiple processes will try to advance
InitializedUpTo, they could hijack initiative from each other, but
there is always a leader which will finish the work.

There is only one ConditionVariable to wait for InitializedUpTo being advanced.

I didn't benchmark my version, just checked that tests passed.

------
Regards,
Alexander Korotkov
Supabase