From ee6fa35e5bed07995d5e244c504b73e3ad7a8818 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Wed, 1 Feb 2023 20:07:10 +0100 Subject: [PATCH] well-defined marker file contents to support storing TenantConfOpt in the future --- pageserver/src/tenant.rs | 150 +++++++++++++++++++++++++++++++-------- 1 file changed, 122 insertions(+), 28 deletions(-) diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index ab63cd3f13..7756df99ed 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -19,6 +19,7 @@ use futures::TryFutureExt; use pageserver_api::models::TimelineState; use remote_storage::DownloadError; use remote_storage::GenericRemoteStorage; +use scopeguard::ScopeGuard; use tokio::sync::watch; use tokio::task::JoinSet; use tracing::*; @@ -115,6 +116,19 @@ pub const TIMELINES_SEGMENT_NAME: &str = "timelines"; pub const TENANT_ATTACHING_LEGACY_MARKER_FILENAME: &str = "attaching"; pub const TENANT_ATTACHING_MARKER_SUFFIX: &str = "___attaching"; +#[derive(Debug, serde::Serialize, serde::Deserialize)] +struct AttachMarkerFileContentsV1 { + format_version: u32, +} + +impl AttachMarkerFileContentsV1 { + fn new() -> Self { + Self { format_version: 1 } + } +} + +type AttachMarkerFileContents = AttachMarkerFileContentsV1; + /// The error message does not include the tenant ID. /// The caller is responsible for adding it. #[derive(Debug, thiserror::Error)] @@ -126,10 +140,17 @@ pub enum AttachError { op: &'static str, path: PathBuf, #[source] - error: std::io::Error, + error: Box, }, #[error("marker file is missing: {path:?}")] MarkerFileMissing { path: PathBuf }, + #[error("{op} marker file {path:?}: {error:#}")] + ReadMarkerFile { + op: &'static str, + path: PathBuf, + #[source] + error: Box, + }, } /// Attach invokes the load routine after it has set up the local filesystem state. @@ -690,6 +711,96 @@ impl Tenant { Ok(()) } + /// Create the attach marker file, i.e., `/tenants/${tenant_id}___attaching`. + /// If we fail during the creating process, this function returns an error and tries + /// to remove the unfinished marker. It logs a warn-level message if that cleanup fails. + fn create_attach_marker_file(path: &Path) -> Result<(), AttachError> { + let contents = AttachMarkerFileContents::new(); + // Serialize before creating the file. + let contents_string = + serde_json::to_string(&contents).map_err(|error| AttachError::CreateMarkerFile { + op: "serialize contents of", + path: path.to_owned(), + error: Box::new(error), + })?; + // Atomically create the file, fail if it already exists. + let mut file = match std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(path) + { + Ok(file) => file, + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + return Err(AttachError::LocalStateExists { + what: "marker file", + path: path.to_owned(), + }); + } + Err(error) => { + return Err(AttachError::CreateMarkerFile { + op: "create", + path: path.to_owned(), + error: Box::new(error), + }); + } + }; + // If we fail in any of the steps starting from here, try to clean up. + let guard = scopeguard::guard((), |()| { + if let Err(e) = std::fs::remove_file(path) { + warn!("failed to remove attach marker {path:?}: {e:#}"); + } + }); + file.write_all(contents_string.as_bytes()) + .map_err(|error| AttachError::CreateMarkerFile { + op: "write", + path: path.to_owned(), + error: Box::new(error), + })?; + crashsafe::fsync_file_and_parent(path).map_err(|error| AttachError::CreateMarkerFile { + op: "fsync_file_and_parent", + path: path.to_owned(), + error: Box::new(error), + })?; + // All good, don't remove the file upon return! + ScopeGuard::into_inner(guard); + Ok(()) + } + + /// Read the marker file at the given path + fn read_attach_marker_file(path: &Path) -> Result { + let mut file = match std::fs::File::open(path) { + Ok(file) => file, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return Err(AttachError::MarkerFileMissing { + path: path.to_owned(), + }); + } + Err(error) => { + return Err(AttachError::ReadMarkerFile { + op: "open", + path: path.to_owned(), + error: Box::new(error), + }); + } + }; + let mut contents = String::new(); + use std::io::Read; + file.read_to_string(&mut contents) + .map_err(|error| AttachError::ReadMarkerFile { + op: "read_to_string", + path: path.to_owned(), + error: Box::new(error), + })?; + // XXX if we change the format, check format_version to handle updates + let contents = + serde_json::from_str(&contents).map_err(|error| AttachError::ReadMarkerFile { + op: "parse", + path: path.to_owned(), + error: Box::new(error), + })?; + Ok(contents) + } + /// /// Attach a tenant that's available in cloud storage. /// @@ -717,26 +828,8 @@ impl Tenant { path: tenant_dir, }); } - if marker_file.exists() { - return Err(AttachError::LocalStateExists { - what: "attaching marker file", - path: marker_file, - }); - } - - // create the marker file, i.e., /tenants/${tenant_id}___attaching - std::fs::File::create(&marker_file).map_err(|error| AttachError::CreateMarkerFile { - op: "create", - path: marker_file.clone(), - error, - })?; - crashsafe::fsync_file_and_parent(&marker_file).map_err(|error| { - AttachError::CreateMarkerFile { - op: "fsync_file_and_parent", - path: marker_file.clone(), - error, - } - })?; + // fails if the marker file already exists + Self::create_attach_marker_file(&marker_file)?; Self::spawn_resume_attach(conf, tenant_id, remote_storage, ctx) } @@ -752,15 +845,16 @@ impl Tenant { remote_storage: GenericRemoteStorage, ctx: &RequestContext, ) -> Result, AttachError> { - // Sanity-check, shouldn't happen in practice. + // Sanity-check, duplicated from `Tenant::resume_attach` let marker_file_path = conf.tenant_attaching_mark_file_path(&tenant_id); - if !marker_file_path.is_file() { - return Err(AttachError::MarkerFileMissing { - path: marker_file_path, - }); - } + let _marker_file_contents = Self::read_attach_marker_file(&marker_file_path)?; - // XXX: Attach should provide the config, especially during tenant migration. + // XXX: In the future, spawn_start_attach will receive a tenant config from control plane + // and persist it in the attach marker file. When resuming attach, + // the Tenant::resume_attach function should read it and persist it in the final location. + // For now, the contract is that the tenant starts with the default config after attach, + // and if the control plane requires tenant config changes, it will make another API call + // after the tenant has become active. // See https://github.com/neondatabase/neon/issues/1555 let tenant_conf = TenantConfOpt::default();