Avoid point in time concept on pageserver level

control_plane/src/compute.rs

@@ -10,7 +10,7 @@ use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
 
-use anyhow::{Context, Result};
+use anyhow::{bail, Context, Result};
 use zenith_utils::connstring::connection_host_port;
 use zenith_utils::lsn::Lsn;
 use zenith_utils::postgres_backend::AuthType;
@@ -73,31 +73,6 @@ impl ComputeControlPlane {
             .unwrap_or(self.base_port)
     }
 
-    // FIXME: see also parse_point_in_time in timelines.rs.
-    fn parse_point_in_time(
-        &self,
-        tenant_id: ZTenantId,
-        s: &str,
-    ) -> Result<(ZTimelineId, Option<Lsn>)> {
-        let _strings = s.split('@');
-        // let name = strings.next().unwrap();
-
-        // let lsn = strings
-        //     .next()
-        //     .map(Lsn::from_str)
-        //     .transpose()
-        //     .context("invalid LSN in point-in-time specification")?;
-
-        // // Resolve the timeline ID, given the human-readable branch name
-        // let timeline_id = self
-        //     .pageserver
-        //     .branch_get_by_name(&tenant_id, name)?
-        //     .timeline_id;
-
-        // Ok((timeline_id, lsn))
-        todo!("TODO kb check more about the '@name' format")
-    }
-
     pub fn new_node(
         &mut self,
         tenantid: ZTenantId,
@@ -107,7 +82,7 @@ impl ComputeControlPlane {
     ) -> Result<Arc<PostgresNode>> {
         // Resolve the human-readable timeline spec into timeline ID and LSN
         let (timelineid, lsn) = match timeline_spec {
-            Some(timeline_spec) => self.parse_point_in_time(tenantid, timeline_spec)?,
+            Some(timeline_spec) => parse_point_in_time(timeline_spec)?,
             None => (ZTimelineId::generate(), None),
         };
 
@@ -134,6 +109,44 @@ impl ComputeControlPlane {
     }
 }
 
+// Parse user-given string that represents a point-in-time.
+//
+// Variants suported:
+//
+// Raw timeline id in hex, meaning the end of that timeline:
+//    bc62e7d612d0e6fe8f99a6dd2f281f9d
+//
+// A specific LSN on a timeline:
+//    bc62e7d612d0e6fe8f99a6dd2f281f9d@2/15D3DD8
+//
+fn parse_point_in_time(timeline_spec: &str) -> anyhow::Result<(ZTimelineId, Option<Lsn>)> {
+    let mut strings = timeline_spec.split('@');
+
+    let name = match strings.next() {
+        Some(n) => n,
+        None => bail!("invalid timeline specification: {}", timeline_spec),
+    };
+    let timeline_id = ZTimelineId::from_str(name).with_context(|| {
+        format!(
+            "failed to parse the timeline id from specification: {}",
+            timeline_spec
+        )
+    })?;
+
+    let lsn = strings
+        .next()
+        .map(Lsn::from_str)
+        .transpose()
+        .with_context(|| {
+            format!(
+                "failed to parse the Lsn from timeline specification: {}",
+                timeline_spec
+            )
+        })?;
+
+    Ok((timeline_id, lsn))
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 #[derive(Debug)]

control_plane/src/storage.rs

@@ -16,6 +16,7 @@ use reqwest::blocking::{Client, RequestBuilder, Response};
 use reqwest::{IntoUrl, Method};
 use thiserror::Error;
 use zenith_utils::http::error::HttpErrorBody;
+use zenith_utils::lsn::Lsn;
 use zenith_utils::postgres_backend::AuthType;
 use zenith_utils::zid::{ZTenantId, ZTimelineId};
 
@@ -348,16 +349,16 @@ impl PageServerNode {
 
     pub fn timeline_create(
         &self,
-        timeline_id: ZTimelineId,
-        start_point: String,
         tenant_id: ZTenantId,
+        timeline_id: ZTimelineId,
+        start_lsn: Option<Lsn>,
     ) -> Result<TimelineInfo> {
         Ok(self
             .http_request(Method::POST, format!("{}/timeline", self.http_base_url))
             .json(&TimelineCreateRequest {
                 tenant_id,
                 timeline_id,
-                start_point,
+                start_lsn,
             })
             .send()?
             .error_from_body()?

pageserver/src/config.rs

@@ -392,14 +392,6 @@ impl PageServerConf {
         self.tenants_path().join(tenantid.to_string())
     }
 
-    pub fn tags_path(&self, tenantid: &ZTenantId) -> PathBuf {
-        self.tenant_path(tenantid).join("refs").join("tags")
-    }
-
-    pub fn tag_path(&self, tag_name: &str, tenantid: &ZTenantId) -> PathBuf {
-        self.tags_path(tenantid).join(tag_name)
-    }
-
     pub fn timelines_path(&self, tenantid: &ZTenantId) -> PathBuf {
         self.tenant_path(tenantid).join(TIMELINES_SEGMENT_NAME)
     }
@@ -408,10 +400,6 @@ impl PageServerConf {
         self.timelines_path(tenantid).join(timelineid.to_string())
     }
 
-    pub fn ancestor_path(&self, timelineid: &ZTimelineId, tenantid: &ZTenantId) -> PathBuf {
-        self.timeline_path(timelineid, tenantid).join("ancestor")
-    }
-
     //
     // Postgres distribution paths
     //

pageserver/src/http/models.rs

@@ -1,5 +1,5 @@
 use serde::{Deserialize, Serialize};
-use zenith_utils::zid::ZTimelineId;
+use zenith_utils::{lsn::Lsn, zid::ZTimelineId};
 
 use crate::ZTenantId;
 use zenith_utils::zid::ZNodeId;
@@ -10,7 +10,7 @@ pub struct TimelineCreateRequest {
     pub tenant_id: ZTenantId,
     #[serde(with = "hex")]
     pub timeline_id: ZTimelineId,
-    pub start_point: String,
+    pub start_lsn: Option<Lsn>,
 }
 
 #[derive(Serialize, Deserialize)]

pageserver/src/http/routes.rs

@@ -77,12 +77,12 @@ async fn timeline_create_handler(mut request: Request<Body>) -> Result<Response<
     check_permission(&request, Some(request_data.tenant_id))?;
 
     let response_data = tokio::task::spawn_blocking(move || {
-        let _enter = info_span!("/timeline_create", timeline = %request_data.timeline_id, tenant = %request_data.tenant_id, startpoint=%request_data.start_point).entered();
+        let _enter = info_span!("/timeline_create", timeline = %request_data.timeline_id, tenant = %request_data.tenant_id, lsn=?request_data.start_lsn).entered();
         timelines::create_timeline(
             get_config(&request),
-            &request_data.start_point,
             request_data.tenant_id,
             request_data.timeline_id,
+            request_data.start_lsn,
         )
     })
     .await

pageserver/src/timelines.rs

@@ -39,7 +39,7 @@ pub struct TimelineInfo {
 
 #[derive(Debug, Clone, Copy)]
 pub struct PointInTime {
-    pub timelineid: ZTimelineId,
+    pub timeline_id: ZTimelineId,
     pub lsn: Lsn,
 }
 
@@ -86,7 +86,6 @@ pub fn create_repo(
         .with_context(|| format!("could not create directory {}", repo_dir.display()))?;
 
     crashsafe_dir::create_dir(conf.timelines_path(&tenantid))?;
-    crashsafe_dir::create_dir_all(conf.tags_path(&tenantid))?;
 
     info!("created directory structure in {}", repo_dir.display());
 
@@ -253,19 +252,23 @@ pub(crate) fn get_timelines(
 
 pub(crate) fn create_timeline(
     conf: &PageServerConf,
-    startpoint_str: &str,
     tenant_id: ZTenantId,
     timeline_id: ZTimelineId,
+    start_lsn: Option<Lsn>,
 ) -> Result<TimelineInfo> {
-    let repo = tenant_mgr::get_repository_for_tenant(tenant_id)?;
-
     if conf.timeline_path(&timeline_id, &tenant_id).exists() {
         bail!("timeline {} already exists", timeline_id);
     }
 
-    let mut startpoint = parse_point_in_time(conf, startpoint_str, &tenant_id)?;
+    let repo = tenant_mgr::get_repository_for_tenant(tenant_id)?;
+
+    let mut startpoint = PointInTime {
+        timeline_id,
+        lsn: start_lsn.unwrap_or(Lsn(0)),
+    };
+
     let timeline = repo
-        .get_timeline(startpoint.timelineid)?
+        .get_timeline(startpoint.timeline_id)?
         .local_timeline()
         .context("Cannot branch off the timeline that's not present locally")?;
     if startpoint.lsn == Lsn(0) {
@@ -297,7 +300,7 @@ pub(crate) fn create_timeline(
 
     // Forward entire timeline creation routine to repository
     // backend, so it can do all needed initialization
-    repo.branch_timeline(startpoint.timelineid, new_timeline_id, startpoint.lsn)?;
+    repo.branch_timeline(startpoint.timeline_id, new_timeline_id, startpoint.lsn)?;
 
     // Remember the human-readable branch name for the new timeline.
     // FIXME: there's a race condition, if you create a branch with the same
@@ -309,59 +312,9 @@ pub(crate) fn create_timeline(
     Ok(TimelineInfo {
         timeline_id: new_timeline_id,
         latest_valid_lsn: startpoint.lsn,
-        ancestor_id: Some(startpoint.timelineid.to_string()),
+        ancestor_id: Some(startpoint.timeline_id.to_string()),
         ancestor_lsn: Some(startpoint.lsn.to_string()),
         current_logical_size: 0,
         current_logical_size_non_incremental: Some(0),
     })
 }
-
-//
-// Parse user-given string that represents a point-in-time.
-//
-// We support multiple variants:
-//
-// Raw timeline id in hex, meaning the end of that timeline:
-//    bc62e7d612d0e6fe8f99a6dd2f281f9d
-//
-// A specific LSN on a timeline:
-//    bc62e7d612d0e6fe8f99a6dd2f281f9d@2/15D3DD8
-//
-fn parse_point_in_time(
-    conf: &PageServerConf,
-    s: &str,
-    tenantid: &ZTenantId,
-) -> Result<PointInTime> {
-    let mut strings = s.split('@');
-    let name = strings.next().unwrap();
-
-    let lsn = strings
-        .next()
-        .map(Lsn::from_str)
-        .transpose()
-        .context("invalid LSN in point-in-time specification")?;
-
-    // Check if it's a tag
-    if lsn.is_none() {
-        let tagpath = conf.tag_path(name, tenantid);
-        if tagpath.exists() {
-            let pointstr = fs::read_to_string(tagpath)?;
-
-            return parse_point_in_time(conf, &pointstr, tenantid);
-        }
-    }
-
-    // Check if it's a timelineid
-    // Check if it's timelineid @ LSN
-    if let Ok(timelineid) = ZTimelineId::from_str(name) {
-        let tlipath = conf.timeline_path(&timelineid, tenantid);
-        if tlipath.exists() {
-            return Ok(PointInTime {
-                timelineid,
-                lsn: lsn.unwrap_or(Lsn(0)),
-            });
-        }
-    }
-
-    bail!("could not parse point-in-time {}", s);
-}

zenith/src/main.rs

@@ -17,6 +17,7 @@ use walkeeper::defaults::{
     DEFAULT_PG_LISTEN_PORT as DEFAULT_SAFEKEEPER_PG_PORT,
 };
 use zenith_utils::auth::{Claims, Scope};
+use zenith_utils::lsn::Lsn;
 use zenith_utils::postgres_backend::AuthType;
 use zenith_utils::zid::{ZNodeId, ZTenantId, ZTimelineId};
 use zenith_utils::GIT_VERSION;
@@ -464,14 +465,15 @@ fn handle_timeline(timeline_match: &ArgMatches, env: &local_env::LocalEnv) -> Re
     let tenant_id = get_tenantid(timeline_match, env)?;
 
     if let Some(timeline_id) = timeline_match.value_of("timeline-id") {
-        let startpoint_str = timeline_match
-            .value_of("start-point")
-            .context("Missing start-point")?;
+        let start_lsn = timeline_match
+            .value_of("start-lsn")
+            .map(|lsn| lsn.parse::<Lsn>())
+            .transpose()
+            .context("Failed to parse start Lsn from the request")?;
         let timeline_id = timeline_id
             .parse::<ZTimelineId>()
             .context("Failed to parse timeline id from the request")?;
-        let timeline =
-            pageserver.timeline_create(timeline_id, startpoint_str.to_owned(), tenant_id)?;
+        let timeline = pageserver.timeline_create(tenant_id, timeline_id, start_lsn)?;
         println!(
             "Created timeline '{}' at {:?} for tenant: {}",
             timeline.timeline_id, timeline.latest_valid_lsn, tenant_id,