From 57a6e931eaa4bebdff339386f3d3fd7027760098 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 12 Jan 2023 17:50:16 +0200 Subject: [PATCH] Comment, formatting and other cosmetic cleanup. --- pageserver/src/http/routes.rs | 2 +- pageserver/src/page_service.rs | 3 +- pageserver/src/tenant.rs | 2 +- .../src/tenant/remote_timeline_client.rs | 4 +-- pageserver/src/tenant/tasks.rs | 2 +- pageserver/src/tenant/timeline.rs | 28 +++++++++++++------ 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index f522b2b949..6d0db1b79f 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -595,7 +595,7 @@ async fn tenant_create_handler(mut request: Request) -> Result>, @@ -643,7 +642,7 @@ impl PageServerHandler { pgb.write_message(&BeMessage::CopyOutResponse)?; pgb.flush().await?; - /* Send a tarball of the latest layer on the timeline */ + // Send a tarball of the latest layer on the timeline { let mut writer = pgb.copyout_writer(); basebackup::send_basebackup_tarball(&mut writer, &timeline, lsn, prev_lsn, full_backup) diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index d1d1efac87..66f7e79877 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -612,7 +612,7 @@ impl Tenant { #[instrument(skip(self), fields(tenant_id=%self.tenant_id))] async fn attach(self: &Arc) -> anyhow::Result<()> { // Create directory with marker file to indicate attaching state. - // The load_local_tenants() function in tenant_mgr relies on the marker file + // The load_local_tenants() function in tenant::mgr relies on the marker file // to determine whether a tenant has finished attaching. let tenant_dir = self.conf.tenant_path(&self.tenant_id); let marker_file = self.conf.tenant_attaching_mark_file_path(&self.tenant_id); diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 1db69d8b73..c90d078034 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -16,7 +16,7 @@ //! unless the pageserver is configured without remote storage. //! //! We allocate the client instance in [Timeline][`crate::tenant::Timeline`], i.e., -//! either in [`crate::tenant_mgr`] during startup or when creating a new +//! either in [`crate::tenant::mgr`] during startup or when creating a new //! timeline. //! However, the client does not become ready for use until we've initialized its upload queue: //! @@ -756,7 +756,7 @@ impl RemoteTimelineClient { // Note: We only check for the shutdown requests between retries, so // if a shutdown request arrives while we're busy uploading, in the // upload::upload:*() call below, we will wait not exit until it has - // finisheed. We probably could cancel the upload by simply dropping + // finished. We probably could cancel the upload by simply dropping // the Future, but we're not 100% sure if the remote storage library // is cancellation safe, so we don't dare to do that. Hopefully, the // upload finishes or times out soon enough. diff --git a/pageserver/src/tenant/tasks.rs b/pageserver/src/tenant/tasks.rs index 8397d26e5d..b7ad8fe791 100644 --- a/pageserver/src/tenant/tasks.rs +++ b/pageserver/src/tenant/tasks.rs @@ -83,7 +83,7 @@ async fn compaction_loop(tenant_id: TenantId) { tokio::select! { _ = task_mgr::shutdown_watcher() => { info!("received cancellation request during idling"); - break ; + break; }, _ = tokio::time::sleep(sleep_duration) => {}, } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 146b4191fd..4c76ac614d 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -193,22 +193,29 @@ pub struct Timeline { } /// Internal structure to hold all data needed for logical size calculation. -/// Calculation consists of two parts: -/// 1. Initial size calculation. That might take a long time, because it requires -/// reading all layers containing relation sizes up to the `initial_part_end`. +/// +/// Calculation consists of two stages: +/// +/// 1. Initial size calculation. That might take a long time, because it requires +/// reading all layers containing relation sizes at `initial_part_end`. +/// /// 2. Collecting an incremental part and adding that to the initial size. /// Increments are appended on walreceiver writing new timeline data, /// which result in increase or decrease of the logical size. struct LogicalSize { - /// Size, potentially slow to compute, derived from all layers located locally on this node's FS. - /// Might require reading multiple layers, and even ancestor's layers, to collect the size. + /// Size, potentially slow to compute. Calculating this might require reading multiple + /// layers, and even ancestor's layers. /// - /// NOTE: initial size is not a constant and will change between restarts. + /// NOTE: size at a given LSN is constant, but after a restart we will calculate + /// the initial size at a different LSN. initial_logical_size: OnceCell, + /// Semaphore to track ongoing calculation of `initial_logical_size`. initial_size_computation: Arc, + /// Latest Lsn that has its size uncalculated, could be absent for freshly created timelines. initial_part_end: Option, + /// All other size changes after startup, combined together. /// /// Size shouldn't ever be negative, but this is signed for two reasons: @@ -370,6 +377,7 @@ pub enum PageReconstructError { #[error(transparent)] Other(#[from] anyhow::Error), // source and Display delegate to anyhow::Error + /// An error happened replaying WAL records #[error(transparent)] WalRedo(#[from] crate::walredo::WalRedoError), } @@ -1422,7 +1430,8 @@ impl Timeline { /// Calculate the logical size of the database at the latest LSN. /// - /// NOTE: counted incrementally, includes ancestors, this can be a slow operation. + /// NOTE: counted incrementally, includes ancestors. This can be a slow operation, + /// especially if we need to download remote layers. async fn calculate_logical_size( &self, up_to_lsn: Lsn, @@ -2942,9 +2951,10 @@ impl Timeline { if data.records.is_empty() { if let Some((img_lsn, img)) = &data.img { trace!( - "found page image for key {} at {}, no WAL redo required", + "found page image for key {} at {}, no WAL redo required, req LSN {}", key, - img_lsn + img_lsn, + request_lsn, ); Ok(img.clone()) } else {