From 16090c876d5f37aa7d35e6e5c0bed57bebcd2e0e Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 4 Jan 2024 12:47:42 +0000 Subject: [PATCH] and now it's obvious that new_heap_blkno and old_heap_blkno really are independent --- pageserver/src/walingest.rs | 49 +++++++++++++------------------------ pageserver/src/walrecord.rs | 9 ++++--- pageserver/src/walredo.rs | 33 ++++++++++++------------- 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 14070688d6..b91cb8620f 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -746,9 +746,6 @@ impl WalIngest { }; match (new, old) { - (None, None) => { - // Nothing to do - } (Some((new_heap_blkno, new_vm_blk)), Some((old_heap_blkno, old_vm_blk))) if new_vm_blk == old_vm_blk => { @@ -759,8 +756,8 @@ impl WalIngest { vm_rel, new_vm_blk, // could also be old_vm_blk, they're the same NeonWalRecord::ClearVisibilityMapFlags { - new_heap_blkno: Some(new_heap_blkno), - old_heap_blkno: Some(old_heap_blkno), + heap_blkno_1: Some(new_heap_blkno), + heap_blkno_2: Some(old_heap_blkno), flags, }, ctx, @@ -769,33 +766,21 @@ impl WalIngest { } (new, old) => { // Emit one record per VM block that needs updating. - if let Some((new_heap_blkno, new_vm_blk)) = new { - self.put_rel_wal_record( - modification, - vm_rel, - new_vm_blk, - NeonWalRecord::ClearVisibilityMapFlags { - new_heap_blkno: Some(new_heap_blkno), - old_heap_blkno: None, - flags, - }, - ctx, - ) - .await?; - } - if let Some(Some((old_heap_blkno, old_vm_blk))) = old { - self.put_rel_wal_record( - modification, - vm_rel, - old_vm_blk, - NeonWalRecord::ClearVisibilityMapFlags { - new_heap_blkno: None, - old_heap_blkno: Some(old_heap_blkno), - flags, - }, - ctx, - ) - .await?; + for update in [new, old] { + if let Some((heap_blkno, vm_blk)) = update { + self.put_rel_wal_record( + modification, + vm_rel, + vm_blk, + NeonWalRecord::ClearVisibilityMapFlags { + heap_blkno_1: Some(heap_blkno), + heap_blkno_2: None, + flags, + }, + ctx, + ) + .await?; + } } } } diff --git a/pageserver/src/walrecord.rs b/pageserver/src/walrecord.rs index ff6bc9194b..0bba6e2a51 100644 --- a/pageserver/src/walrecord.rs +++ b/pageserver/src/walrecord.rs @@ -21,10 +21,13 @@ pub enum NeonWalRecord { /// Native PostgreSQL WAL record Postgres { will_init: bool, rec: Bytes }, - /// Clear bits in heap visibility map. ('flags' is bitmap of bits to clear) + /// Clear the bits specified in `flags` in the heap visibility map for the given heap blocks. + /// + /// For example, for `{ heap_blkno_1: None, heap_blkno_2: Some(23), flags: 0b0010_0000}` + /// redo will apply `&=0b0010_0000` on heap block 23's visibility map bitmask. ClearVisibilityMapFlags { - new_heap_blkno: Option, - old_heap_blkno: Option, + heap_blkno_1: Option, + heap_blkno_2: Option, flags: u8, }, /// Mark transaction IDs as committed on a CLOG page diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index 3bec2846e9..984b73bc14 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -403,8 +403,8 @@ impl PostgresRedoManager { anyhow::bail!("tried to pass postgres wal record to neon WAL redo"); } NeonWalRecord::ClearVisibilityMapFlags { - new_heap_blkno, - old_heap_blkno, + heap_blkno_1, + heap_blkno_2, flags, } => { // sanity check that this is modifying the correct relation @@ -414,25 +414,22 @@ impl PostgresRedoManager { "ClearVisibilityMapFlags record on unexpected rel {}", rel ); - let mut doit = |heap_blkno| { - // Calculate the VM block and offset that corresponds to the heap block. - let map_block = pg_constants::HEAPBLK_TO_MAPBLOCK(heap_blkno); - let map_byte = pg_constants::HEAPBLK_TO_MAPBYTE(heap_blkno); - let map_offset = pg_constants::HEAPBLK_TO_OFFSET(heap_blkno); + for update in [heap_blkno_1, heap_blkno_2] { + if let Some(heap_blkno) = update { + let heap_blkno = *heap_blkno; + // Calculate the VM block and offset that corresponds to the heap block. + let map_block = pg_constants::HEAPBLK_TO_MAPBLOCK(heap_blkno); + let map_byte = pg_constants::HEAPBLK_TO_MAPBYTE(heap_blkno); + let map_offset = pg_constants::HEAPBLK_TO_OFFSET(heap_blkno); - // Check that we're modifying the correct VM block. - assert!(map_block == blknum); + // Check that we're modifying the correct VM block. + assert!(map_block == blknum); - // equivalent to PageGetContents(page) - let map = &mut page[pg_constants::MAXALIGN_SIZE_OF_PAGE_HEADER_DATA..]; + // equivalent to PageGetContents(page) + let map = &mut page[pg_constants::MAXALIGN_SIZE_OF_PAGE_HEADER_DATA..]; - map[map_byte as usize] &= !(flags << map_offset); - }; - if let Some(heap_blkno) = *new_heap_blkno { - doit(heap_blkno); - } - if let Some(heap_blkno) = *old_heap_blkno { - doit(heap_blkno); + map[map_byte as usize] &= !(flags << map_offset); + } } } // Non-relational WAL records are handled here, with custom code that has the