From 8ac523d2ee8e29ecc6891f6bd661fcf51b2147ba Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Fri, 1 Nov 2024 20:31:29 +0200 Subject: [PATCH] Do not assign page LSN to new (uninitialized) page in ClearVisibilityMapFlags redo handler (#9287) ## Problem https://neondb.slack.com/archives/C04DGM6SMTM/p1727872045252899 See https://github.com/neondatabase/neon/issues/9240 ## Summary of changes Add `!page_is_new` check before assigning page lsn. ## Checklist before requesting a review - [ ] I have performed a self-review of my code. - [ ] If it is a core feature, I have added thorough tests. - [ ] Do we need to implement analytics? if so did you add the relevant metrics to the dashboard? - [ ] If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section. ## Checklist before merging - [ ] Do not forget to reformat commit message to not include the above checklist --------- Co-authored-by: Konstantin Knizhnik --- pageserver/src/walredo/apply_neon.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pageserver/src/walredo/apply_neon.rs b/pageserver/src/walredo/apply_neon.rs index 7aaa357318..d712d8bf5e 100644 --- a/pageserver/src/walredo/apply_neon.rs +++ b/pageserver/src/walredo/apply_neon.rs @@ -67,7 +67,10 @@ pub(crate) fn apply_in_neon( let map = &mut page[pg_constants::MAXALIGN_SIZE_OF_PAGE_HEADER_DATA..]; map[map_byte as usize] &= !(flags << map_offset); - postgres_ffi::page_set_lsn(page, lsn); + // The page should never be empty, but we're checking it anyway as a precaution, so that if it is empty for some reason anyway, we don't make matters worse by setting the LSN on it. + if !postgres_ffi::page_is_new(page) { + postgres_ffi::page_set_lsn(page, lsn); + } } // Repeat for 'old_heap_blkno', if any @@ -81,7 +84,10 @@ pub(crate) fn apply_in_neon( let map = &mut page[pg_constants::MAXALIGN_SIZE_OF_PAGE_HEADER_DATA..]; map[map_byte as usize] &= !(flags << map_offset); - postgres_ffi::page_set_lsn(page, lsn); + // The page should never be empty, but we're checking it anyway as a precaution, so that if it is empty for some reason anyway, we don't make matters worse by setting the LSN on it. + if !postgres_ffi::page_is_new(page) { + postgres_ffi::page_set_lsn(page, lsn); + } } } // Non-relational WAL records are handled here, with custom code that has the