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