From 782575067036e710009cf6a5d00a3fe8ac291a90 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 20 Feb 2024 05:52:29 +0000 Subject: [PATCH v25 1/4] Add check in WALReadFromBuffers against requested WAL --- src/backend/access/transam/xlog.c | 26 ++++++++++++++++++------- src/backend/access/transam/xlogreader.c | 3 +++ src/include/access/xlog.h | 1 + 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 50c347a679..b01a3b4ed1 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -698,7 +698,6 @@ static void ReserveXLogInsertLocation(int size, XLogRecPtr *StartPos, XLogRecPtr *EndPos, XLogRecPtr *PrevPtr); static bool ReserveXLogSwitch(XLogRecPtr *StartPos, XLogRecPtr *EndPos, XLogRecPtr *PrevPtr); -static XLogRecPtr WaitXLogInsertionsToFinish(XLogRecPtr upto); static char *GetXLogBuffer(XLogRecPtr ptr, TimeLineID tli); static XLogRecPtr XLogBytePosToRecPtr(uint64 bytepos); static XLogRecPtr XLogBytePosToEndRecPtr(uint64 bytepos); @@ -1493,7 +1492,7 @@ WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt) * uninitialized page), and the inserter might need to evict an old WAL buffer * to make room for a new one, which in turn requires WALWriteLock. */ -static XLogRecPtr +XLogRecPtr WaitXLogInsertionsToFinish(XLogRecPtr upto) { uint64 bytepos; @@ -1710,13 +1709,14 @@ GetXLogBuffer(XLogRecPtr ptr, TimeLineID tli) * of bytes read successfully. * * Fewer than 'count' bytes may be read if some of the requested WAL data has - * already been evicted. + * already been evicted from the WAL buffers. * * No locks are taken. * - * Caller should ensure that it reads no further than LogwrtResult.Write - * (which should have been updated by the caller when determining how far to - * read). The 'tli' argument is only used as a convenient safety check so that + * Caller should ensure that it reads no further than current insert position + * with the help of WaitXLogInsertionsToFinish(). + * + * The 'tli' argument is only used as a convenient safety check so that * callers do not read from WAL buffers on a historical timeline. */ Size @@ -1731,7 +1731,19 @@ WALReadFromBuffers(char *dstbuf, XLogRecPtr startptr, Size count, return 0; Assert(!XLogRecPtrIsInvalid(startptr)); - Assert(startptr + count <= LogwrtResult.Write); + +#ifdef USE_ASSERT_CHECKING + { + XLogRecPtr upto = startptr + count; + XLogRecPtr insert_pos = GetXLogInsertRecPtr(); + + if (upto > insert_pos) + ereport(ERROR, + (errmsg("cannot read past end of current insert position; request %X/%X, insert position %X/%X", + LSN_FORMAT_ARGS(upto), + LSN_FORMAT_ARGS(insert_pos)))); + } +#endif /* * Loop through the buffers without a lock. For each buffer, atomically diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 74a6b11866..ae9904e7e4 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1500,6 +1500,9 @@ err: * * Returns true if succeeded, false if an error occurs, in which case * 'errinfo' receives error details. + * + * Note: It is the caller's responsibility to ensure requested WAL is written + * to disk, that is 'startptr'+'count' > LogwrtResult.Write. */ bool WALRead(XLogReaderState *state, diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 76787a8267..74606a6846 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -252,6 +252,7 @@ extern XLogRecPtr GetLastImportantRecPtr(void); extern void SetWalWriterSleeping(bool sleeping); +extern XLogRecPtr WaitXLogInsertionsToFinish(XLogRecPtr upto); extern Size WALReadFromBuffers(char *dstbuf, XLogRecPtr startptr, Size count, TimeLineID tli); -- 2.34.1