diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 9192af0ee835..d5ef1f2a24b1 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -470,14 +470,14 @@ impl<'a> WalIngest<'a> { // we can't validate the remaining number of bytes without parsing // the tuple data. if (xlrec.flags & pg_constants::XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) != 0 { - old_heap_blkno = Some(decoded.blocks[0].blkno); + old_heap_blkno = Some(decoded.blocks.last().unwrap().blkno); } if (xlrec.flags & pg_constants::XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED) != 0 { // PostgreSQL only uses XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED on a // non-HOT update where the new tuple goes to different page than // the old one. Otherwise, only XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED is // set. - new_heap_blkno = Some(decoded.blocks[1].blkno); + new_heap_blkno = Some(decoded.blocks[0].blkno); } } } else if decoded.xl_rmid == pg_constants::RM_HEAP2_ID { @@ -526,14 +526,14 @@ impl<'a> WalIngest<'a> { // we can't validate the remaining number of bytes without parsing // the tuple data. if (xlrec.flags & pg_constants::XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) != 0 { - old_heap_blkno = Some(decoded.blocks[0].blkno); + old_heap_blkno = Some(decoded.blocks.last().unwrap().blkno); } if (xlrec.flags & pg_constants::XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED) != 0 { // PostgreSQL only uses XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED on a // non-HOT update where the new tuple goes to different page than // the old one. Otherwise, only XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED is // set. - new_heap_blkno = Some(decoded.blocks[1].blkno); + new_heap_blkno = Some(decoded.blocks[0].blkno); } } } else if decoded.xl_rmid == pg_constants::RM_HEAP2_ID { @@ -582,14 +582,14 @@ impl<'a> WalIngest<'a> { // we can't validate the remaining number of bytes without parsing // the tuple data. if (xlrec.flags & pg_constants::XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) != 0 { - old_heap_blkno = Some(decoded.blocks[0].blkno); + old_heap_blkno = Some(decoded.blocks.last().unwrap().blkno); } if (xlrec.flags & pg_constants::XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED) != 0 { // PostgreSQL only uses XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED on a // non-HOT update where the new tuple goes to different page than // the old one. Otherwise, only XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED is // set. - new_heap_blkno = Some(decoded.blocks[1].blkno); + new_heap_blkno = Some(decoded.blocks[0].blkno); } } } else if decoded.xl_rmid == pg_constants::RM_HEAP2_ID { @@ -745,14 +745,14 @@ impl<'a> WalIngest<'a> { // we can't validate the remaining number of bytes without parsing // the tuple data. if (xlrec.flags & pg_constants::XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) != 0 { - old_heap_blkno = Some(decoded.blocks[0].blkno); + old_heap_blkno = Some(decoded.blocks.last().unwrap().blkno); } if (xlrec.flags & pg_constants::XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED) != 0 { // PostgreSQL only uses XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED on a // non-HOT update where the new tuple goes to different page than // the old one. Otherwise, only XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED is // set. - new_heap_blkno = Some(decoded.blocks[1].blkno); + new_heap_blkno = Some(decoded.blocks[0].blkno); } } pg_constants::XLOG_NEON_HEAP_MULTI_INSERT => { diff --git a/test_runner/regress/test_vm_bits.py b/test_runner/regress/test_vm_bits.py index d8034b31b046..8e20a444075e 100644 --- a/test_runner/regress/test_vm_bits.py +++ b/test_runner/regress/test_vm_bits.py @@ -19,18 +19,40 @@ def test_vm_bit_clear(neon_simple_env: NeonEnv): # Install extension containing function needed for test cur.execute("CREATE EXTENSION neon_test_utils") - # Create a test table and freeze it to set the VM bit. + # Create a test table for a few different scenarios and freeze it to set the VM bits. cur.execute("CREATE TABLE vmtest_delete (id integer PRIMARY KEY)") cur.execute("INSERT INTO vmtest_delete VALUES (1)") cur.execute("VACUUM FREEZE vmtest_delete") - cur.execute("CREATE TABLE vmtest_update (id integer PRIMARY KEY)") - cur.execute("INSERT INTO vmtest_update SELECT g FROM generate_series(1, 1000) g") - cur.execute("VACUUM FREEZE vmtest_update") + cur.execute("CREATE TABLE vmtest_hot_update (id integer PRIMARY KEY, filler text)") + cur.execute("INSERT INTO vmtest_hot_update VALUES (1, 'x')") + cur.execute("VACUUM FREEZE vmtest_hot_update") + + cur.execute("CREATE TABLE vmtest_cold_update (id integer PRIMARY KEY)") + cur.execute("INSERT INTO vmtest_cold_update SELECT g FROM generate_series(1, 1000) g") + cur.execute("VACUUM FREEZE vmtest_cold_update") + + cur.execute( + "CREATE TABLE vmtest_cold_update2 (id integer PRIMARY KEY, filler text) WITH (fillfactor=100)" + ) + cur.execute("INSERT INTO vmtest_cold_update2 SELECT g, '' FROM generate_series(1, 1000) g") + cur.execute("VACUUM FREEZE vmtest_cold_update2") # DELETE and UPDATE the rows. cur.execute("DELETE FROM vmtest_delete WHERE id = 1") - cur.execute("UPDATE vmtest_update SET id = 5000 WHERE id = 1") + cur.execute("UPDATE vmtest_hot_update SET filler='x' WHERE id = 1") + cur.execute("UPDATE vmtest_cold_update SET id = 5000 WHERE id = 1") + + # Clear the VM bit on the last page with an INSERT. Then clear the VM bit on + # the page where row 1 is (block 0), by doing an UPDATE. The UPDATE is a + # cold update, and the new tuple goes to the last page, which already had + # its VM bit cleared. The point is that the UPDATE *only* clears the VM bit + # on the page containing the old tuple. We had a bug where we got the old + # and new pages mixed up, and that only shows up when one of the bits is + # cleared, but not the other one. + cur.execute("INSERT INTO vmtest_cold_update2 VALUES (9999, 'x')") + # Clears the VM bit on the old page + cur.execute("UPDATE vmtest_cold_update2 SET id = 5000, filler=repeat('x', 200) WHERE id = 1") # Branch at this point, to test that later fork_at_current_lsn(env, endpoint, "test_vm_bit_clear_new", "test_vm_bit_clear") @@ -50,9 +72,13 @@ def test_vm_bit_clear(neon_simple_env: NeonEnv): """ ) - cur.execute("SELECT * FROM vmtest_delete WHERE id = 1") + cur.execute("SELECT id FROM vmtest_delete WHERE id = 1") + assert cur.fetchall() == [] + cur.execute("SELECT id FROM vmtest_hot_update WHERE id = 1") + assert cur.fetchall() == [(1,)] + cur.execute("SELECT id FROM vmtest_cold_update WHERE id = 1") assert cur.fetchall() == [] - cur.execute("SELECT * FROM vmtest_update WHERE id = 1") + cur.execute("SELECT id FROM vmtest_cold_update2 WHERE id = 1") assert cur.fetchall() == [] cur.close() @@ -77,7 +103,11 @@ def test_vm_bit_clear(neon_simple_env: NeonEnv): """ ) - cur_new.execute("SELECT * FROM vmtest_delete WHERE id = 1") + cur_new.execute("SELECT id FROM vmtest_delete WHERE id = 1") + assert cur_new.fetchall() == [] + cur_new.execute("SELECT id FROM vmtest_hot_update WHERE id = 1") + assert cur_new.fetchall() == [(1,)] + cur_new.execute("SELECT id FROM vmtest_cold_update WHERE id = 1") assert cur_new.fetchall() == [] - cur_new.execute("SELECT * FROM vmtest_update WHERE id = 1") + cur_new.execute("SELECT id FROM vmtest_cold_update2 WHERE id = 1") assert cur_new.fetchall() == []