From 310c507303d642c97a778f9850b57e1593ba5717 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 20 Sep 2022 07:58:06 +0300 Subject: [PATCH] Merge path retrieval methods in config.rs --- pageserver/src/config.rs | 17 +++++++++++++++++ pageserver/src/storage_sync.rs | 13 ++++--------- pageserver/src/storage_sync/download.rs | 15 +++++++-------- pageserver/src/storage_sync/upload.rs | 5 +++-- pageserver/src/tenant.rs | 9 ++++----- pageserver/src/tenant/metadata.rs | 17 +---------------- pageserver/src/tenant/timeline.rs | 4 ++-- pageserver/src/tenant_config.rs | 11 ----------- pageserver/src/tenant_mgr.rs | 12 +++++------- 9 files changed, 43 insertions(+), 60 deletions(-) diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 75c71b09d2..945ee098ea 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -22,6 +22,10 @@ use utils::{ use crate::tenant::TIMELINES_SEGMENT_NAME; use crate::tenant_config::{TenantConf, TenantConfOpt}; +/// The name of the metadata file pageserver creates per timeline. +pub const METADATA_FILE_NAME: &str = "metadata"; +const TENANT_CONFIG_NAME: &str = "config"; + pub mod defaults { use crate::tenant_config::defaults::*; use const_format::formatcp; @@ -346,6 +350,12 @@ impl PageServerConf { self.tenants_path().join(tenant_id.to_string()) } + /// Points to a place in pageserver's local directory, + /// where certain tenant's tenantconf file should be located. + pub fn tenant_config_path(&self, tenant_id: TenantId) -> PathBuf { + self.tenant_path(&tenant_id).join(TENANT_CONFIG_NAME) + } + pub fn timelines_path(&self, tenant_id: &TenantId) -> PathBuf { self.tenant_path(tenant_id).join(TIMELINES_SEGMENT_NAME) } @@ -354,6 +364,13 @@ impl PageServerConf { self.timelines_path(tenant_id).join(timeline_id.to_string()) } + /// Points to a place in pageserver's local directory, + /// where certain timeline's metadata file should be located. + pub fn metadata_path(&self, timeline_id: TimelineId, tenant_id: TenantId) -> PathBuf { + self.timeline_path(&timeline_id, &tenant_id) + .join(METADATA_FILE_NAME) + } + // // Postgres distribution paths // diff --git a/pageserver/src/storage_sync.rs b/pageserver/src/storage_sync.rs index 64e0f9a9e3..489d0ad4ed 100644 --- a/pageserver/src/storage_sync.rs +++ b/pageserver/src/storage_sync.rs @@ -169,13 +169,8 @@ use self::{ upload::{upload_index_part, upload_timeline_layers, UploadedTimeline}, }; use crate::{ - config::PageServerConf, - exponential_backoff, - storage_sync::index::RemoteIndex, - task_mgr, - task_mgr::TaskKind, - task_mgr::BACKGROUND_RUNTIME, - tenant::metadata::{metadata_path, TimelineMetadata}, + config::PageServerConf, exponential_backoff, storage_sync::index::RemoteIndex, task_mgr, + task_mgr::TaskKind, task_mgr::BACKGROUND_RUNTIME, tenant::metadata::TimelineMetadata, tenant_mgr::attach_local_tenants, }; use crate::{ @@ -1012,7 +1007,7 @@ async fn update_local_metadata( }; let remote_lsn = remote_metadata.disk_consistent_lsn(); - let local_metadata_path = metadata_path(conf, sync_id.timeline_id, sync_id.tenant_id); + let local_metadata_path = conf.metadata_path(sync_id.timeline_id, sync_id.tenant_id); let local_lsn = if local_metadata_path.exists() { let local_metadata = read_metadata_file(&local_metadata_path) .await @@ -1433,7 +1428,7 @@ mod test_utils { } fs::write( - metadata_path(harness.conf, timeline_id, harness.tenant_id), + harness.conf.metadata_path(timeline_id, harness.tenant_id), metadata.to_bytes()?, ) .await?; diff --git a/pageserver/src/storage_sync/download.rs b/pageserver/src/storage_sync/download.rs index 80d5ca5994..980001f95d 100644 --- a/pageserver/src/storage_sync/download.rs +++ b/pageserver/src/storage_sync/download.rs @@ -16,10 +16,7 @@ use tokio::{ }; use tracing::{debug, error, info, warn}; -use crate::{ - config::PageServerConf, storage_sync::SyncTask, tenant::metadata::metadata_path, - TEMP_FILE_SUFFIX, -}; +use crate::{config::PageServerConf, storage_sync::SyncTask, TEMP_FILE_SUFFIX}; use utils::id::{TenantId, TenantTimelineId, TimelineId}; use super::{ @@ -137,7 +134,8 @@ async fn download_index_part( storage: &GenericRemoteStorage, sync_id: TenantTimelineId, ) -> Result { - let index_part_path = metadata_path(conf, sync_id.timeline_id, sync_id.tenant_id) + let index_part_path = conf + .metadata_path(sync_id.timeline_id, sync_id.tenant_id) .with_file_name(IndexPart::FILE_NAME); let mut index_part_download = storage .download_storage_object(None, &index_part_path) @@ -620,9 +618,10 @@ mod tests { metadata.to_bytes()?, ); - let local_index_part_path = - metadata_path(harness.conf, sync_id.timeline_id, sync_id.tenant_id) - .with_file_name(IndexPart::FILE_NAME); + let local_index_part_path = harness + .conf + .metadata_path(sync_id.timeline_id, sync_id.tenant_id) + .with_file_name(IndexPart::FILE_NAME); let index_part_remote_id = local_storage.remote_object_id(&local_index_part_path)?; let index_part_local_path = PathBuf::from(index_part_remote_id.to_string()); fs::create_dir_all(index_part_local_path.parent().unwrap()).await?; diff --git a/pageserver/src/storage_sync/upload.rs b/pageserver/src/storage_sync/upload.rs index aa5a2232cf..75657915c0 100644 --- a/pageserver/src/storage_sync/upload.rs +++ b/pageserver/src/storage_sync/upload.rs @@ -15,7 +15,7 @@ use super::{ LayersUpload, SyncData, SyncQueue, }; use crate::metrics::NO_LAYERS_UPLOAD; -use crate::{config::PageServerConf, storage_sync::SyncTask, tenant::metadata::metadata_path}; +use crate::{config::PageServerConf, storage_sync::SyncTask}; /// Serializes and uploads the given index part data to the remote storage. pub(super) async fn upload_index_part( @@ -29,7 +29,8 @@ pub(super) async fn upload_index_part( let index_part_size = index_part_bytes.len(); let index_part_bytes = tokio::io::BufReader::new(std::io::Cursor::new(index_part_bytes)); - let index_part_path = metadata_path(conf, sync_id.timeline_id, sync_id.tenant_id) + let index_part_path = conf + .metadata_path(sync_id.timeline_id, sync_id.tenant_id) .with_file_name(IndexPart::FILE_NAME); storage .upload_storage_object( diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index cf236a0a9c..b753c1979c 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -41,7 +41,7 @@ use crate::metrics::{remove_tenant_metrics, STORAGE_TIME}; use crate::repository::GcResult; use crate::storage_sync::index::RemoteIndex; use crate::task_mgr; -use crate::tenant_config::{TenantConf, TenantConfOpt}; +use crate::tenant_config::TenantConfOpt; use crate::virtual_file::VirtualFile; use crate::walredo::WalRedoManager; use crate::{CheckpointConfig, TEMP_FILE_SUFFIX}; @@ -676,7 +676,7 @@ impl Tenant { conf: &'static PageServerConf, tenant_id: TenantId, ) -> anyhow::Result { - let target_config_path = TenantConf::path(conf, tenant_id); + let target_config_path = conf.tenant_config_path(tenant_id); let target_config_display = target_config_path.display(); info!("loading tenantconf from {target_config_display}"); @@ -1134,7 +1134,6 @@ pub mod harness { walredo::{WalRedoError, WalRedoManager}, }; - use super::metadata::metadata_path; use super::*; use crate::tenant_config::{TenantConf, TenantConfOpt}; use hex_literal::hex; @@ -1270,7 +1269,7 @@ pub mod harness { timeline_id: TimelineId, tenant_id: TenantId, ) -> anyhow::Result { - let metadata_path = metadata_path(conf, timeline_id, tenant_id); + let metadata_path = conf.metadata_path(timeline_id, tenant_id); let metadata_bytes = std::fs::read(&metadata_path).with_context(|| { format!( "Failed to read metadata bytes from path {}", @@ -1316,8 +1315,8 @@ pub mod harness { #[cfg(test)] mod tests { - use super::metadata::METADATA_FILE_NAME; use super::*; + use crate::config::METADATA_FILE_NAME; use crate::keyspace::KeySpaceAccum; use crate::repository::{Key, Value}; use crate::tenant::harness::*; diff --git a/pageserver/src/tenant/metadata.rs b/pageserver/src/tenant/metadata.rs index ace4dc91e9..606acbf2f1 100644 --- a/pageserver/src/tenant/metadata.rs +++ b/pageserver/src/tenant/metadata.rs @@ -8,7 +8,6 @@ use std::fs::{File, OpenOptions}; use std::io::Write; -use std::path::PathBuf; use anyhow::{bail, ensure, Context}; use serde::{Deserialize, Serialize}; @@ -29,9 +28,6 @@ use crate::STORAGE_FORMAT_VERSION; /// see PG_CONTROL_MAX_SAFE_SIZE const METADATA_MAX_SIZE: usize = 512; -/// The name of the metadata file pageserver creates per timeline. -pub const METADATA_FILE_NAME: &str = "metadata"; - /// Metadata stored on disk for each timeline /// /// The fields correspond to the values we hold in memory, in Timeline. @@ -166,17 +162,6 @@ 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, - timeline_id: TimelineId, - tenant_id: TenantId, -) -> PathBuf { - conf.timeline_path(&timeline_id, &tenant_id) - .join(METADATA_FILE_NAME) -} - /// Save timeline metadata to file pub fn save_metadata( conf: &'static PageServerConf, @@ -186,7 +171,7 @@ pub fn save_metadata( first_save: bool, ) -> anyhow::Result<()> { let _enter = info_span!("saving metadata").entered(); - let path = metadata_path(conf, timeline_id, tenant_id); + let path = conf.metadata_path(timeline_id, tenant_id); // use OpenOptions to ensure file presence is consistent with first_save let mut file = VirtualFile::open_with_options( &path, diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 8670e979ee..b80d023c7f 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -24,12 +24,12 @@ use crate::tenant::{ image_layer::{ImageLayer, ImageLayerWriter}, inmemory_layer::InMemoryLayer, layer_map::{LayerMap, SearchResult}, - metadata::{save_metadata, TimelineMetadata, METADATA_FILE_NAME}, + metadata::{save_metadata, TimelineMetadata}, par_fsync, storage_layer::{Layer, ValueReconstructResult, ValueReconstructState}, }; -use crate::config::PageServerConf; +use crate::config::{PageServerConf, METADATA_FILE_NAME}; use crate::keyspace::{KeyPartitioning, KeySpace}; use crate::metrics::TimelineMetrics; use crate::pgdatadir_mapping::BlockNumber; diff --git a/pageserver/src/tenant_config.rs b/pageserver/src/tenant_config.rs index 4448ffc456..4c5d5cc3f3 100644 --- a/pageserver/src/tenant_config.rs +++ b/pageserver/src/tenant_config.rs @@ -8,14 +8,9 @@ //! We cannot use global or default config instead, because wrong settings //! may lead to a data loss. //! -use crate::config::PageServerConf; use serde::{Deserialize, Serialize}; use std::num::NonZeroU64; -use std::path::PathBuf; use std::time::Duration; -use utils::id::TenantId; - -pub const TENANT_CONFIG_NAME: &str = "config"; pub mod defaults { // FIXME: This current value is very low. I would imagine something like 1 GB or 10 GB @@ -215,12 +210,6 @@ impl TenantConf { } } - /// Points to a place in pageserver's local directory, - /// where certain tenant's tenantconf file should be located. - pub fn path(conf: &'static PageServerConf, tenant_id: TenantId) -> PathBuf { - conf.tenant_path(&tenant_id).join(TENANT_CONFIG_NAME) - } - #[cfg(test)] pub fn dummy_conf() -> Self { TenantConf { diff --git a/pageserver/src/tenant_mgr.rs b/pageserver/src/tenant_mgr.rs index d6fa843305..2c6f5fa863 100644 --- a/pageserver/src/tenant_mgr.rs +++ b/pageserver/src/tenant_mgr.rs @@ -12,17 +12,15 @@ use tracing::*; use remote_storage::{path_with_suffix_extension, GenericRemoteStorage}; -use crate::config::PageServerConf; +use crate::config::{PageServerConf, METADATA_FILE_NAME}; use crate::http::models::TenantInfo; use crate::storage_sync::index::{RemoteIndex, RemoteTimelineIndex}; use crate::storage_sync::{self, LocalTimelineInitStatus, SyncStartupData}; use crate::task_mgr::{self, TaskKind}; use crate::tenant::{ - ephemeral_file::is_ephemeral_file, - metadata::{TimelineMetadata, METADATA_FILE_NAME}, - Tenant, TenantState, + ephemeral_file::is_ephemeral_file, metadata::TimelineMetadata, Tenant, TenantState, }; -use crate::tenant_config::{TenantConf, TenantConfOpt}; +use crate::tenant_config::TenantConfOpt; use crate::walredo::PostgresRedoManager; use crate::{TenantTimelineValues, TEMP_FILE_SUFFIX}; @@ -246,7 +244,7 @@ fn create_tenant_files( &temporary_tenant_dir, )?; let temporary_tenant_config_path = rebase_directory( - &TenantConf::path(conf, tenant_id), + &conf.tenant_config_path(tenant_id), &target_tenant_directory, &temporary_tenant_dir, )?; @@ -343,7 +341,7 @@ pub fn update_tenant_config( ) -> anyhow::Result<()> { info!("configuring tenant {tenant_id}"); get_tenant(tenant_id, true)?.update_tenant_config(tenant_conf); - Tenant::persist_tenant_config(&TenantConf::path(conf, tenant_id), tenant_conf, false)?; + Tenant::persist_tenant_config(&conf.tenant_config_path(tenant_id), tenant_conf, false)?; Ok(()) }