diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index b3026fe3a7..db5b77a4d9 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -232,23 +232,32 @@ impl Repository for LayeredRepository { fn create_empty_timeline( &self, - timelineid: ZTimelineId, + timeline_id: ZTimelineId, initdb_lsn: Lsn, ) -> Result> { let mut timelines = self.timelines.lock().unwrap(); + let vacant_timeline_entry = match timelines.entry(timeline_id) { + Entry::Occupied(_) => bail!("Timeline already exists"), + Entry::Vacant(vacant_entry) => vacant_entry, + }; + + let timeline_path = self.conf.timeline_path(&timeline_id, &self.tenant_id); + if timeline_path.exists() { + bail!("Timeline directory already exists, but timeline is missing in repository map. This is a bug.") + } // Create the timeline directory, and write initial metadata to file. - crashsafe_dir::create_dir_all(self.conf.timeline_path(&timelineid, &self.tenant_id))?; + crashsafe_dir::create_dir_all(timeline_path)?; let metadata = TimelineMetadata::new(Lsn(0), None, None, Lsn(0), initdb_lsn, initdb_lsn); - Self::save_metadata(self.conf, timelineid, self.tenant_id, &metadata, true)?; + Self::save_metadata(self.conf, timeline_id, self.tenant_id, &metadata, true)?; let timeline = LayeredTimeline::new( self.conf, Arc::clone(&self.tenant_conf), metadata, None, - timelineid, + timeline_id, self.tenant_id, Arc::clone(&self.walredo_mgr), self.upload_layers, @@ -257,12 +266,7 @@ impl Repository for LayeredRepository { // Insert if not exists let timeline = Arc::new(timeline); - match timelines.entry(timelineid) { - Entry::Occupied(_) => bail!("Timeline already exists"), - Entry::Vacant(vacant) => { - vacant.insert(LayeredTimelineEntry::Loaded(Arc::clone(&timeline))) - } - }; + vacant_timeline_entry.insert(LayeredTimelineEntry::Loaded(Arc::clone(&timeline))); Ok(timeline) } diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 756c3b8191..9501a416b4 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -225,7 +225,7 @@ pub trait Repository: Send + Sync { /// Initdb lsn is provided for timeline impl to be able to perform checks for some operations against it. fn create_empty_timeline( &self, - timelineid: ZTimelineId, + timeline_id: ZTimelineId, initdb_lsn: Lsn, ) -> Result>; @@ -636,6 +636,19 @@ mod tests { Ok(()) } + #[test] + fn no_duplicate_timelines() -> Result<()> { + let repo = RepoHarness::create("no_duplicate_timelines")?.load(); + let _ = repo.create_empty_timeline(TIMELINE_ID, Lsn(0))?; + + match repo.create_empty_timeline(TIMELINE_ID, Lsn(0)) { + Ok(_) => panic!("duplicate timeline creation should fail"), + Err(e) => assert_eq!(e.to_string(), "Timeline already exists"), + } + + Ok(()) + } + /// Convenience function to create a page image with given string as the only content pub fn test_value(s: &str) -> Value { let mut buf = BytesMut::new();