better wording

Signed-off-by: Alex Chi Z <chi@neon.tech>
feat(storcon): passthrough detach v2 requests
2026-05-16 20:50:37 +00:00 · 2025-03-12 12:33:14 -04:00 · 2025-03-12 11:57:02 -04:00 · 2025-03-12 11:40:50 -04:00 · 2025-03-11 15:15:56 -04:00 · 2025-03-11 14:20:44 -04:00
11 changed files with 401 additions and 30 deletions
--- a/pageserver/client/src/mgmt_api.rs
+++ b/pageserver/client/src/mgmt_api.rs
@@ -471,6 +471,23 @@ impl Client {
            .map_err(Error::ReceiveBody)
    }

+    pub async fn timeline_detach_ancestor_v2(
+        &self,
+        tenant_shard_id: TenantShardId,
+        timeline_id: TimelineId,
+    ) -> Result<AncestorDetached> {
+        let uri = format!(
+            "{}/v1/tenant/{tenant_shard_id}/timeline/{timeline_id}/detach_ancestor_v2",
+            self.mgmt_api_endpoint
+        );
+
+        self.request(Method::PUT, &uri, ())
+            .await?
+            .json()
+            .await
+            .map_err(Error::ReceiveBody)
+    }
+
    pub async fn timeline_block_unblock_gc(
        &self,
        tenant_shard_id: TenantShardId,
--- a/pageserver/src/http/routes.rs
+++ b/pageserver/src/http/routes.rs
@@ -71,6 +71,7 @@ use crate::tenant::remote_timeline_client::{
 use crate::tenant::secondary::SecondaryController;
 use crate::tenant::size::ModelInputs;
 use crate::tenant::storage_layer::{IoConcurrency, LayerAccessStatsReset, LayerName};
+use crate::tenant::timeline::detach_ancestor::DetachBehavior;
 use crate::tenant::timeline::offload::{OffloadError, offload_timeline};
 use crate::tenant::timeline::{
    CompactFlags, CompactOptions, CompactRequest, CompactionError, Timeline, WaitLsnTimeout,
@@ -2489,8 +2490,29 @@ async fn timeline_download_remote_layers_handler_get(
    json_response(StatusCode::OK, info)
 }

-async fn timeline_detach_ancestor_handler(
+async fn timeline_detach_ancestor_handler_v1(
    request: Request<Body>,
+    cancel: CancellationToken,
+) -> Result<Response<Body>, ApiError> {
+    timeline_detach_ancestor_handler_common(request, DetachBehavior::NoAncestorAndReparent, cancel)
+        .await
+}
+
+async fn timeline_detach_ancestor_handler_v2(
+    request: Request<Body>,
+    cancel: CancellationToken,
+) -> Result<Response<Body>, ApiError> {
+    timeline_detach_ancestor_handler_common(
+        request,
+        DetachBehavior::MultiLevelAndNoReparent,
+        cancel,
+    )
+    .await
+}
+
+async fn timeline_detach_ancestor_handler_common(
+    request: Request<Body>,
+    behavior: DetachBehavior,
    _cancel: CancellationToken,
 ) -> Result<Response<Body>, ApiError> {
    use pageserver_api::models::detach_ancestor::AncestorDetached;
@@ -2548,7 +2570,7 @@ async fn timeline_detach_ancestor_handler(
        let timeline = tenant.get_timeline(timeline_id, true)?;

        let progress = timeline
-            .prepare_to_detach_from_ancestor(&tenant, options, ctx)
+            .prepare_to_detach_from_ancestor(&tenant, options, behavior, ctx)
            .await?;

        // uncomment to allow early as possible Tenant::drop
@@ -2563,6 +2585,7 @@ async fn timeline_detach_ancestor_handler(
                        tenant_shard_id,
                        timeline_id,
                        prepared,
+                        behavior,
                        attempt,
                        ctx,
                    )
@@ -3742,7 +3765,11 @@ pub fn make_router(
        )
        .put(
            "/v1/tenant/:tenant_shard_id/timeline/:timeline_id/detach_ancestor",
-            |r| api_handler(r, timeline_detach_ancestor_handler),
+            |r| api_handler(r, timeline_detach_ancestor_handler_v1),
+        )
+        .put(
+            "/v1/tenant/:tenant_shard_id/timeline/:timeline_id/detach_ancestor_v2",
+            |r| api_handler(r, timeline_detach_ancestor_handler_v2),
        )
        .delete("/v1/tenant/:tenant_shard_id/timeline/:timeline_id", |r| {
            api_handler(r, timeline_delete_handler)
--- a/pageserver/src/tenant/metadata.rs
+++ b/pageserver/src/tenant/metadata.rs
@@ -300,9 +300,8 @@ impl TimelineMetadata {

    /// Returns true if anything was changed
    pub fn detach_from_ancestor(&mut self, branchpoint: &(TimelineId, Lsn)) {
-        if let Some(ancestor) = self.body.ancestor_timeline {
-            assert_eq!(ancestor, branchpoint.0);
-        }
+        // Detaching from ancestor now doesn't always detach directly to the direct ancestor, but we
+        // ensure the LSN is the same. So we don't check the timeline ID.
        if self.body.ancestor_lsn != Lsn(0) {
            assert_eq!(self.body.ancestor_lsn, branchpoint.1);
        }
--- a/pageserver/src/tenant/mgr.rs
+++ b/pageserver/src/tenant/mgr.rs
@@ -1914,6 +1914,7 @@ impl TenantManager {
        tenant_shard_id: TenantShardId,
        timeline_id: TimelineId,
        prepared: PreparedTimelineDetach,
+        behavior: detach_ancestor::DetachBehavior,
        mut attempt: detach_ancestor::Attempt,
        ctx: &RequestContext,
    ) -> Result<HashSet<TimelineId>, detach_ancestor::Error> {
@@ -1957,7 +1958,14 @@ impl TenantManager {
            .map_err(Error::NotFound)?;

        let resp = timeline
-            .detach_from_ancestor_and_reparent(&tenant, prepared, ctx)
+            .detach_from_ancestor_and_reparent(
+                &tenant,
+                prepared,
+                attempt.ancestor_timeline_id,
+                attempt.ancestor_lsn,
+                behavior,
+                ctx,
+            )
            .await?;

        let mut slot_guard = slot_guard;
--- a/pageserver/src/tenant/timeline.rs
+++ b/pageserver/src/tenant/timeline.rs
@@ -5388,9 +5388,10 @@ impl Timeline {
        self: &Arc<Timeline>,
        tenant: &crate::tenant::Tenant,
        options: detach_ancestor::Options,
+        behavior: detach_ancestor::DetachBehavior,
        ctx: &RequestContext,
    ) -> Result<detach_ancestor::Progress, detach_ancestor::Error> {
-        detach_ancestor::prepare(self, tenant, options, ctx).await
+        detach_ancestor::prepare(self, tenant, behavior, options, ctx).await
    }

    /// Second step of detach from ancestor; detaches the `self` from it's current ancestor and
@@ -5406,9 +5407,21 @@ impl Timeline {
        self: &Arc<Timeline>,
        tenant: &crate::tenant::Tenant,
        prepared: detach_ancestor::PreparedTimelineDetach,
+        ancestor_timeline_id: TimelineId,
+        ancestor_lsn: Lsn,
+        behavior: detach_ancestor::DetachBehavior,
        ctx: &RequestContext,
    ) -> Result<detach_ancestor::DetachingAndReparenting, detach_ancestor::Error> {
-        detach_ancestor::detach_and_reparent(self, tenant, prepared, ctx).await
+        detach_ancestor::detach_and_reparent(
+            self,
+            tenant,
+            prepared,
+            ancestor_timeline_id,
+            ancestor_lsn,
+            behavior,
+            ctx,
+        )
+        .await
    }

    /// Final step which unblocks the GC.
--- a/pageserver/src/tenant/timeline/detach_ancestor.rs
+++ b/pageserver/src/tenant/timeline/detach_ancestor.rs
@@ -32,6 +32,9 @@ pub(crate) enum Error {
    #[error("too many ancestors")]
    TooManyAncestors,

+    #[error("ancestor is not empty")]
+    AncestorNotEmpty,
+
    #[error("shutting down, please retry later")]
    ShuttingDown,

@@ -89,7 +92,9 @@ impl From<Error> for ApiError {
    fn from(value: Error) -> Self {
        match value {
            Error::NoAncestor => ApiError::Conflict(value.to_string()),
-            Error::TooManyAncestors => ApiError::BadRequest(anyhow::anyhow!("{value}")),
+            Error::TooManyAncestors | Error::AncestorNotEmpty => {
+                ApiError::BadRequest(anyhow::anyhow!("{value}"))
+            }
            Error::ShuttingDown => ApiError::ShuttingDown,
            Error::Archived(_) => ApiError::BadRequest(anyhow::anyhow!("{value}")),
            Error::OtherTimelineDetachOngoing(_) | Error::FailedToReparentAll => {
@@ -127,13 +132,22 @@ pub(crate) struct PreparedTimelineDetach {
    layers: Vec<Layer>,
 }

-/// TODO: this should be part of PageserverConf because we cannot easily modify cplane arguments.
+// TODO: this should be part of PageserverConf because we cannot easily modify cplane arguments.
 #[derive(Debug)]
 pub(crate) struct Options {
    pub(crate) rewrite_concurrency: std::num::NonZeroUsize,
    pub(crate) copy_concurrency: std::num::NonZeroUsize,
 }

+/// Controls the detach ancestor behavior.
+/// - When set to `NoAncestorAndReparent`, we will only detach a branch if its ancestor is a root branch. It will automatically reparent any children of the ancestor before and at the branch point.
+/// - When set to `MultiLevelAndNoReparent`, we will detach a branch from multiple levels of ancestors, and no reparenting will happen at all.
+#[derive(Debug, Clone, Copy)]
+pub enum DetachBehavior {
+    NoAncestorAndReparent,
+    MultiLevelAndNoReparent,
+}
+
 impl Default for Options {
    fn default() -> Self {
        Self {
@@ -147,7 +161,8 @@ impl Default for Options {
 #[derive(Debug)]
 pub(crate) struct Attempt {
    pub(crate) timeline_id: TimelineId,
-
+    pub(crate) ancestor_timeline_id: TimelineId,
+    pub(crate) ancestor_lsn: Lsn,
    _guard: completion::Completion,
    gate_entered: Option<utils::sync::gate::GateGuard>,
 }
@@ -167,25 +182,30 @@ impl Attempt {
 pub(super) async fn prepare(
    detached: &Arc<Timeline>,
    tenant: &Tenant,
+    behavior: DetachBehavior,
    options: Options,
    ctx: &RequestContext,
 ) -> Result<Progress, Error> {
    use Error::*;

-    let Some((ancestor, ancestor_lsn)) = detached
+    let Some((mut ancestor, mut ancestor_lsn)) = detached
        .ancestor_timeline
        .as_ref()
        .map(|tl| (tl.clone(), detached.ancestor_lsn))
    else {
+        let ancestor_id;
+        let ancestor_lsn;
        let still_in_progress = {
            let accessor = detached.remote_client.initialized_upload_queue()?;

            // we are safe to inspect the latest uploaded, because we can only witness this after
            // restart is complete and ancestor is no more.
            let latest = accessor.latest_uploaded_index_part();
-            if latest.lineage.detached_previous_ancestor().is_none() {
+            let Some((id, lsn)) = latest.lineage.detached_previous_ancestor() else {
                return Err(NoAncestor);
            };
+            ancestor_id = id;
+            ancestor_lsn = lsn;

            latest
                .gc_blocking
@@ -196,7 +216,8 @@ pub(super) async fn prepare(
        if still_in_progress {
            // gc is still blocked, we can still reparent and complete.
            // we are safe to reparent remaining, because they were locked in in the beginning.
-            let attempt = continue_with_blocked_gc(detached, tenant).await?;
+            let attempt =
+                continue_with_blocked_gc(detached, tenant, ancestor_id, ancestor_lsn).await?;

            // because the ancestor of detached is already set to none, we have published all
            // of the layers, so we are still "prepared."
@@ -224,13 +245,34 @@ pub(super) async fn prepare(

    check_no_archived_children_of_ancestor(tenant, detached, &ancestor, ancestor_lsn)?;

-    if ancestor.ancestor_timeline.is_some() {
+    if let DetachBehavior::MultiLevelAndNoReparent = behavior {
+        // If the ancestor has an ancestor, we might be able to fast-path detach it if the current ancestor does not have any data written/used by the detaching timeline.
+        while let Some(ancestor_of_ancestor) = ancestor.ancestor_timeline.clone() {
+            if ancestor_lsn != ancestor.ancestor_lsn {
+                // non-technical requirement; we could flatten still if ancestor LSN does not match but that needs
+                // us to copy and cut more layers.
+                return Err(AncestorNotEmpty);
+            }
+            // Use the ancestor of the ancestor as the new ancestor (only when the ancestor LSNs are the same)
+            ancestor_lsn = ancestor.ancestor_lsn; // Get the LSN first before resetting the `ancestor` variable
+            ancestor = ancestor_of_ancestor;
+            // TODO: do we still need to check if we don't want to reparent?
+            check_no_archived_children_of_ancestor(tenant, detached, &ancestor, ancestor_lsn)?;
+        }
+    } else if ancestor.ancestor_timeline.is_some() {
        // non-technical requirement; we could flatten N ancestors just as easily but we chose
        // not to, at least initially
        return Err(TooManyAncestors);
    }

-    let attempt = start_new_attempt(detached, tenant).await?;
+    tracing::info!(
+        "attempt to detach the timeline from the ancestor: {}@{}, behavior={:?}",
+        ancestor.timeline_id,
+        ancestor_lsn,
+        behavior
+    );
+
+    let attempt = start_new_attempt(detached, tenant, ancestor.timeline_id, ancestor_lsn).await?;

    utils::pausable_failpoint!("timeline-detach-ancestor::before_starting_after_locking-pausable");

@@ -450,8 +492,13 @@ pub(super) async fn prepare(
    Ok(Progress::Prepared(attempt, prepared))
 }

-async fn start_new_attempt(detached: &Timeline, tenant: &Tenant) -> Result<Attempt, Error> {
-    let attempt = obtain_exclusive_attempt(detached, tenant)?;
+async fn start_new_attempt(
+    detached: &Timeline,
+    tenant: &Tenant,
+    ancestor_timeline_id: TimelineId,
+    ancestor_lsn: Lsn,
+) -> Result<Attempt, Error> {
+    let attempt = obtain_exclusive_attempt(detached, tenant, ancestor_timeline_id, ancestor_lsn)?;

    // insert the block in the index_part.json, if not already there.
    let _dont_care = tenant
@@ -466,13 +513,23 @@ async fn start_new_attempt(detached: &Timeline, tenant: &Tenant) -> Result<Attem
    Ok(attempt)
 }

-async fn continue_with_blocked_gc(detached: &Timeline, tenant: &Tenant) -> Result<Attempt, Error> {
+async fn continue_with_blocked_gc(
+    detached: &Timeline,
+    tenant: &Tenant,
+    ancestor_timeline_id: TimelineId,
+    ancestor_lsn: Lsn,
+) -> Result<Attempt, Error> {
    // FIXME: it would be nice to confirm that there is an in-memory version, since we've just
    // verified there is a persistent one?
-    obtain_exclusive_attempt(detached, tenant)
+    obtain_exclusive_attempt(detached, tenant, ancestor_timeline_id, ancestor_lsn)
 }

-fn obtain_exclusive_attempt(detached: &Timeline, tenant: &Tenant) -> Result<Attempt, Error> {
+fn obtain_exclusive_attempt(
+    detached: &Timeline,
+    tenant: &Tenant,
+    ancestor_timeline_id: TimelineId,
+    ancestor_lsn: Lsn,
+) -> Result<Attempt, Error> {
    use Error::{OtherTimelineDetachOngoing, ShuttingDown};

    // ensure we are the only active attempt for this tenant
@@ -493,6 +550,8 @@ fn obtain_exclusive_attempt(detached: &Timeline, tenant: &Tenant) -> Result<Atte

    Ok(Attempt {
        timeline_id: detached.timeline_id,
+        ancestor_timeline_id,
+        ancestor_lsn,
        _guard: guard,
        gate_entered: Some(_gate_entered),
    })
@@ -795,6 +854,9 @@ pub(super) async fn detach_and_reparent(
    detached: &Arc<Timeline>,
    tenant: &Tenant,
    prepared: PreparedTimelineDetach,
+    ancestor_timeline_id: TimelineId,
+    ancestor_lsn: Lsn,
+    behavior: DetachBehavior,
    _ctx: &RequestContext,
 ) -> Result<DetachingAndReparenting, Error> {
    let PreparedTimelineDetach { layers } = prepared;
@@ -822,7 +884,30 @@ pub(super) async fn detach_and_reparent(
        "cannot (detach? reparent)? complete if the operation is not still ongoing"
    );

-    let ancestor = match (detached.ancestor_timeline.as_ref(), recorded_branchpoint) {
+    let ancestor_to_detach = match detached.ancestor_timeline.as_ref() {
+        Some(mut ancestor) => {
+            while ancestor.timeline_id != ancestor_timeline_id {
+                match ancestor.ancestor_timeline.as_ref() {
+                    Some(found) => {
+                        if ancestor_lsn != ancestor.ancestor_lsn {
+                            return Err(Error::DetachReparent(anyhow::anyhow!(
+                                "cannot find the ancestor timeline to detach from: wrong ancestor lsn"
+                            )));
+                        }
+                        ancestor = found;
+                    }
+                    None => {
+                        return Err(Error::DetachReparent(anyhow::anyhow!(
+                            "cannot find the ancestor timeline to detach from"
+                        )));
+                    }
+                }
+            }
+            Some(ancestor)
+        }
+        None => None,
+    };
+    let ancestor = match (ancestor_to_detach, recorded_branchpoint) {
        (Some(ancestor), None) => {
            assert!(
                !layers.is_empty(),
@@ -895,6 +980,11 @@ pub(super) async fn detach_and_reparent(
        Ancestor::Detached(ancestor, ancestor_lsn) => (ancestor, ancestor_lsn, false),
    };

+    if let DetachBehavior::MultiLevelAndNoReparent = behavior {
+        // Do not reparent if the user requests to behave so.
+        return Ok(DetachingAndReparenting::Reparented(HashSet::new()));
+    }
+
    let mut tasks = tokio::task::JoinSet::new();

    // Returns a single permit semaphore which will be used to make one reparenting succeed,
@@ -1032,6 +1122,11 @@ pub(super) async fn complete(
 }

 /// Query against a locked `Tenant::timelines`.
+///
+/// A timeline is reparentable if:
+///
+/// - It is not the timeline being detached.
+/// - It has the same ancestor as the timeline being detached. Note that the ancestor might not be the direct ancestor.
 fn reparentable_timelines<'a, I>(
    timelines: I,
    detached: &'a Arc<Timeline>,
--- a/storage_controller/src/http.rs
+++ b/storage_controller/src/http.rs
@@ -543,6 +543,30 @@ async fn handle_tenant_timeline_detach_ancestor(
    json_response(StatusCode::OK, res)
 }

+async fn handle_tenant_timeline_detach_ancestor_v2(
+    service: Arc<Service>,
+    req: Request<Body>,
+) -> Result<Response<Body>, ApiError> {
+    let tenant_id: TenantId = parse_request_param(&req, "tenant_id")?;
+    let timeline_id: TimelineId = parse_request_param(&req, "timeline_id")?;
+
+    check_permissions(&req, Scope::PageServerApi)?;
+    maybe_rate_limit(&req, tenant_id).await;
+
+    match maybe_forward(req).await {
+        ForwardOutcome::Forwarded(res) => {
+            return res;
+        }
+        ForwardOutcome::NotForwarded(_req) => {}
+    };
+
+    let res = service
+        .tenant_timeline_detach_ancestor_v2(tenant_id, timeline_id)
+        .await?;
+
+    json_response(StatusCode::OK, res)
+}
+
 async fn handle_tenant_timeline_block_unblock_gc(
    service: Arc<Service>,
    req: Request<Body>,
@@ -2162,6 +2186,16 @@ pub fn make_router(
                )
            },
        )
+        .put(
+            "/v1/tenant/:tenant_id/timeline/:timeline_id/detach_ancestor_v2",
+            |r| {
+                tenant_service_handler(
+                    r,
+                    handle_tenant_timeline_detach_ancestor_v2,
+                    RequestName("v1_tenant_timeline_detach_ancestor_v2"),
+                )
+            },
+        )
        .post(
            "/v1/tenant/:tenant_id/timeline/:timeline_id/block_gc",
            |r| {
--- a/storage_controller/src/pageserver_client.rs
+++ b/storage_controller/src/pageserver_client.rs
@@ -263,6 +263,21 @@ impl PageserverClient {
        )
    }

+    pub(crate) async fn timeline_detach_ancestor_v2(
+        &self,
+        tenant_shard_id: TenantShardId,
+        timeline_id: TimelineId,
+    ) -> Result<AncestorDetached> {
+        measured_request!(
+            "timeline_detach_ancestor_v2",
+            crate::metrics::Method::Put,
+            &self.node_id_label,
+            self.inner
+                .timeline_detach_ancestor_v2(tenant_shard_id, timeline_id)
+                .await
+        )
+    }
+
    pub(crate) async fn timeline_block_unblock_gc(
        &self,
        tenant_shard_id: TenantShardId,
--- a/storage_controller/src/service.rs
+++ b/storage_controller/src/service.rs
@@ -3668,6 +3668,25 @@ impl Service {
        &self,
        tenant_id: TenantId,
        timeline_id: TimelineId,
+    ) -> Result<models::detach_ancestor::AncestorDetached, ApiError> {
+        self.tenant_timeline_detach_ancestor_inner(tenant_id, timeline_id, false)
+            .await
+    }
+
+    pub(crate) async fn tenant_timeline_detach_ancestor_v2(
+        &self,
+        tenant_id: TenantId,
+        timeline_id: TimelineId,
+    ) -> Result<models::detach_ancestor::AncestorDetached, ApiError> {
+        self.tenant_timeline_detach_ancestor_inner(tenant_id, timeline_id, true)
+            .await
+    }
+
+    pub(crate) async fn tenant_timeline_detach_ancestor_inner(
+        &self,
+        tenant_id: TenantId,
+        timeline_id: TimelineId,
+        detach_v2_behavior: bool,
    ) -> Result<models::detach_ancestor::AncestorDetached, ApiError> {
        tracing::info!("Detaching timeline {tenant_id}/{timeline_id}",);

@@ -3691,6 +3710,7 @@ impl Service {
                node: Node,
                jwt: Option<String>,
                ssl_ca_cert: Option<Certificate>,
+                detach_v2_behavior: bool,
            ) -> Result<(ShardNumber, models::detach_ancestor::AncestorDetached), ApiError> {
                tracing::info!(
                    "Detaching timeline on shard {tenant_shard_id}/{timeline_id}, attached to node {node}",
@@ -3699,10 +3719,17 @@ impl Service {
                let client = PageserverClient::new(node.get_id(), node.base_url(), jwt.as_deref(), ssl_ca_cert)
                    .map_err(|e| passthrough_api_error(&node, e))?;

-                client
-                    .timeline_detach_ancestor(tenant_shard_id, timeline_id)
-                    .await
-                    .map_err(|e| {
+                let res = if detach_v2_behavior {
+                        client
+                            .timeline_detach_ancestor_v2(tenant_shard_id, timeline_id)
+                            .await
+                    } else {
+                        client
+                            .timeline_detach_ancestor(tenant_shard_id, timeline_id)
+                            .await
+                    };
+
+                    res.map_err(|e| {
                        use mgmt_api::Error;

                        match e {
@@ -3738,6 +3765,7 @@ impl Service {
                        node,
                        self.config.pageserver_jwt_token.clone(),
                        self.config.ssl_ca_cert.clone(),
+                        detach_v2_behavior
                    ))
                })
                .await?;
--- a/test_runner/fixtures/pageserver/http.py
+++ b/test_runner/fixtures/pageserver/http.py
@@ -1070,13 +1070,15 @@ class PageserverHttpClient(requests.Session, MetricsGetter):
        tenant_id: TenantId | TenantShardId,
        timeline_id: TimelineId,
        batch_size: int | None = None,
+        behavior_v2: bool = False,
        **kwargs,
    ) -> set[TimelineId]:
        params = {}
        if batch_size is not None:
            params["batch_size"] = batch_size
+        endpoint = "detach_ancestor" if not behavior_v2 else "detach_ancestor_v2"
        res = self.put(
-            f"http://localhost:{self.port}/v1/tenant/{tenant_id}/timeline/{timeline_id}/detach_ancestor",
+            f"http://localhost:{self.port}/v1/tenant/{tenant_id}/timeline/{timeline_id}/{endpoint}",
            params=params,
            **kwargs,
        )
--- a/test_runner/regress/test_timeline_detach_ancestor.py
+++ b/test_runner/regress/test_timeline_detach_ancestor.py
@@ -319,8 +319,9 @@ def test_ancestor_detach_reparents_earlier(neon_env_builder: NeonEnvBuilder):
            # this does not contain Z in the end, so fromisoformat accepts it
            # it is to be in line with the deletion timestamp.. well, almost.
            when = original_ancestor[2][:26]
-            when_ts = datetime.datetime.fromisoformat(when)
-            assert when_ts < datetime.datetime.now()
+            when_ts = datetime.datetime.fromisoformat(when).replace(tzinfo=datetime.UTC)
+            now = datetime.datetime.utcnow().replace(tzinfo=datetime.UTC)
+            assert when_ts < now
            assert len(lineage.get("reparenting_history", [])) == 0
        elif expected_ancestor == timeline_id:
            assert len(lineage.get("original_ancestor", [])) == 0
@@ -342,6 +343,138 @@ def test_ancestor_detach_reparents_earlier(neon_env_builder: NeonEnvBuilder):
    wait_timeline_detail_404(client, env.initial_tenant, env.initial_timeline)


+def test_ancestor_detach_behavior_v2(neon_env_builder: NeonEnvBuilder):
+    """
+    Test the v2 behavior of ancestor detach.
+
+    old main -------|---------X--------->
+                    |         |         |
+                    |         |         +-> after
+                    |         +--X empty snapshot branch
+                    |            |
+                    |            +-> branch-to-detach
+                    |
+                    +-> earlier
+
+    Ends up as:
+
+    old main -------|---------X--------->
+                    |         |         |
+                    |         |         +-> after
+                    |         +--X empty snapshot branch
+                    |
+                    +-> earlier
+
+
+    new main -------|---------|----> branch-to-detach
+    """
+
+    env = neon_env_builder.init_start()
+
+    env.pageserver.allowed_errors.extend(SHUTDOWN_ALLOWED_ERRORS)
+
+    client = env.pageserver.http_client()
+
+    with env.endpoints.create_start("main", tenant_id=env.initial_tenant) as ep:
+        ep.safe_psql("CREATE TABLE foo (i BIGINT);")
+        ep.safe_psql("CREATE TABLE audit AS SELECT 1 as starts;")
+
+        branchpoint_pipe = wait_for_last_flush_lsn(
+            env, ep, env.initial_tenant, env.initial_timeline
+        )
+
+        ep.safe_psql("INSERT INTO foo SELECT i::bigint FROM generate_series(0, 8191) g(i);")
+
+        branchpoint_x = wait_for_last_flush_lsn(env, ep, env.initial_tenant, env.initial_timeline)
+        client.timeline_checkpoint(env.initial_tenant, env.initial_timeline)
+
+        ep.safe_psql("INSERT INTO foo SELECT i::bigint FROM generate_series(8192, 16383) g(i);")
+        wait_for_last_flush_lsn(env, ep, env.initial_tenant, env.initial_timeline)
+
+    earlier = env.create_branch(
+        "earlier", ancestor_branch_name="main", ancestor_start_lsn=branchpoint_pipe
+    )
+
+    snapshot_branchpoint = env.create_branch(
+        "snapshot_branchpoint", ancestor_branch_name="main", ancestor_start_lsn=branchpoint_x
+    )
+
+    branch_to_detach = env.create_branch(
+        "branch_to_detach",
+        ancestor_branch_name="snapshot_branchpoint",
+        ancestor_start_lsn=branchpoint_x,
+    )
+
+    after = env.create_branch("after", ancestor_branch_name="main", ancestor_start_lsn=None)
+
+    all_reparented = client.detach_ancestor(env.initial_tenant, branch_to_detach, behavior_v2=True)
+    assert set(all_reparented) == set()
+
+    env.pageserver.quiesce_tenants()
+
+    # checking the ancestor after is much faster than waiting for the endpoint not start
+    expected_result = [
+        ("main", env.initial_timeline, None, 16384, 1),
+        ("after", after, env.initial_timeline, 16384, 1),
+        ("snapshot_branchpoint", snapshot_branchpoint, env.initial_timeline, 8192, 1),
+        ("branch_to_detach", branch_to_detach, None, 8192, 1),
+        ("earlier", earlier, env.initial_timeline, 0, 1),
+    ]
+
+    assert isinstance(env.pageserver_remote_storage, LocalFsStorage)
+
+    for branch_name, queried_timeline, expected_ancestor, _, _ in expected_result:
+        details = client.timeline_detail(env.initial_tenant, queried_timeline)
+        ancestor_timeline_id = details["ancestor_timeline_id"]
+        if expected_ancestor is None:
+            assert ancestor_timeline_id is None
+        else:
+            assert (
+                TimelineId(ancestor_timeline_id) == expected_ancestor
+            ), f"when checking branch {branch_name}, mapping={expected_result}"
+
+        index_part = env.pageserver_remote_storage.index_content(
+            env.initial_tenant, queried_timeline
+        )
+        lineage = index_part["lineage"]
+        assert lineage is not None
+
+        assert lineage.get("reparenting_history_overflown", "false") == "false"
+
+        if queried_timeline == branch_to_detach:
+            original_ancestor = lineage["original_ancestor"]
+            assert original_ancestor is not None
+            assert original_ancestor[0] == str(env.initial_timeline)
+            assert original_ancestor[1] == str(branchpoint_x)
+
+            # this does not contain Z in the end, so fromisoformat accepts it
+            # it is to be in line with the deletion timestamp.. well, almost.
+            when = original_ancestor[2][:26]
+            when_ts = datetime.datetime.fromisoformat(when).replace(tzinfo=datetime.UTC)
+            now = datetime.datetime.utcnow().replace(tzinfo=datetime.UTC)
+            assert when_ts < now
+            assert len(lineage.get("reparenting_history", [])) == 0
+        elif expected_ancestor == branch_to_detach:
+            assert len(lineage.get("original_ancestor", [])) == 0
+            assert lineage["reparenting_history"] == [str(env.initial_timeline)]
+        else:
+            assert len(lineage.get("original_ancestor", [])) == 0
+            assert len(lineage.get("reparenting_history", [])) == 0
+
+    for name, _, _, rows, starts in expected_result:
+        with env.endpoints.create_start(name, tenant_id=env.initial_tenant) as ep:
+            assert ep.safe_psql("SELECT count(*) FROM foo;")[0][0] == rows
+            assert ep.safe_psql(f"SELECT count(*) FROM audit WHERE starts = {starts}")[0][0] == 1
+
+    # delete the new timeline to confirm it doesn't carry over the anything from the old timeline
+    client.timeline_delete(env.initial_tenant, branch_to_detach)
+    wait_timeline_detail_404(client, env.initial_tenant, branch_to_detach)
+
+    # delete the after timeline
+    client.timeline_delete(env.initial_tenant, after)
+    wait_timeline_detail_404(client, env.initial_tenant, after)
+
+
 def test_detached_receives_flushes_while_being_detached(neon_env_builder: NeonEnvBuilder):
    """
    Makes sure that the timeline is able to receive writes through-out the detach process.
Author	SHA1	Message	Date
Alex Chi Z	cd32bb43a0	better wording Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-12 12:33:14 -04:00
Alex Chi Z	9a2068af56	feat(storcon): passthrough detach v2 requests Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-12 11:57:02 -04:00
Alex Chi Z	08babb4f50	resolve comments Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-12 11:40:50 -04:00
Alex Chi Z	ac8b4048fd	fix err msg Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-11 15:15:56 -04:00
Alex Chi Z	5ea3662494	fix ancestor behavior Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-11 14:20:44 -04:00
Alex Chi Z	d29d506b63	no reparenting for behavior v2 Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-11 13:38:25 -04:00
Alex Chi Z	b225b336e8	handle ts correctly Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-10 16:01:43 -04:00
Alex Chi Z	7d6c9d0557	feat(pageserver): support detaching from multiple ancestors Signed-off-by: Alex Chi Z <chi@neon.tech>	2025-03-10 15:45:14 -04:00