diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index 55f01ee962..392aa52844 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -270,7 +270,12 @@ impl Repository for LayeredRepository { } /// Branch a timeline - fn branch_timeline(&self, src: ZTimelineId, dst: ZTimelineId, start_lsn: Lsn) -> Result<()> { + fn branch_timeline( + &self, + src: ZTimelineId, + dst: ZTimelineId, + start_lsn: Option, + ) -> Result<()> { // We need to hold this lock to prevent GC from starting at the same time. GC scans the directory to learn // about timelines, so otherwise a race condition is possible, where we create new timeline and GC // concurrently removes data that is needed by the new timeline. @@ -283,6 +288,14 @@ impl Repository for LayeredRepository { .context("failed to load timeline for branching")? .ok_or_else(|| anyhow::anyhow!("unknown timeline id: {}", &src))?; let latest_gc_cutoff_lsn = src_timeline.get_latest_gc_cutoff_lsn(); + + // If no start LSN is specified, we branch the new timeline from the source timeline's last record LSN + let start_lsn = start_lsn.unwrap_or_else(|| { + let lsn = src_timeline.get_last_record_lsn(); + info!("branching timeline {dst} from timeline {src} at last record LSN: {lsn}"); + lsn + }); + src_timeline .check_lsn_is_in_scope(start_lsn, &latest_gc_cutoff_lsn) .context("invalid branch start lsn")?; @@ -2874,7 +2887,7 @@ pub mod tests { let mut tline_id = TIMELINE_ID; for _ in 0..50 { let new_tline_id = ZTimelineId::generate(); - repo.branch_timeline(tline_id, new_tline_id, lsn)?; + repo.branch_timeline(tline_id, new_tline_id, Some(lsn))?; tline = repo.get_timeline_load(new_tline_id)?; tline_id = new_tline_id; @@ -2933,7 +2946,7 @@ pub mod tests { #[allow(clippy::needless_range_loop)] for idx in 0..NUM_TLINES { let new_tline_id = ZTimelineId::generate(); - repo.branch_timeline(tline_id, new_tline_id, lsn)?; + repo.branch_timeline(tline_id, new_tline_id, Some(lsn))?; tline = repo.get_timeline_load(new_tline_id)?; tline_id = new_tline_id; diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 5b28681b16..17e8806899 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -211,7 +211,12 @@ pub trait Repository: Send + Sync { ) -> Result>; /// Branch a timeline - fn branch_timeline(&self, src: ZTimelineId, dst: ZTimelineId, start_lsn: Lsn) -> Result<()>; + fn branch_timeline( + &self, + src: ZTimelineId, + dst: ZTimelineId, + start_lsn: Option, + ) -> Result<()>; /// Flush all data to disk. /// @@ -662,7 +667,7 @@ mod tests { //assert_current_logical_size(&tline, Lsn(0x40)); // Branch the history, modify relation differently on the new timeline - repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x30))?; + repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x30)))?; let newtline = repo .get_timeline_load(NEW_TIMELINE_ID) .expect("Should have a local timeline"); @@ -744,7 +749,7 @@ mod tests { repo.gc_iteration(Some(TIMELINE_ID), 0x10, Duration::ZERO, false)?; // try to branch at lsn 25, should fail because we already garbage collected the data - match repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x25)) { + match repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x25))) { Ok(_) => panic!("branching should have failed"), Err(err) => { assert!(err.to_string().contains("invalid branch start lsn")); @@ -765,7 +770,7 @@ mod tests { repo.create_empty_timeline(TIMELINE_ID, Lsn(0x50))?; // try to branch at lsn 0x25, should fail because initdb lsn is 0x50 - match repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x25)) { + match repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x25))) { Ok(_) => panic!("branching should have failed"), Err(err) => { assert!(&err.to_string().contains("invalid branch start lsn")); @@ -810,7 +815,7 @@ mod tests { let tline = repo.create_empty_timeline(TIMELINE_ID, Lsn(0))?; make_some_layers(tline.as_ref(), Lsn(0x20))?; - repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x40))?; + repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x40)))?; let newtline = repo .get_timeline_load(NEW_TIMELINE_ID) .expect("Should have a local timeline"); @@ -826,7 +831,7 @@ mod tests { let tline = repo.create_empty_timeline(TIMELINE_ID, Lsn(0))?; make_some_layers(tline.as_ref(), Lsn(0x20))?; - repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x40))?; + repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x40)))?; let newtline = repo .get_timeline_load(NEW_TIMELINE_ID) .expect("Should have a local timeline"); @@ -884,7 +889,7 @@ mod tests { make_some_layers(tline.as_ref(), Lsn(0x20))?; tline.checkpoint(CheckpointConfig::Forced)?; - repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x40))?; + repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Some(Lsn(0x40)))?; let newtline = repo .get_timeline_load(NEW_TIMELINE_ID) diff --git a/pageserver/src/timelines.rs b/pageserver/src/timelines.rs index e0e79e4166..a40e705cb9 100644 --- a/pageserver/src/timelines.rs +++ b/pageserver/src/timelines.rs @@ -347,7 +347,7 @@ pub(crate) fn create_timeline( tenant_id: ZTenantId, new_timeline_id: Option, ancestor_timeline_id: Option, - ancestor_start_lsn: Option, + mut ancestor_start_lsn: Option, ) -> Result> { let new_timeline_id = new_timeline_id.unwrap_or_else(ZTimelineId::generate); let repo = tenant_mgr::get_repository_for_tenant(tenant_id)?; @@ -357,41 +357,35 @@ pub(crate) fn create_timeline( return Ok(None); } - let mut start_lsn = ancestor_start_lsn.unwrap_or(Lsn(0)); - let new_timeline_info = match ancestor_timeline_id { Some(ancestor_timeline_id) => { let ancestor_timeline = repo .get_timeline_load(ancestor_timeline_id) .context("Cannot branch off the timeline that's not present locally")?; - if start_lsn == Lsn(0) { - // Find end of WAL on the old timeline - let end_of_wal = ancestor_timeline.get_last_record_lsn(); - info!("branching at end of WAL: {}", end_of_wal); - start_lsn = end_of_wal; - } else { + if let Some(lsn) = ancestor_start_lsn.as_mut() { // Wait for the WAL to arrive and be processed on the parent branch up // to the requested branch point. The repository code itself doesn't // require it, but if we start to receive WAL on the new timeline, // decoding the new WAL might need to look up previous pages, relation // sizes etc. and that would get confused if the previous page versions // are not in the repository yet. - ancestor_timeline.wait_lsn(start_lsn)?; - } - start_lsn = start_lsn.align(); + *lsn = lsn.align(); + ancestor_timeline.wait_lsn(*lsn)?; - let ancestor_ancestor_lsn = ancestor_timeline.get_ancestor_lsn(); - if ancestor_ancestor_lsn > start_lsn { - // can we safely just branch from the ancestor instead? - anyhow::bail!( + let ancestor_ancestor_lsn = ancestor_timeline.get_ancestor_lsn(); + if ancestor_ancestor_lsn > *lsn { + // can we safely just branch from the ancestor instead? + anyhow::bail!( "invalid start lsn {} for ancestor timeline {}: less than timeline ancestor lsn {}", - start_lsn, + lsn, ancestor_timeline_id, ancestor_ancestor_lsn, ); + } } - repo.branch_timeline(ancestor_timeline_id, new_timeline_id, start_lsn)?; + + repo.branch_timeline(ancestor_timeline_id, new_timeline_id, ancestor_start_lsn)?; // load the timeline into memory let loaded_timeline = tenant_mgr::get_local_timeline_with_load(tenant_id, new_timeline_id)?;