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.
2026-06-04 22:10:39 +00:00 · 2023-08-17 11:00:30 +01:00
parent 7e0e3517c1
commit 31729d6f4d
3 changed files with 49 additions and 35 deletions
--- 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,
    ))?;

--- 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<GenericRemoteStorage>,
+    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<Arc<Tenant>> {
        // 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<GenericRemoteStorage>,
-        deletion_queue: &DeletionQueue,
+        resources: TenantSharedResources,
        init_order: Option<InitializationOrder>,
        tenants: &'static tokio::sync::RwLock<TenantsMap>,
        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);

--- 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<RwLock<TenantsMap>> = 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<GenericRemoteStorage>,
-    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<GenericRemoteStorage>,
-    deletion_queue: &DeletionQueue,
+    resources: TenantSharedResources,
    init_order: Option<InitializationOrder>,
    tenants: &'static tokio::sync::RwLock<TenantsMap>,
    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