From 7a840ec60ca7248625a9e88fbddcf14ca67207ed Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sat, 27 Aug 2022 18:14:40 +0300 Subject: [PATCH] Move save_metadata function. `timeline.rs` seems like a better home for it. --- pageserver/src/layered_repository.rs | 6 +- pageserver/src/layered_repository/metadata.rs | 64 +++++++++++++++---- pageserver/src/layered_repository/timeline.rs | 41 +----------- 3 files changed, 56 insertions(+), 55 deletions(-) diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index fae52c3daf..36b8e3eb9e 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -69,7 +69,7 @@ pub use timeline::Timeline; pub use crate::layered_repository::ephemeral_file::writeback as writeback_ephemeral_file; // re-export for use in storage_sync.rs -pub use crate::layered_repository::timeline::save_metadata; +pub use crate::layered_repository::metadata::save_metadata; // re-export for use in walreceiver pub use crate::layered_repository::timeline::WalReceiverInfo; @@ -185,7 +185,7 @@ impl Repository { crashsafe_dir::create_dir_all(timeline_path)?; let metadata = TimelineMetadata::new(Lsn(0), None, None, Lsn(0), initdb_lsn, initdb_lsn); - timeline::save_metadata(self.conf, timeline_id, self.tenant_id, &metadata, true)?; + save_metadata(self.conf, timeline_id, self.tenant_id, &metadata, true)?; let timeline = Timeline::new( self.conf, @@ -294,7 +294,7 @@ impl Repository { src_timeline.initdb_lsn, ); crashsafe_dir::create_dir_all(self.conf.timeline_path(&dst, &self.tenant_id))?; - timeline::save_metadata(self.conf, dst, self.tenant_id, &metadata, true)?; + save_metadata(self.conf, dst, self.tenant_id, &metadata, true)?; timelines.insert(dst, LayeredTimelineEntry::Unloaded { id: dst, metadata }); info!("branched timeline {} from {} at {}", dst, src, start_lsn); diff --git a/pageserver/src/layered_repository/metadata.rs b/pageserver/src/layered_repository/metadata.rs index f3ddd42e76..910dba4644 100644 --- a/pageserver/src/layered_repository/metadata.rs +++ b/pageserver/src/layered_repository/metadata.rs @@ -6,10 +6,13 @@ //! //! The module contains all structs and related helper methods related to timeline metadata. +use std::fs::{File, OpenOptions}; +use std::io::Write; use std::path::PathBuf; -use anyhow::ensure; +use anyhow::{bail, ensure, Context}; use serde::{Deserialize, Serialize}; +use tracing::info_span; use utils::{ bin_ser::BeSer, lsn::Lsn, @@ -17,6 +20,7 @@ use utils::{ }; use crate::config::PageServerConf; +use crate::virtual_file::VirtualFile; use crate::STORAGE_FORMAT_VERSION; /// We assume that a write of up to METADATA_MAX_SIZE bytes is atomic. @@ -65,17 +69,6 @@ struct TimelineMetadataBody { initdb_lsn: Lsn, } -/// Points to a place in pageserver's local directory, -/// where certain timeline's metadata file should be located. -pub fn metadata_path( - conf: &'static PageServerConf, - timelineid: ZTimelineId, - tenantid: ZTenantId, -) -> PathBuf { - conf.timeline_path(&timelineid, &tenantid) - .join(METADATA_FILE_NAME) -} - impl TimelineMetadata { pub fn new( disk_consistent_lsn: Lsn, @@ -173,6 +166,53 @@ impl TimelineMetadata { } } +/// Points to a place in pageserver's local directory, +/// where certain timeline's metadata file should be located. +pub fn metadata_path( + conf: &'static PageServerConf, + timelineid: ZTimelineId, + tenantid: ZTenantId, +) -> PathBuf { + conf.timeline_path(&timelineid, &tenantid) + .join(METADATA_FILE_NAME) +} + +/// Save timeline metadata to file +pub fn save_metadata( + conf: &'static PageServerConf, + timelineid: ZTimelineId, + tenantid: ZTenantId, + data: &TimelineMetadata, + first_save: bool, +) -> anyhow::Result<()> { + let _enter = info_span!("saving metadata").entered(); + let path = metadata_path(conf, timelineid, tenantid); + // use OpenOptions to ensure file presence is consistent with first_save + let mut file = VirtualFile::open_with_options( + &path, + OpenOptions::new().write(true).create_new(first_save), + )?; + + let metadata_bytes = data.to_bytes().context("Failed to get metadata bytes")?; + + if file.write(&metadata_bytes)? != metadata_bytes.len() { + bail!("Could not write all the metadata bytes in a single call"); + } + file.sync_all()?; + + // fsync the parent directory to ensure the directory entry is durable + if first_save { + let timeline_dir = File::open( + &path + .parent() + .expect("Metadata should always have a parent dir"), + )?; + timeline_dir.sync_all()?; + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/pageserver/src/layered_repository/timeline.rs b/pageserver/src/layered_repository/timeline.rs index ecf9a87500..5f3d669dc1 100644 --- a/pageserver/src/layered_repository/timeline.rs +++ b/pageserver/src/layered_repository/timeline.rs @@ -11,8 +11,6 @@ use tracing::*; use std::cmp::{max, min, Ordering}; use std::collections::{HashMap, HashSet}; use std::fs; -use std::fs::{File, OpenOptions}; -use std::io::Write; use std::ops::{Deref, Range}; use std::path::PathBuf; use std::sync::atomic::{self, AtomicBool, AtomicI64, Ordering as AtomicOrdering}; @@ -32,7 +30,7 @@ use crate::layered_repository::{ image_layer::{ImageLayer, ImageLayerWriter}, inmemory_layer::InMemoryLayer, layer_map::{LayerMap, SearchResult}, - metadata::{metadata_path, TimelineMetadata, METADATA_FILE_NAME}, + metadata::{save_metadata, TimelineMetadata, METADATA_FILE_NAME}, par_fsync, storage_layer::{Layer, ValueReconstructResult, ValueReconstructState}, }; @@ -54,7 +52,6 @@ use utils::{ use crate::repository::{GcResult, RepositoryTimeline}; use crate::repository::{Key, Value}; use crate::thread_mgr; -use crate::virtual_file::VirtualFile; use crate::walreceiver::IS_WAL_RECEIVER; use crate::walredo::WalRedoManager; use crate::CheckpointConfig; @@ -2342,39 +2339,3 @@ fn rename_to_backup(path: PathBuf) -> anyhow::Result<()> { bail!("couldn't find an unused backup number for {:?}", path) } - -/// Save timeline metadata to file -pub fn save_metadata( - conf: &'static PageServerConf, - timelineid: ZTimelineId, - tenantid: ZTenantId, - data: &TimelineMetadata, - first_save: bool, -) -> Result<()> { - let _enter = info_span!("saving metadata").entered(); - let path = metadata_path(conf, timelineid, tenantid); - // use OpenOptions to ensure file presence is consistent with first_save - let mut file = VirtualFile::open_with_options( - &path, - OpenOptions::new().write(true).create_new(first_save), - )?; - - let metadata_bytes = data.to_bytes().context("Failed to get metadata bytes")?; - - if file.write(&metadata_bytes)? != metadata_bytes.len() { - bail!("Could not write all the metadata bytes in a single call"); - } - file.sync_all()?; - - // fsync the parent directory to ensure the directory entry is durable - if first_save { - let timeline_dir = File::open( - &path - .parent() - .expect("Metadata should always have a parent dir"), - )?; - timeline_dir.sync_all()?; - } - - Ok(()) -}