From 122e23071b4aee2e50f0802b89e529f67f3e2623 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Fri, 26 May 2023 09:23:26 +0200
Subject: [PATCH] fix the tests (commenting out too-conservative "Timeline has
 no ancestor and no layer files" assert)

---
 pageserver/src/tenant.rs | 54 +++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs
index 2b58881cf1..cacf80b364 100644
--- a/pageserver/src/tenant.rs
+++ b/pageserver/src/tenant.rs
@@ -462,17 +462,17 @@ impl Tenant {
         }
 
         // Sanity check: a timeline should have some content.
-        anyhow::ensure!(
-            ancestor.is_some()
-                || timeline
-                    .layers
-                    .read()
-                    .unwrap()
-                    .iter_historic_layers()
-                    .next()
-                    .is_some(),
-            "Timeline has no ancestor and no layer files"
-        );
+        // anyhow::ensure!(
+        //     ancestor.is_some()
+        //         || timeline
+        //             .layers
+        //             .read()
+        //             .unwrap()
+        //             .iter_historic_layers()
+        //             .next()
+        //             .is_some(),
+        //     "Timeline has no ancestor and no layer files"
+        // );
 
         // Save the metadata file to local disk.
         if !picked_local {
@@ -1225,13 +1225,16 @@ impl Tenant {
     ) -> anyhow::Result<Arc<Timeline>> {
         let (uninit_mark, tl) = self
             .create_empty_timeline(new_timeline_id, initdb_lsn, pg_version, ctx)
+            // make the debug_assert_current_span_has_tenant_id() in create_empty_timeline() happy
+            .instrument(tracing::info_span!("create_test_timeline", tenant_id=%self.tenant_id))
             .await
-            .context("create empty timelien")?;
+            .context("create empty timeline")?;
         // the tests don't need any content in the timeline, we're done here
         uninit_mark
             .remove_uninit_mark()
             .context("remove_uninit_mark")?;
         // The non-test code would call tl.activate() here.
+        tl.maybe_spawn_flush_loop();
         tl.set_state(TimelineState::Active);
         Ok(tl)
     }
@@ -2487,6 +2490,7 @@ impl Tenant {
     /// calls pass, but, we do not actually call `.activate()` under the hood. So, none of the
     /// timeline background tasks are launched, except the flush loop.
     #[cfg(test)]
+    #[instrument(skip_all, fields(tenant_id=%self.tenant_id))]
     async fn branch_timeline_test(
         &self,
         src_timeline: &Arc<Timeline>,
@@ -2510,6 +2514,7 @@ impl Tenant {
         let metadata = load_metadata(self.conf, dst_id, self.tenant_id)
             .context("load newly created on-disk timeline metadata")?;
         self.load_local_timeline(dst_id, metadata, TimelineLoadCause::Test, ctx)
+            .instrument(info_span!("load_local_timeline", timeline_id=%dst_id))
             .await
             .context("load newly created on-disk timeline state")?;
 
@@ -3475,10 +3480,7 @@ mod tests {
             Ok(_) => panic!("duplicate timeline creation should fail"),
             Err(e) => assert_eq!(
                 e.to_string(),
-                format!(
-                    "Timeline {}/{} already exists in pageserver's memory",
-                    tenant.tenant_id, TIMELINE_ID
-                )
+                format!("timeline {} already exists", TIMELINE_ID)
             ),
         }
 
@@ -3619,12 +3621,11 @@ mod tests {
         {
             Ok(_) => panic!("branching should have failed"),
             Err(err) => {
-                assert!(err.to_string().contains("invalid branch start lsn"));
-                assert!(err
-                    .source()
-                    .unwrap()
-                    .to_string()
-                    .contains("we might've already garbage collected needed data"))
+                println!("err: {:?}", err);
+                assert!(format!("{err:?}").contains("invalid branch start lsn"));
+                assert!(format!("{err:?}").contains("is earlier than latest GC horizon"));
+                assert!(format!("{err:?}")
+                    .contains("we might've already garbage collected needed data"));
             }
         }
 
@@ -3648,12 +3649,9 @@ mod tests {
         {
             Ok(_) => panic!("branching should have failed"),
             Err(err) => {
-                assert!(&err.to_string().contains("invalid branch start lsn"));
-                assert!(&err
-                    .source()
-                    .unwrap()
-                    .to_string()
-                    .contains("is earlier than latest GC horizon"));
+                println!("err: {:?}", err);
+                assert!(format!("{err:?}").contains("invalid branch start lsn"));
+                assert!(format!("{err:?}").contains("is earlier than latest GC horizon"));
             }
         }