From fb63bd14253e9c0566e91361df84630d6df1afbe Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Mon, 24 Feb 2025 18:15:34 +0100 Subject: [PATCH] undo all the propagation changes --- pageserver/src/context.rs | 20 +------ pageserver/src/http/routes.rs | 8 +-- pageserver/src/metrics.rs | 21 ------- pageserver/src/page_service.rs | 2 +- pageserver/src/tenant.rs | 75 +++++++----------------- pageserver/src/tenant/timeline/delete.rs | 12 ++-- 6 files changed, 33 insertions(+), 105 deletions(-) diff --git a/pageserver/src/context.rs b/pageserver/src/context.rs index 3ac4b20410..518445bf98 100644 --- a/pageserver/src/context.rs +++ b/pageserver/src/context.rs @@ -94,10 +94,7 @@ use std::sync::Arc; use once_cell::sync::Lazy; use tracing::warn; -use crate::{ - task_mgr::TaskKind, - tenant::{Tenant, Timeline}, -}; +use crate::{task_mgr::TaskKind, tenant::Timeline}; // The main structure of this module, see module-level comment. #[derive(Debug)] @@ -115,9 +112,6 @@ pub(crate) enum Scope { Global { io_size_metrics: &'static crate::metrics::StorageIoSizeMetrics, }, - Tenant { - tenant: Arc, - }, Timeline { timeline: Arc, }, @@ -131,22 +125,14 @@ impl Scope { io_size_metrics: &&GLOBAL_IO_SIZE_METRICS, } } - pub(crate) fn new_tenant(tenant: &Arc) -> Self { - Scope::Tenant { - tenant: Arc::clone(tenant), - } - } pub(crate) fn new_timeline(timeline: &Arc) -> Self { Scope::Timeline { timeline: Arc::clone(timeline), } } - - pub(crate) fn io_size_metrics(&self) -> &crate::metrics::StorageIoSizeMetrics { match self { Scope::Global { io_size_metrics } => io_size_metrics, - Scope::Tenant { tenant } => &tenant.virtual_file_io_metrics, Scope::Timeline { timeline } => &timeline.metrics.storage_io_size, } } @@ -366,10 +352,6 @@ impl RequestContext { &self.scope } - pub(crate) fn scope_mut(&mut self) -> &mut Scope { - &mut self.scope - } - pub(crate) fn assert_is_timeline_scoped(&self, what: &str) { if let Scope::Timeline { .. } = self.scope() { return; diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 9d2f04dc4d..56a84a98a8 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -68,7 +68,6 @@ use tokio_util::sync::CancellationToken; use tracing::*; use crate::config::PageServerConf; -use crate::context; use crate::context::RequestContextBuilder; use crate::context::{DownloadBehavior, RequestContext}; use crate::deletion_queue::DeletionQueueClient; @@ -2617,9 +2616,8 @@ async fn getpage_at_lsn_handler_inner( async { let ctx = RequestContext::new(TaskKind::MgmtRequest, DownloadBehavior::Download); // Enable read path debugging + let ctx = RequestContextBuilder::extend(&ctx).read_path_debug(true).build(); let timeline = active_timeline_of_active_tenant(&state.tenant_manager, tenant_shard_id, timeline_id).await?; - let ctx = RequestContextBuilder::extend(&ctx).read_path_debug(true) - .scope(context::Scope::new_timeline(&timeline)).build(); // Use last_record_lsn if no lsn is provided let lsn = lsn.unwrap_or_else(|| timeline.get_last_record_lsn()); @@ -3286,7 +3284,7 @@ async fn put_tenant_timeline_import_basebackup( tenant.wait_to_become_active(ACTIVE_TENANT_TIMEOUT).await?; - let (timeline, timeline_ctx) = tenant + let timeline = tenant .create_empty_timeline(timeline_id, base_lsn, pg_version, &ctx) .map_err(ApiError::InternalServerError) .await?; @@ -3305,7 +3303,7 @@ async fn put_tenant_timeline_import_basebackup( info!("importing basebackup"); timeline - .import_basebackup_from_tar(tenant.clone(), &mut body, base_lsn, broker_client, &timeline_ctx) + .import_basebackup_from_tar(tenant.clone(), &mut body, base_lsn, broker_client, &ctx) .await .map_err(ApiError::InternalServerError)?; diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index cad18ebfe0..e683add134 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -1251,25 +1251,6 @@ impl StorageIoSizeMetrics { .unwrap(); Self { read, write } } - - pub(crate) fn new_tenant(tenant_shard_id: &TenantShardId) -> Self { - Self::new( - &tenant_shard_id.tenant_id.to_string(), - &tenant_shard_id.shard_slug().to_string(), - "*", - ) - } - - fn remove_per_tenant_metrics(tenant_shard_id: &TenantShardId) { - for operation in StorageIoSizeOperation::VARIANTS { - let _ = STORAGE_IO_SIZE.remove_label_values(&[ - operation, - &tenant_shard_id.tenant_id.to_string(), - &tenant_shard_id.shard_slug().to_string(), - "*", - ]); - } - } } #[cfg(not(test))] @@ -3260,8 +3241,6 @@ pub(crate) fn remove_tenant_metrics(tenant_shard_id: &TenantShardId) { tenant_throttling::remove_tenant_metrics(tenant_shard_id); - StorageIoSizeMetrics::remove_per_tenant_metrics(&tenant_shard_id); - // we leave the BROKEN_TENANTS_SET entry if any } diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 8380cae379..7285697040 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -53,7 +53,7 @@ use utils::{ use crate::auth::check_permission; use crate::basebackup::BasebackupError; use crate::config::PageServerConf; -use crate::context::{self, DownloadBehavior, RequestContext, RequestContextBuilder}; +use crate::context::{DownloadBehavior, RequestContext}; use crate::metrics::{self, SmgrOpTimer}; use crate::metrics::{ComputeCommandKind, COMPUTE_COMMANDS_COUNTERS, LIVE_CONNECTIONS}; use crate::pgdatadir_mapping::Version; diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 2449df4a26..efb35625f2 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -99,8 +99,6 @@ use self::timeline::TimelineDeleteProgress; use self::timeline::TimelineResources; use self::timeline::WaitLsnError; use crate::config::PageServerConf; -use crate::context; -use crate::context::RequestContextBuilder; use crate::context::{DownloadBehavior, RequestContext}; use crate::deletion_queue::DeletionQueueClient; use crate::deletion_queue::DeletionQueueError; @@ -387,8 +385,6 @@ pub struct Tenant { pub(crate) pagestream_throttle_metrics: Arc, - pub(crate) virtual_file_io_metrics: crate::metrics::StorageIoSizeMetrics, - /// An ongoing timeline detach concurrency limiter. /// /// As a tenant will likely be restarted as part of timeline detach ancestor it makes no sense @@ -1164,7 +1160,7 @@ impl Tenant { } }; - let (timeline, timeline_ctx) = self.create_timeline_struct( + let timeline = self.create_timeline_struct( timeline_id, &metadata, previous_heatmap, @@ -1172,7 +1168,6 @@ impl Tenant { resources, CreateTimelineCause::Load, idempotency.clone(), - ctx, )?; let disk_consistent_lsn = timeline.get_disk_consistent_lsn(); anyhow::ensure!( @@ -1336,9 +1331,6 @@ impl Tenant { // Do all the hard work in the background let tenant_clone = Arc::clone(&tenant); let ctx = ctx.detached_child(TaskKind::Attach, DownloadBehavior::Warn); - let ctx = RequestContextBuilder::extend(&ctx) - .scope(context::Scope::new_tenant(&tenant)) - .build(); task_mgr::spawn( &tokio::runtime::Handle::current(), TaskKind::Attach, @@ -1724,9 +1716,6 @@ impl Tenant { // layer file. let sorted_timelines = tree_sort_timelines(timeline_ancestors, |m| m.ancestor_timeline())?; for (timeline_id, remote_metadata) in sorted_timelines { - let ctx = RequestContextBuilder::extend(ctx) - .scope(context::Scope::new_tenant(self)) - .build(); let (index_part, remote_client, previous_heatmap) = remote_index_and_client .remove(&timeline_id) .expect("just put it in above"); @@ -1750,7 +1739,7 @@ impl Tenant { previous_heatmap, self.get_timeline_resources_for(remote_client), LoadTimelineCause::Attach, - &ctx, + ctx, ) .await .with_context(|| { @@ -1776,7 +1765,6 @@ impl Tenant { import_pgdata, ActivateTimelineArgs::No, guard, - ctx.detached_child(TaskKind::ImportPgdata, DownloadBehavior::Warn), )); } } @@ -1794,7 +1782,6 @@ impl Tenant { timeline_id, &index_part.metadata, remote_timeline_client, - ctx, ) .instrument(tracing::info_span!("timeline_delete", %timeline_id)) .await @@ -2428,8 +2415,8 @@ impl Tenant { new_timeline_id: TimelineId, initdb_lsn: Lsn, pg_version: u32, - ctx: &RequestContext, - ) -> anyhow::Result<(UninitializedTimeline, RequestContext)> { + _ctx: &RequestContext, + ) -> anyhow::Result { anyhow::ensure!( self.is_active(), "Cannot create empty timelines on inactive tenant" @@ -2463,7 +2450,6 @@ impl Tenant { create_guard, initdb_lsn, None, - ctx, ) .await } @@ -2481,7 +2467,7 @@ impl Tenant { pg_version: u32, ctx: &RequestContext, ) -> anyhow::Result> { - let (uninit_tl, ctx) = self + let uninit_tl = self .create_empty_timeline(new_timeline_id, initdb_lsn, pg_version, ctx) .await?; let tline = uninit_tl.raw_timeline().expect("we just created it"); @@ -2493,7 +2479,7 @@ impl Tenant { .init_empty_test_timeline() .context("init_empty_test_timeline")?; modification - .commit(&ctx) + .commit(ctx) .await .context("commit init_empty_test_timeline modification")?; @@ -2765,9 +2751,10 @@ impl Tenant { } }; - let (mut uninit_timeline, timeline_ctx) = { + let mut uninit_timeline = { let this = &self; let initdb_lsn = Lsn(0); + let _ctx = ctx; async move { let new_metadata = TimelineMetadata::new( // Initialize disk_consistent LSN to 0, The caller must import some data to @@ -2786,7 +2773,6 @@ impl Tenant { timeline_create_guard, initdb_lsn, None, - &ctx, ) .await } @@ -2816,7 +2802,6 @@ impl Tenant { index_part, activate, timeline_create_guard, - ctx.detached_child(TaskKind::ImportPgdata, DownloadBehavior::Warn), )); // NB: the timeline doesn't exist in self.timelines at this point @@ -2830,7 +2815,6 @@ impl Tenant { index_part: import_pgdata::index_part_format::Root, activate: ActivateTimelineArgs, timeline_create_guard: TimelineCreateGuard, - ctx: RequestContext, ) { debug_assert_current_span_has_tenant_and_timeline_id(); info!("starting"); @@ -2842,7 +2826,6 @@ impl Tenant { index_part, activate, timeline_create_guard, - ctx, ) .await; if let Err(err) = &res { @@ -2858,8 +2841,9 @@ impl Tenant { index_part: import_pgdata::index_part_format::Root, activate: ActivateTimelineArgs, timeline_create_guard: TimelineCreateGuard, - ctx: RequestContext, ) -> Result<(), anyhow::Error> { + let ctx = RequestContext::new(TaskKind::ImportPgdata, DownloadBehavior::Warn); + info!("importing pgdata"); import_pgdata::doit(&timeline, index_part, &ctx, self.cancel.clone()) .await @@ -4133,8 +4117,7 @@ impl Tenant { resources: TimelineResources, cause: CreateTimelineCause, create_idempotency: CreateTimelineIdempotency, - ctx: &RequestContext, - ) -> anyhow::Result<(Arc, RequestContext)> { + ) -> anyhow::Result> { let state = match cause { CreateTimelineCause::Load => { let ancestor_id = new_metadata.ancestor_timeline(); @@ -4168,11 +4151,7 @@ impl Tenant { self.cancel.child_token(), ); - let timeline_ctx = RequestContextBuilder::extend(ctx) - .scope(context::Scope::new_timeline(&timeline)) - .build(); - - Ok((timeline, timeline_ctx)) + Ok(timeline) } /// [`Tenant::shutdown`] must be called before dropping the returned [`Tenant`] object @@ -4291,9 +4270,6 @@ impl Tenant { pagestream_throttle_metrics: Arc::new( crate::metrics::tenant_throttling::Pagestream::new(&tenant_shard_id), ), - virtual_file_io_metrics: crate::metrics::StorageIoSizeMetrics::new_tenant( - &tenant_shard_id, - ), tenant_conf: Arc::new(ArcSwap::from_pointee(attached_conf)), ongoing_timeline_detach: std::sync::Mutex::default(), gc_block: Default::default(), @@ -4764,7 +4740,7 @@ impl Tenant { src_timeline: &Arc, dst_id: TimelineId, start_lsn: Option, - ctx: &RequestContext, + _ctx: &RequestContext, ) -> Result { let src_id = src_timeline.timeline_id; @@ -4864,14 +4840,13 @@ impl Tenant { src_timeline.pg_version, ); - let (uninitialized_timeline, _timeline_ctx) = self + let uninitialized_timeline = self .prepare_new_timeline( dst_id, &metadata, timeline_create_guard, start_lsn + 1, Some(Arc::clone(src_timeline)), - &ctx, ) .await?; @@ -5136,14 +5111,13 @@ impl Tenant { pgdata_lsn, pg_version, ); - let (mut raw_timeline, timeline_ctx) = self + let mut raw_timeline = self .prepare_new_timeline( timeline_id, &new_metadata, timeline_create_guard, pgdata_lsn, None, - ctx, ) .await?; @@ -5154,7 +5128,7 @@ impl Tenant { &unfinished_timeline, &pgdata_path, pgdata_lsn, - &timeline_ctx, + ctx, ) .await .with_context(|| { @@ -5222,8 +5196,7 @@ impl Tenant { create_guard: TimelineCreateGuard, start_lsn: Lsn, ancestor: Option>, - ctx: &RequestContext, - ) -> anyhow::Result<(UninitializedTimeline<'a>, RequestContext)> { + ) -> anyhow::Result> { let tenant_shard_id = self.tenant_shard_id; let resources = self.build_timeline_resources(new_timeline_id); @@ -5231,7 +5204,7 @@ impl Tenant { .remote_client .init_upload_queue_for_empty_remote(new_metadata)?; - let (timeline_struct, timeline_ctx) = self + let timeline_struct = self .create_timeline_struct( new_timeline_id, new_metadata, @@ -5240,7 +5213,6 @@ impl Tenant { resources, CreateTimelineCause::Load, create_guard.idempotency.clone(), - ctx, ) .context("Failed to create timeline data structure")?; @@ -5259,13 +5231,10 @@ impl Tenant { "Successfully created initial files for timeline {tenant_shard_id}/{new_timeline_id}" ); - Ok(( - UninitializedTimeline::new( - self, - new_timeline_id, - Some((timeline_struct, create_guard)), - ), - timeline_ctx, + Ok(UninitializedTimeline::new( + self, + new_timeline_id, + Some((timeline_struct, create_guard)), )) } diff --git a/pageserver/src/tenant/timeline/delete.rs b/pageserver/src/tenant/timeline/delete.rs index 1761d7bc71..841b2fa1c7 100644 --- a/pageserver/src/tenant/timeline/delete.rs +++ b/pageserver/src/tenant/timeline/delete.rs @@ -6,18 +6,20 @@ use std::{ use anyhow::Context; use pageserver_api::{models::TimelineState, shard::TenantShardId}; use remote_storage::DownloadError; -use reqwest::Request; use tokio::sync::OwnedMutexGuard; use tracing::{error, info, info_span, instrument, Instrument}; use utils::{crashsafe, fs_ext, id::TimelineId, pausable_failpoint}; use crate::{ - config::PageServerConf, context::RequestContext, task_mgr::{self, TaskKind}, tenant::{ + config::PageServerConf, + task_mgr::{self, TaskKind}, + tenant::{ metadata::TimelineMetadata, remote_timeline_client::{PersistIndexPartWithDeletedFlagError, RemoteTimelineClient}, CreateTimelineCause, DeleteTimelineError, MaybeDeletedIndexPart, Tenant, TenantManifestError, Timeline, TimelineOrOffloaded, - }, virtual_file::MaybeFatalIo + }, + virtual_file::MaybeFatalIo, }; /// Mark timeline as deleted in S3 so we won't pick it up next time @@ -284,11 +286,10 @@ impl DeleteTimelineFlow { timeline_id: TimelineId, local_metadata: &TimelineMetadata, remote_client: RemoteTimelineClient, - ctx: &RequestContext, ) -> anyhow::Result<()> { // Note: here we even skip populating layer map. Timeline is essentially uninitialized. // RemoteTimelineClient is the only functioning part. - let (timeline, timeline_ctx) = tenant + let timeline = tenant .create_timeline_struct( timeline_id, local_metadata, @@ -299,7 +300,6 @@ impl DeleteTimelineFlow { // Thus we need to skip the validation here. CreateTimelineCause::Delete, crate::tenant::CreateTimelineIdempotency::FailWithConflict, // doesn't matter what we put here - ctx, ) .context("create_timeline_struct")?;