On Wed, Nov 13, 2019 at 09:17:03PM +0900, Michael Paquier wrote: > Actually, no, this is not good. I have been studying more the patch, > and after stressing more this code path with a cluster having > checksums enabled and shared_buffers at 1MB, I have been able to make > a couple of page's LSNs go backwards with pgbench -s 100. The cause > was simply that the page got flushed with a newer LSN than what was > returned by XLogSaveBufferForHint() before taking the buffer header > lock, so updating only the LSN for a non-dirty page was simply > guarding against that.
for the reference attached is the trick I have used, adding an extra assertion check in PageSetLSN(). -- Michael
diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h
index 4ef6d8ddd4..3a7c4051c0 100644
--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -366,7 +366,10 @@ PageValidateSpecialPointer(Page page)
#define PageGetLSN(page) \
PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn)
#define PageSetLSN(page, lsn) \
- PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn)
+ do { \
+ Assert(PageGetLSN(page) <= lsn); \
+ PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn); \
+ } while(0)
#define PageHasFreeLinePointers(page) \
(((PageHeader) (page))->pd_flags & PD_HAS_FREE_LINES)
signature.asc
Description: PGP signature
