From e418fc6dc3e1617b9f2a130962d77fc63ceb8be9 Mon Sep 17 00:00:00 2001 From: Dmitry Rodionov Date: Thu, 6 Jul 2023 00:31:55 +0300 Subject: [PATCH] move some tracing related assertions to separate module for tenant and timeline --- pageserver/src/tenant.rs | 44 +++++-------------- .../tenant/remote_timeline_client/download.rs | 2 +- pageserver/src/tenant/span.rs | 24 ++++++++++ pageserver/src/tenant/timeline.rs | 32 ++------------ pageserver/src/tenant/timeline/span.rs | 26 +++++++++++ 5 files changed, 64 insertions(+), 64 deletions(-) create mode 100644 pageserver/src/tenant/span.rs create mode 100644 pageserver/src/tenant/timeline/span.rs diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index a4ea6b0035..1650b267f1 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -90,6 +90,7 @@ pub mod disk_btree; pub(crate) mod ephemeral_file; pub mod layer_map; pub mod manifest; +mod span; pub mod metadata; mod par_fsync; @@ -105,7 +106,7 @@ mod timeline; pub mod size; -pub(crate) use timeline::debug_assert_current_span_has_tenant_and_timeline_id; +pub(crate) use timeline::span::debug_assert_current_span_has_tenant_and_timeline_id; pub use timeline::{ LocalLayerInfoForDiskUsageEviction, LogicalSizeCalculationCause, PageReconstructError, Timeline, }; @@ -504,7 +505,7 @@ impl Tenant { /// No background tasks are started as part of this routine. /// async fn attach(self: &Arc, ctx: &RequestContext) -> anyhow::Result<()> { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); let marker_file = self.conf.tenant_attaching_mark_file_path(&self.tenant_id); if !tokio::fs::try_exists(&marker_file) @@ -642,7 +643,7 @@ impl Tenant { remote_client: RemoteTimelineClient, ctx: &RequestContext, ) -> anyhow::Result<()> { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); info!("downloading index file for timeline {}", timeline_id); tokio::fs::create_dir_all(self.conf.timeline_path(&timeline_id, &self.tenant_id)) @@ -721,7 +722,7 @@ impl Tenant { init_order: Option, ctx: &RequestContext, ) -> Arc { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); let tenant_conf = match Self::load_tenant_config(conf, tenant_id) { Ok(conf) => conf, @@ -907,7 +908,7 @@ impl Tenant { init_order: Option<&InitializationOrder>, ctx: &RequestContext, ) -> anyhow::Result<()> { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); debug!("loading tenant task"); @@ -953,7 +954,7 @@ impl Tenant { init_order: Option<&InitializationOrder>, ctx: &RequestContext, ) -> anyhow::Result<()> { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); let remote_client = self.remote_storage.as_ref().map(|remote_storage| { RemoteTimelineClient::new( @@ -1544,7 +1545,7 @@ impl Tenant { timeline_id: TimelineId, _ctx: &RequestContext, ) -> Result<(), DeleteTimelineError> { - timeline::debug_assert_current_span_has_tenant_and_timeline_id(); + debug_assert_current_span_has_tenant_and_timeline_id(); // Transition the timeline into TimelineState::Stopping. // This should prevent new operations from starting. @@ -1708,7 +1709,7 @@ impl Tenant { background_jobs_can_start: Option<&completion::Barrier>, ctx: &RequestContext, ) { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); let mut activating = false; self.state.send_modify(|current_state| { @@ -1779,7 +1780,7 @@ impl Tenant { /// /// This will attempt to shutdown even if tenant is broken. pub(crate) async fn shutdown(&self, freeze_and_flush: bool) -> Result<(), ShutdownError> { - debug_assert_current_span_has_tenant_id(); + span::debug_assert_current_span_has_tenant_id(); // Set tenant (and its timlines) to Stoppping state. // // Since we can only transition into Stopping state after activation is complete, @@ -4380,28 +4381,3 @@ mod tests { Ok(()) } } - -#[cfg(not(debug_assertions))] -#[inline] -pub(crate) fn debug_assert_current_span_has_tenant_id() {} - -#[cfg(debug_assertions)] -pub static TENANT_ID_EXTRACTOR: once_cell::sync::Lazy< - utils::tracing_span_assert::MultiNameExtractor<2>, -> = once_cell::sync::Lazy::new(|| { - utils::tracing_span_assert::MultiNameExtractor::new("TenantId", ["tenant_id", "tenant"]) -}); - -#[cfg(debug_assertions)] -#[inline] -pub(crate) fn debug_assert_current_span_has_tenant_id() { - use utils::tracing_span_assert; - - match tracing_span_assert::check_fields_present([&*TENANT_ID_EXTRACTOR]) { - Ok(()) => (), - Err(missing) => panic!( - "missing extractors: {:?}", - missing.into_iter().map(|e| e.name()).collect::>() - ), - } -} diff --git a/pageserver/src/tenant/remote_timeline_client/download.rs b/pageserver/src/tenant/remote_timeline_client/download.rs index a0d8c0193a..87b1026a76 100644 --- a/pageserver/src/tenant/remote_timeline_client/download.rs +++ b/pageserver/src/tenant/remote_timeline_client/download.rs @@ -16,7 +16,7 @@ use tracing::{info, warn}; use crate::config::PageServerConf; use crate::tenant::storage_layer::LayerFileName; -use crate::tenant::timeline::debug_assert_current_span_has_tenant_and_timeline_id; +use crate::tenant::timeline::span::debug_assert_current_span_has_tenant_and_timeline_id; use crate::{exponential_backoff, DEFAULT_BASE_BACKOFF_SECONDS, DEFAULT_MAX_BACKOFF_SECONDS}; use remote_storage::{DownloadError, GenericRemoteStorage}; use utils::crashsafe::path_with_suffix_extension; diff --git a/pageserver/src/tenant/span.rs b/pageserver/src/tenant/span.rs new file mode 100644 index 0000000000..1ee8aac31e --- /dev/null +++ b/pageserver/src/tenant/span.rs @@ -0,0 +1,24 @@ +#[cfg(not(debug_assertions))] +#[inline] +pub(crate) fn debug_assert_current_span_has_tenant_id() {} + +#[cfg(debug_assertions)] +pub static TENANT_ID_EXTRACTOR: once_cell::sync::Lazy< + utils::tracing_span_assert::MultiNameExtractor<2>, +> = once_cell::sync::Lazy::new(|| { + utils::tracing_span_assert::MultiNameExtractor::new("TenantId", ["tenant_id", "tenant"]) +}); + +#[cfg(debug_assertions)] +#[inline] +pub(crate) fn debug_assert_current_span_has_tenant_id() { + use utils::tracing_span_assert; + + match tracing_span_assert::check_fields_present([&*TENANT_ID_EXTRACTOR]) { + Ok(()) => (), + Err(missing) => panic!( + "missing extractors: {:?}", + missing.into_iter().map(|e| e.name()).collect::>() + ), + } +} diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 577ba5d867..7a8d4ced6f 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -1,6 +1,7 @@ //! mod eviction_task; +pub mod span; pub mod uninit; mod walreceiver; @@ -2240,7 +2241,7 @@ impl Timeline { ctx: &RequestContext, cancel: CancellationToken, ) -> Result { - debug_assert_current_span_has_tenant_and_timeline_id(); + span::debug_assert_current_span_has_tenant_and_timeline_id(); let mut timeline_state_updates = self.subscribe_for_state_updates(); let self_calculation = Arc::clone(self); @@ -4546,7 +4547,7 @@ impl Timeline { &self, remote_layer: Arc, ) -> anyhow::Result<()> { - debug_assert_current_span_has_tenant_and_timeline_id(); + span::debug_assert_current_span_has_tenant_and_timeline_id(); use std::sync::atomic::Ordering::Relaxed; @@ -4980,33 +4981,6 @@ fn rename_to_backup(path: &Path) -> anyhow::Result<()> { bail!("couldn't find an unused backup number for {:?}", path) } -#[cfg(not(debug_assertions))] -#[inline] -pub(crate) fn debug_assert_current_span_has_tenant_and_timeline_id() {} - -#[cfg(debug_assertions)] -#[inline] -pub(crate) fn debug_assert_current_span_has_tenant_and_timeline_id() { - use utils::tracing_span_assert; - - pub static TIMELINE_ID_EXTRACTOR: once_cell::sync::Lazy< - tracing_span_assert::MultiNameExtractor<2>, - > = once_cell::sync::Lazy::new(|| { - tracing_span_assert::MultiNameExtractor::new("TimelineId", ["timeline_id", "timeline"]) - }); - - match tracing_span_assert::check_fields_present([ - &*super::TENANT_ID_EXTRACTOR, - &*TIMELINE_ID_EXTRACTOR, - ]) { - Ok(()) => (), - Err(missing) => panic!( - "missing extractors: {:?}", - missing.into_iter().map(|e| e.name()).collect::>() - ), - } -} - /// Similar to `Arc::ptr_eq`, but only compares the object pointers, not vtables. /// /// Returns `true` if the two `Arc` point to the same layer, false otherwise. diff --git a/pageserver/src/tenant/timeline/span.rs b/pageserver/src/tenant/timeline/span.rs new file mode 100644 index 0000000000..3cd405dc31 --- /dev/null +++ b/pageserver/src/tenant/timeline/span.rs @@ -0,0 +1,26 @@ +#[cfg(not(debug_assertions))] +#[inline] +pub(crate) fn debug_assert_current_span_has_tenant_and_timeline_id() {} + +#[cfg(debug_assertions)] +#[inline] +pub(crate) fn debug_assert_current_span_has_tenant_and_timeline_id() { + use utils::tracing_span_assert; + + pub static TIMELINE_ID_EXTRACTOR: once_cell::sync::Lazy< + tracing_span_assert::MultiNameExtractor<2>, + > = once_cell::sync::Lazy::new(|| { + tracing_span_assert::MultiNameExtractor::new("TimelineId", ["timeline_id", "timeline"]) + }); + + match tracing_span_assert::check_fields_present([ + &*crate::tenant::span::TENANT_ID_EXTRACTOR, + &*TIMELINE_ID_EXTRACTOR, + ]) { + Ok(()) => (), + Err(missing) => panic!( + "missing extractors: {:?}", + missing.into_iter().map(|e| e.name()).collect::>() + ), + } +}