From 31729d6f4d41f2043e56719cb52d72fd9d187033 Mon Sep 17 00:00:00 2001 From: John Spray Date: Thu, 17 Aug 2023 11:00:30 +0100 Subject: [PATCH] pageserver: refactor tenant args into a structure This way, when we add some new shared structure that the tenants need a reference to, we do not have to add it individually as an extra argument to the various functions. --- pageserver/src/bin/pageserver.rs | 9 ++++-- pageserver/src/tenant.rs | 24 ++++++++++----- pageserver/src/tenant/mgr.rs | 51 ++++++++++++++++---------------- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 88a3f0229d..5b00715090 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -13,6 +13,7 @@ use pageserver::deletion_queue::DeletionQueue; use pageserver::disk_usage_eviction_task::{self, launch_disk_usage_global_eviction_task}; use pageserver::metrics::{STARTUP_DURATION, STARTUP_IS_LOADING}; use pageserver::task_mgr::WALRECEIVER_RUNTIME; +use pageserver::tenant::TenantSharedResources; use remote_storage::GenericRemoteStorage; use tokio::time::Instant; use tracing::*; @@ -404,9 +405,11 @@ fn start_pageserver( BACKGROUND_RUNTIME.block_on(mgr::init_tenant_mgr( conf, - broker_client.clone(), - remote_storage.clone(), - deletion_queue.clone(), + TenantSharedResources { + broker_client: broker_client.clone(), + remote_storage: remote_storage.clone(), + deletion_queue_client: deletion_queue.new_client(), + }, order, ))?; diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 11a61e3119..1f4bd0c692 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -57,7 +57,6 @@ use self::timeline::uninit::UninitializedTimeline; use self::timeline::EvictionTaskTenantState; use crate::config::PageServerConf; use crate::context::{DownloadBehavior, RequestContext}; -use crate::deletion_queue::DeletionQueue; use crate::deletion_queue::DeletionQueueClient; use crate::import_datadir; use crate::is_uninit_mark; @@ -154,6 +153,15 @@ pub const TENANT_ATTACHING_MARKER_FILENAME: &str = "attaching"; pub const TENANT_DELETED_MARKER_FILE_NAME: &str = "deleted"; +/// References to shared objects that are passed into each tenant, such +/// as the shared remote storage client and process initialization state. +#[derive(Clone)] +pub struct TenantSharedResources { + pub broker_client: storage_broker::BrokerClientChannel, + pub remote_storage: Option, + pub deletion_queue_client: DeletionQueueClient, +} + /// /// Tenant consists of multiple timelines. Keep them in a hash table. /// @@ -510,7 +518,7 @@ impl Tenant { tenant_id: TenantId, broker_client: storage_broker::BrokerClientChannel, remote_storage: GenericRemoteStorage, - deletion_queue: &DeletionQueue, + deletion_queue_client: DeletionQueueClient, ctx: &RequestContext, ) -> anyhow::Result> { // TODO dedup with spawn_load @@ -525,7 +533,7 @@ impl Tenant { wal_redo_manager, tenant_id, Some(remote_storage), - Some(deletion_queue.new_client()), + Some(deletion_queue_client), )); // Do all the hard work in the background @@ -796,9 +804,7 @@ impl Tenant { pub(crate) fn spawn_load( conf: &'static PageServerConf, tenant_id: TenantId, - broker_client: storage_broker::BrokerClientChannel, - remote_storage: Option, - deletion_queue: &DeletionQueue, + resources: TenantSharedResources, init_order: Option, tenants: &'static tokio::sync::RwLock, ctx: &RequestContext, @@ -813,6 +819,10 @@ impl Tenant { } }; + let broker_client = resources.broker_client; + let remote_storage = resources.remote_storage; + let deletion_queue_client = resources.deletion_queue_client; + let wal_redo_manager = Arc::new(PostgresRedoManager::new(conf, tenant_id)); let tenant = Tenant::new( TenantState::Loading, @@ -821,7 +831,7 @@ impl Tenant { wal_redo_manager, tenant_id, remote_storage.clone(), - Some(deletion_queue.new_client()), + Some(deletion_queue_client), ); let tenant = Arc::new(tenant); diff --git a/pageserver/src/tenant/mgr.rs b/pageserver/src/tenant/mgr.rs index b0f925bdb7..b8366d6269 100644 --- a/pageserver/src/tenant/mgr.rs +++ b/pageserver/src/tenant/mgr.rs @@ -30,6 +30,7 @@ use utils::id::{TenantId, TimelineId}; use super::delete::{remote_delete_mark_exists, DeleteTenantError}; use super::timeline::delete::DeleteTimelineFlow; +use super::TenantSharedResources; /// The tenants known to the pageserver. /// The enum variants are used to distinguish the different states that the pageserver can be in. @@ -67,9 +68,7 @@ static TENANTS: Lazy> = Lazy::new(|| RwLock::new(TenantsMap:: #[instrument(skip_all)] pub async fn init_tenant_mgr( conf: &'static PageServerConf, - broker_client: storage_broker::BrokerClientChannel, - remote_storage: Option, - deletion_queue: DeletionQueue, + resources: TenantSharedResources, init_order: InitializationOrder, ) -> anyhow::Result<()> { // Scan local filesystem for attached tenants @@ -127,9 +126,7 @@ pub async fn init_tenant_mgr( match schedule_local_tenant_processing( conf, &tenant_dir_path, - broker_client.clone(), - remote_storage.clone(), - &deletion_queue, + resources.clone(), Some(init_order.clone()), &TENANTS, &ctx, @@ -165,9 +162,7 @@ pub async fn init_tenant_mgr( pub(crate) fn schedule_local_tenant_processing( conf: &'static PageServerConf, tenant_path: &Path, - broker_client: storage_broker::BrokerClientChannel, - remote_storage: Option, - deletion_queue: &DeletionQueue, + resources: TenantSharedResources, init_order: Option, tenants: &'static tokio::sync::RwLock, ctx: &RequestContext, @@ -204,13 +199,13 @@ pub(crate) fn schedule_local_tenant_processing( let tenant = if conf.tenant_attaching_mark_file_path(&tenant_id).exists() { info!("tenant {tenant_id} has attaching mark file, resuming its attach operation"); - if let Some(remote_storage) = remote_storage { + if let Some(remote_storage) = resources.remote_storage { match Tenant::spawn_attach( conf, tenant_id, - broker_client, + resources.broker_client, remote_storage, - deletion_queue, + resources.deletion_queue_client, ctx, ) { Ok(tenant) => tenant, @@ -230,16 +225,7 @@ pub(crate) fn schedule_local_tenant_processing( } else { info!("tenant {tenant_id} is assumed to be loadable, starting load operation"); // Start loading the tenant into memory. It will initially be in Loading state. - Tenant::spawn_load( - conf, - tenant_id, - broker_client, - remote_storage, - deletion_queue, - init_order, - tenants, - ctx, - ) + Tenant::spawn_load(conf, tenant_id, resources, init_order, tenants, ctx) }; Ok(tenant) } @@ -375,8 +361,13 @@ pub async fn create_tenant( // TODO: tenant directory remains on disk if we bail out from here on. // See https://github.com/neondatabase/neon/issues/4233 + let tenant_resources = TenantSharedResources { + broker_client, + remote_storage, + deletion_queue_client: deletion_queue.new_client(), + }; let created_tenant = - schedule_local_tenant_processing(conf, &tenant_directory, broker_client, remote_storage, deletion_queue, None, &TENANTS, ctx)?; + schedule_local_tenant_processing(conf, &tenant_directory, tenant_resources, None, &TENANTS, ctx)?; // TODO: tenant object & its background loops remain, untracked in tenant map, if we fail here. // See https://github.com/neondatabase/neon/issues/4233 @@ -536,7 +527,12 @@ pub async fn load_tenant( .with_context(|| format!("Failed to remove tenant ignore mark {tenant_ignore_mark:?} during tenant loading"))?; } - let new_tenant = schedule_local_tenant_processing(conf, &tenant_path, broker_client, remote_storage, deletion_queue, None, &TENANTS, ctx) + let resources = TenantSharedResources { + broker_client, + remote_storage, + deletion_queue_client: deletion_queue.new_client(), + }; + let new_tenant = schedule_local_tenant_processing(conf, &tenant_path, resources, None, &TENANTS, ctx) .with_context(|| { format!("Failed to schedule tenant processing in path {tenant_path:?}") })?; @@ -624,7 +620,12 @@ pub async fn attach_tenant( .context("check for attach marker file existence")?; anyhow::ensure!(marker_file_exists, "create_tenant_files should have created the attach marker file"); - let attached_tenant = schedule_local_tenant_processing(conf, &tenant_dir, broker_client, Some(remote_storage), deletion_queue, None, &TENANTS, ctx)?; + let resources = TenantSharedResources { + broker_client, + remote_storage: Some(remote_storage), + deletion_queue_client: deletion_queue.new_client(), + }; + let attached_tenant = schedule_local_tenant_processing(conf, &tenant_dir, resources, None, &TENANTS, ctx)?; // TODO: tenant object & its background loops remain, untracked in tenant map, if we fail here. // See https://github.com/neondatabase/neon/issues/4233