diff --git a/Dockerfile.compute-node b/Dockerfile.compute-node index ecf76f97c2..d724185d5d 100644 --- a/Dockerfile.compute-node +++ b/Dockerfile.compute-node @@ -416,7 +416,7 @@ RUN case "${PG_VERSION}" in \ ;; \ "v16") \ export PG_HINT_PLAN_VERSION=16_1_6_0 \ - export PG_HINT_PLAN_CHECKSUM=ce6a8040c78012000f5da7240caf6a971401412f41d33f930f09291e6c304b99 \ + export PG_HINT_PLAN_CHECKSUM=fc85a9212e7d2819d4ae4ac75817481101833c3cfa9f0fe1f980984e12347d00 \ ;; \ *) \ echo "Export the valid PG_HINT_PLAN_VERSION variable" && exit 1 \ diff --git a/libs/vm_monitor/src/cgroup.rs b/libs/vm_monitor/src/cgroup.rs index 4f529d16fd..fe1c514f76 100644 --- a/libs/vm_monitor/src/cgroup.rs +++ b/libs/vm_monitor/src/cgroup.rs @@ -315,12 +315,8 @@ impl CgroupWatcher { where E: Stream>, { - // There are several actions might do when receiving a `memory.high`, - // such as freezing the cgroup, or increasing its `memory.high`. We don't - // want to do these things too often (because postgres needs to run, and - // we only have so much memory). These timers serve as rate limits for this. let mut wait_to_freeze = pin!(tokio::time::sleep(Duration::ZERO)); - let mut wait_to_increase_memory_high = pin!(tokio::time::sleep(Duration::ZERO)); + let mut last_memory_high_increase_at: Option = None; let mut events = pin!(events); // Are we waiting to be upscaled? Could be true if we request upscale due @@ -332,6 +328,8 @@ impl CgroupWatcher { upscale = upscales.recv() => { let Sequenced { seqnum, data } = upscale .context("failed to listen on upscale notification channel")?; + waiting_on_upscale = false; + last_memory_high_increase_at = None; self.last_upscale_seqnum.store(seqnum, Ordering::Release); info!(cpu = data.cpu, mem_bytes = data.mem, "received upscale"); } @@ -396,12 +394,17 @@ impl CgroupWatcher { .send(()) .await .context("failed to request upscale")?; + waiting_on_upscale = true; continue; } // Shoot, we can't freeze or and we're still waiting on upscale, // increase memory.high to reduce throttling - if wait_to_increase_memory_high.is_elapsed() { + let can_increase_memory_high = match last_memory_high_increase_at { + None => true, + Some(t) => t.elapsed() > self.config.memory_high_increase_every, + }; + if can_increase_memory_high { info!( "received memory.high event, \ but too soon to refreeze and already requested upscale \ @@ -437,12 +440,11 @@ impl CgroupWatcher { ); self.set_high_bytes(new_high) .context("failed to set memory.high")?; - wait_to_increase_memory_high - .as_mut() - .reset(Instant::now() + self.config.memory_high_increase_every) + last_memory_high_increase_at = Some(Instant::now()); + continue; } - // we can't do anything + info!("received memory.high event, but can't do anything"); } }; } diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 9192af0ee8..d5ef1f2a24 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 d8034b31b0..8e20a44407 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 * FROM vmtest_update WHERE id = 1") + 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 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 * FROM vmtest_update WHERE id = 1") + 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 id FROM vmtest_cold_update2 WHERE id = 1") assert cur_new.fetchall() == []