Hi,
On 2022-11-23 19:12:07 +0530, Bharath Rupireddy wrote:
> While working on something else, I noticed that each WAL insert lock
> tracks its own last important WAL record's LSN (lastImportantAt) and
> both the bgwriter and checkpointer later computes the max
> value/server-wide last important WAL record's LSN via
> GetLastImportantRecPtr(). While doing so, each WAL insertion lock is
> acquired in exclusive mode in a for loop. This seems like too much
> overhead to me.
GetLastImportantRecPtr() should be a very rare operation, so it's fine for it
to be expensive. The important thing is for the maintenance of the underlying
data to be very cheap.
> I quickly coded a patch (attached herewith) that
> tracks the server-wide last important WAL record's LSN in
> XLogCtlInsert (lastImportantPos) protected with a spinlock and gets
> rid of lastImportantAt from each WAL insert lock.
That strikes me as a very bad idea. It adds another point of contention to a
very very hot code path, to make a very rare code path cheaper.
Greetings,
Andres Freund