Hi all,
Based on the ongoing discussions on Checkpointer Write Combining [1] and
the last patch v11 discussed in forum, I have re-implemented the logic for
CleanVictimBuffer() and BufferNeedsWALFlush() with a few changes:
static void
CleanVictimBuffer(BufferDesc *bufdesc, bool from_ring, IOContext io_context)
{
XLogRecPtr max_lsn = InvalidXLogRecPtr;
LWLock *content_lock;
content_lock = BufHdrGetContentLock(bufdesc);
LWLockAcquire(content_lock, LW_SHARED);
if (!PrepareFlushBuffer(bufdesc, &max_lsn))
{
LWLockRelease(content_lock);
return;
}
LWLockRelease(content_lock);
LWLockAcquire(content_lock, LW_EXCLUSIVE);
if (!PrepareFlushBuffer(bufdesc, &max_lsn))
{
LWLockRelease(content_lock);
return;
}
BufferSetStateWriteInProgress(bufdesc);
LWLockRelease(content_lock);
RecordBufferForBatch(bufdesc, max_lsn);
}
Here I added some extra safety checks and early-exit handling, making sure
the function returns cleanly when a buffer no longer needs writing and
ensuring locks are handled correctly.
static bool
BufferNeedsWALFlush(BufferDesc *bufdesc, XLogRecPtr *lsn)
{
uint32 buf_state;
buf_state = LockBufHdr(bufdesc);
if (buf_state & BM_PERMANENT)
*lsn = BufferGetLSN(bufdesc);
else
*lsn = InvalidXLogRecPtr;
UnlockBufHdr(bufdesc);
if (!(*lsn != InvalidXLogRecPtr))
return false;
return XLogNeedsFlush(*lsn);
}
Here I made some changes in the logic of the function that only returns a
real LSN when the buffer actually needs a WAL flush. If the buffer is not
permanent or doesn’t need WAL, it returns false and set the LSN to
InvalidXLogRecPtr, by which it can avoid confusions or unsafe downstream
behaviors.
I am now focusing on the PageSetBatchChecksumInplace() as mentioned in [1]
and the write combining logic. I will update my implementations soon once I
complete it.
I hope these corrected versions of functions will be helpful in further
implementation. Looking forward to more feedback.
Regards,
Soumya
Reference:
Discussion [1]
https://www.postgresql.org/message-id/CAAKRu_ZiEpE_EHww3S3-E3iznybdnX8mXSO7Wsuru7=P9Y=c...@mail.gmail.com
