From e4f05ce0a228db257cc8489ce6630ce5dd4c9981 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 28 May 2023 02:39:44 +0300 Subject: [PATCH] Enable sanity check that disk_consistent_lsn is valid on created timeline. Commit `create_test_timeline: always put@initdb_lsn the minimum required keys` already switched us over to using valid initdb_lsns. All that's left to do is to actually flush the minimum keys so that we move from disk_consistent_lsn=Lsn(0) to disk_consistent_lsn=initdb_lsn. Co-authored-by: Christian Schwarz Part of https://github.com/neondatabase/neon/pull/4364 --- pageserver/src/tenant.rs | 97 +++++++++++-------- .../src/tenant/remote_timeline_client.rs | 7 +- .../walreceiver/connection_manager.rs | 1 + pageserver/src/walingest.rs | 16 ++- 4 files changed, 78 insertions(+), 43 deletions(-) diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 85393e341f..7e8ca7fb71 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -11,7 +11,7 @@ //! parent timeline, and the last LSN that has been written to disk. //! -use anyhow::{bail, Context}; +use anyhow::{bail, ensure, Context}; use futures::FutureExt; use pageserver_api::models::TimelineState; use remote_storage::DownloadError; @@ -201,11 +201,11 @@ impl UninitializedTimeline<'_> { })?; // Check that the caller initialized disk_consistent_lsn - // - // TODO: many our unit tests violate this. - //let new_disk_consistent_lsn = new_timeline.get_disk_consistent_lsn(); - //anyhow::ensure!(new_disk_consistent_lsn.is_valid(), - // "new timeline {tenant_id}/{timeline_id} has invalid disk_consistent_lsn"); + let new_disk_consistent_lsn = new_timeline.get_disk_consistent_lsn(); + ensure!( + new_disk_consistent_lsn.is_valid(), + "new timeline {tenant_id}/{timeline_id} has invalid disk_consistent_lsn" + ); let mut timelines = self.owning_tenant.timelines.lock().unwrap(); match timelines.entry(timeline_id) { @@ -1286,7 +1286,7 @@ impl Tenant { // This makes the various functions which anyhow::ensure! for Active state work in tests. // Our current tests don't need the background loops. #[cfg(test)] - pub fn create_test_timeline( + pub async fn create_test_timeline( &self, new_timeline_id: TimelineId, initdb_lsn: Lsn, @@ -1306,6 +1306,10 @@ impl Tenant { .commit() .context("commit init_empty_test_timeline modification")?; + // Flush to disk so that uninit_tl's check for valid disk_consistent_lsn passes. + tline.maybe_spawn_flush_loop(); + tline.freeze_and_flush().await.context("freeze_and_flush")?; + let tl = uninit_tl.finish_creation()?; // The non-test code would call tl.activate() here. tl.set_state(TimelineState::Active); @@ -3572,8 +3576,9 @@ mod tests { #[tokio::test] async fn test_basic() -> anyhow::Result<()> { let (tenant, ctx) = TenantHarness::create("test_basic")?.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx) + .await?; let writer = tline.writer(); writer.put(*TEST_KEY, Lsn(0x10), &Value::Image(TEST_IMG("foo at 0x10")))?; @@ -3606,7 +3611,9 @@ mod tests { let (tenant, ctx) = TenantHarness::create("no_duplicate_timelines")? .load() .await; - let _ = tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let _ = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; match tenant.create_empty_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) { Ok(_) => panic!("duplicate timeline creation should fail"), @@ -3637,8 +3644,9 @@ mod tests { use std::str::from_utf8; let (tenant, ctx) = TenantHarness::create("test_branch")?.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; let writer = tline.writer(); #[allow(non_snake_case)] @@ -3735,8 +3743,9 @@ mod tests { TenantHarness::create("test_prohibit_branch_creation_on_garbage_collected_data")? .load() .await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x20)).await?; // this removes layers before lsn 40 (50 minus 10), so there are two remaining layers, image and delta for 31-50 @@ -3773,8 +3782,9 @@ mod tests { .load() .await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x50), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x50), DEFAULT_PG_VERSION, &ctx) + .await?; // try to branch at lsn 0x25, should fail because initdb lsn is 0x50 match tenant .branch_timeline_test(&tline, NEW_TIMELINE_ID, Some(Lsn(0x25)), &ctx) @@ -3823,8 +3833,9 @@ mod tests { TenantHarness::create("test_get_branchpoints_from_an_inactive_timeline")? .load() .await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x20)).await?; tenant @@ -3872,8 +3883,9 @@ mod tests { TenantHarness::create("test_retain_data_in_parent_which_is_needed_for_child")? .load() .await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x20)).await?; tenant @@ -3896,8 +3908,9 @@ mod tests { TenantHarness::create("test_parent_keeps_data_forever_after_branching")? .load() .await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x20)).await?; tenant @@ -3929,8 +3942,9 @@ mod tests { let harness = TenantHarness::create(TEST_NAME)?; { let (tenant, ctx) = harness.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x7000), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x7000), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x8000)).await?; } @@ -3949,8 +3963,9 @@ mod tests { // create two timelines { let (tenant, ctx) = harness.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; make_some_layers(tline.as_ref(), Lsn(0x20)).await?; @@ -3987,8 +4002,9 @@ mod tests { let harness = TenantHarness::create(TEST_NAME)?; let (tenant, ctx) = harness.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; drop(tline); drop(tenant); @@ -4026,8 +4042,9 @@ mod tests { #[tokio::test] async fn test_images() -> anyhow::Result<()> { let (tenant, ctx) = TenantHarness::create("test_images")?.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx) + .await?; let writer = tline.writer(); writer.put(*TEST_KEY, Lsn(0x10), &Value::Image(TEST_IMG("foo at 0x10")))?; @@ -4092,8 +4109,9 @@ mod tests { #[tokio::test] async fn test_bulk_insert() -> anyhow::Result<()> { let (tenant, ctx) = TenantHarness::create("test_bulk_insert")?.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx) + .await?; let mut lsn = Lsn(0x10); @@ -4135,8 +4153,9 @@ mod tests { #[tokio::test] async fn test_random_updates() -> anyhow::Result<()> { let (tenant, ctx) = TenantHarness::create("test_random_updates")?.load().await; - let tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; const NUM_KEYS: usize = 1000; @@ -4209,8 +4228,9 @@ mod tests { let (tenant, ctx) = TenantHarness::create("test_traverse_branches")? .load() .await; - let mut tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let mut tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; const NUM_KEYS: usize = 1000; @@ -4292,8 +4312,9 @@ mod tests { let (tenant, ctx) = TenantHarness::create("test_traverse_ancestors")? .load() .await; - let mut tline = - tenant.create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx)?; + let mut tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) + .await?; const NUM_KEYS: usize = 100; const NUM_TLINES: usize = 50; diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 51623042f3..2c84c59dcb 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -1392,7 +1392,12 @@ mod tests { let harness = TenantHarness::create(test_name)?; let (tenant, ctx) = runtime.block_on(harness.load()); // create an empty timeline directory - let _ = tenant.create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx)?; + let _ = runtime.block_on(tenant.create_test_timeline( + TIMELINE_ID, + Lsn(8), + DEFAULT_PG_VERSION, + &ctx, + ))?; let remote_fs_dir = harness.conf.workdir.join("remote_fs"); std::fs::create_dir_all(remote_fs_dir)?; diff --git a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs index 266bdfca86..83dfc5f598 100644 --- a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs +++ b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs @@ -1325,6 +1325,7 @@ mod tests { let (tenant, ctx) = harness.load().await; let timeline = tenant .create_test_timeline(TIMELINE_ID, Lsn(0x8), crate::DEFAULT_PG_VERSION, &ctx) + .await .expect("Failed to create an empty timeline for dummy wal connection manager"); ConnectionManagerState { diff --git a/pageserver/src/walingest.rs b/pageserver/src/walingest.rs index 40c0c4c7cb..15fd1683f4 100644 --- a/pageserver/src/walingest.rs +++ b/pageserver/src/walingest.rs @@ -1208,7 +1208,9 @@ mod tests { #[tokio::test] async fn test_relsize() -> Result<()> { let (tenant, ctx) = TenantHarness::create("test_relsize")?.load().await; - let tline = tenant.create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx) + .await?; let mut walingest = init_walingest_test(&tline, &ctx).await?; let mut m = tline.begin_modification(Lsn(0x20)); @@ -1427,7 +1429,9 @@ mod tests { #[tokio::test] async fn test_drop_extend() -> Result<()> { let (tenant, ctx) = TenantHarness::create("test_drop_extend")?.load().await; - let tline = tenant.create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx) + .await?; let mut walingest = init_walingest_test(&tline, &ctx).await?; let mut m = tline.begin_modification(Lsn(0x20)); @@ -1496,7 +1500,9 @@ mod tests { #[tokio::test] async fn test_truncate_extend() -> Result<()> { let (tenant, ctx) = TenantHarness::create("test_truncate_extend")?.load().await; - let tline = tenant.create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx) + .await?; let mut walingest = init_walingest_test(&tline, &ctx).await?; // Create a 20 MB relation (the size is arbitrary) @@ -1636,7 +1642,9 @@ mod tests { #[tokio::test] async fn test_large_rel() -> Result<()> { let (tenant, ctx) = TenantHarness::create("test_large_rel")?.load().await; - let tline = tenant.create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx)?; + let tline = tenant + .create_test_timeline(TIMELINE_ID, Lsn(8), DEFAULT_PG_VERSION, &ctx) + .await?; let mut walingest = init_walingest_test(&tline, &ctx).await?; let mut lsn = 0x10;