On Thu, 2024-02-22 at 10:17 +0530, Robert Haas wrote: > I think the problems tend to be worst when you have some bit of data > that's being frequently modified by multiple backends. Every backend > that wants to modify the value needs to steal the cache line, and > eventually you spend most of your CPU time stealing cache lines from > other sockets and not much of it doing any actual work. If you have a > value that's just being read by a lot of backends without > modification, I think the cache line can be shared in read only mode > by all the CPUs and it's not too bad.
That makes sense. I guess they'd be on the same cache line as well, which means a write to either will invalidate both. Some places (XLogWrite, XLogInsertRecord, XLogSetAsyncXactLSN, GetFlushRecPtr, GetXLogWriteRecPtr) already update it from shared memory anyway, so those are non-issues. The potential problem areas (unless I missed something) are: * AdvanceXLInsertBuffer reads it as an early check to see if a buffer is already written out. * XLogFlush / XLogNeedsFlush use it for an early return * WALReadFromBuffers reads it for an error check (currently an Assert, but we might want to make that an elog). * We are discussing adding a Copy pointer, which would be advanced by WaitXLogInsertionsToFinish(), and if we do replication-before-flush, we may want to be more eager about advancing it. That could cause an issue, as well. I don't see the global non-shared variable as a huge problem, so if it serves a purpose then I'm fine keeping it. Perhaps we could make it a bit safer by using some wrapper functions. Something like: bool IsWriteRecPtrAtLeast(XLogRecPtr recptr) { XLogRecPtr writeptr; if (LogwrtResult.Write >= recptr) return true; writeptr = GetXLogWriteRecPtr(); return (writeptr >= recptr); } That would reduce the number of direct references to LogwrtResult, callers would never see a stale value, and would avoid the cache miss problem that you're concerned about. Not sure if they'd need to be inline or not. Regards, Jeff Davis