From f56a9def71bb1d2ccf4fa01a4d1e082c1063d921 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 11 Apr 2024 09:20:51 +0900 Subject: [PATCH] Fix inconsistency with replay of hash squeeze record for clean buffers aa5edbe379d6 has tweaked _hash_freeovflpage() so as the write buffer's LSN is updated only when necessary, when REGBUF_NO_CHANGE is not used. The replay code was not consistent with that, causing the write buffer's LSN to be updated and its page to be marked as dirty even if the buffer was registered in a "clean" state. This was possible for the case of a squeeze record when there are no tuples to add to the write buffer, for (is_prim_bucket_same_wrt && !is_prev_bucket_same_wrt). I have performed some validation of this commit with wal_consistency_checking and a change in WAL that logs REGBUF_NO_CHANGE to a new BKPIMAGE_*. Thanks to that, it is possible to know at replay if a buffer was clean when it was registered, then cross-checked the LSN of the "clean" page copy coming from WAL with the LSN of the block once the record has been replayed. This eats one bit in bimg_info, which is not acceptable to be integrated as-is, but it could become handy in the future. I didn't spot other areas than the one fixed by this commit at the extent of what the main regression test suite covers. As this is an oversight in aa5edbe379d6, no backpatch is required. Reported-by: Zubeyr Eryilmaz Author: Hayato Kuroda Reviewed-by: Amit Kapila, Michael Paquier Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz --- src/backend/access/hash/hash_xlog.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c index cb1a63cfee..4e1ec46856 100644 --- a/src/backend/access/hash/hash_xlog.c +++ b/src/backend/access/hash/hash_xlog.c @@ -666,6 +666,7 @@ hash_xlog_squeeze_page(XLogReaderState *record) char *data; Size datalen; uint16 ninserted = 0; + bool mod_wbuf = false; data = begin = XLogRecGetBlockData(record, 1, &datalen); @@ -695,6 +696,17 @@ hash_xlog_squeeze_page(XLogReaderState *record) ninserted++; } + + mod_wbuf = true; + } + else + { + /* + * Ensure that the required flags are set when there are no + * tuples. See _hash_freeovflpage(). + */ + Assert(xldata->is_prim_bucket_same_wrt || + xldata->is_prev_bucket_same_wrt); } /* @@ -711,10 +723,15 @@ hash_xlog_squeeze_page(XLogReaderState *record) HashPageOpaque writeopaque = HashPageGetOpaque(writepage); writeopaque->hasho_nextblkno = xldata->nextblkno; + mod_wbuf = true; } - PageSetLSN(writepage, lsn); - MarkBufferDirty(writebuf); + /* Set LSN and mark writebuf dirty iff it is modified */ + if (mod_wbuf) + { + PageSetLSN(writepage, lsn); + MarkBufferDirty(writebuf); + } } /* replay the record for initializing overflow buffer */