From a63153f4bcdfd1a29028a6a07870f466858690ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arpad=20M=C3=BCller?= <arpad@neon.tech>
Date: Tue, 21 Jan 2025 19:10:36 +0100
Subject: [PATCH] Optional get_timeline result

---
 storage_controller/src/persistence.rs                 |  9 ++++++---
 storage_controller/src/service.rs                     |  4 ++++
 .../src/service/safekeeper_reconciler.rs              | 11 ++++++-----
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/storage_controller/src/persistence.rs b/storage_controller/src/persistence.rs
index e94c9960ac..bb12583cea 100644
--- a/storage_controller/src/persistence.rs
+++ b/storage_controller/src/persistence.rs
@@ -1258,11 +1258,12 @@ impl Persistence {
         .await
     }
 
+    /// Obtains the timeline, returns None if not present
     pub(crate) async fn get_timeline(
         &self,
         tenant_id: TenantId,
         timeline_id: TimelineId,
-    ) -> DatabaseResult<TimelinePersistence> {
+    ) -> DatabaseResult<Option<TimelinePersistence>> {
         use crate::schema::timelines;
         let mut timelines: Vec<TimelineFromDb> = self
             .with_measured_conn(
@@ -1275,7 +1276,9 @@ impl Persistence {
                 },
             )
             .await?;
-        if timelines.len() != 1 {
+        if timelines.len() == 0 {
+            return Ok(None);
+        } else if timelines.len() > 1 {
             return Err(DatabaseError::Logical(format!(
                 "incorrect number of returned timelines: ({})",
                 timelines.len()
@@ -1286,7 +1289,7 @@ impl Persistence {
 
         tracing::info!("get_timeline: loaded timeline");
 
-        Ok(tl)
+        Ok(Some(tl))
     }
 
     pub(crate) async fn timelines_to_be_reconciled(
diff --git a/storage_controller/src/service.rs b/storage_controller/src/service.rs
index 1e0ba5b7e4..8fda115549 100644
--- a/storage_controller/src/service.rs
+++ b/storage_controller/src/service.rs
@@ -4270,6 +4270,10 @@ impl Service {
             .persistence
             .get_timeline(tenant_id, timeline_id)
             .await?;
+        let Some(tl) = tl else {
+            tracing::info!("timeline {tenant_id}/{timeline_id} doesn't exist in timelines table, no deletions on safekeepers needed");
+            return Ok(());
+        };
         let status_kind =
             TimelineStatusKind::from_str(&tl.status_kind).map_err(ApiError::InternalServerError)?;
         if status_kind != TimelineStatusKind::Deleting {
diff --git a/storage_controller/src/service/safekeeper_reconciler.rs b/storage_controller/src/service/safekeeper_reconciler.rs
index c5208c5f36..8744b75dca 100644
--- a/storage_controller/src/service/safekeeper_reconciler.rs
+++ b/storage_controller/src/service/safekeeper_reconciler.rs
@@ -76,11 +76,7 @@ impl SafekeeperReconciler {
         tl: &TimelinePersistence,
         sk_persistences: &HashMap<i64, SafekeeperPersistence>,
     ) -> Result<(), anyhow::Error> {
-        tracing::info!(
-            "Reconciling timeline on safekeepers {}/{}",
-            tl.tenant_id,
-            tl.timeline_id,
-        );
+        tracing::info!("Reconciling timeline on safekeepers");
         let tenant_id = TenantId::from_slice(tl.tenant_id.as_bytes())?;
         let timeline_id = TimelineId::from_slice(tl.timeline_id.as_bytes())?;
 
@@ -98,6 +94,11 @@ impl SafekeeperReconciler {
             .persistence
             .get_timeline(tenant_id, timeline_id)
             .await?;
+        let Some(tl) = tl else {
+            // This can happen but is a bit unlikely, so print it on the warn level instead of info
+            tracing::warn!("timeline row in database disappeared");
+            return Ok(());
+        };
         let status = TimelineStatusKind::from_str(&tl.status)?;
         match status {
             TimelineStatusKind::Created | TimelineStatusKind::Deleted => (),