From edcf4d61a473ca9b0399607c71006dc1c351e57b Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Fri, 12 May 2023 19:57:33 +0200 Subject: [PATCH] distinguish imitated from real size::gather_input calls in metrics (#4224) Before this PR, the gather_inputs() calls made to imitate synthetic size calculation accesses were accounted towards the real logical size calculation metric. This PR forces all callers to declare the cause for making logical size calculations, making the decision which cause counts towards which metric explicit. This is follow-up to ``` commit 1d266a6365565b30fc2d913bdf00490c8f51fe9e Author: Christian Schwarz Date: Thu May 11 16:09:29 2023 +0200 logical size calculation metrics: differentiate regular vs imitated (#4197) ``` After merging this patch, I hope to be able to explain why we have ca 30x more "logical size" ops in prod than "imitate logical size" for any given observation interval. refs https://github.com/neondatabase/neon/issues/4154 --- pageserver/src/consumption_metrics.rs | 6 ++-- pageserver/src/http/routes.rs | 8 +++-- pageserver/src/tenant.rs | 14 ++++++-- pageserver/src/tenant/size.rs | 18 ++++++++-- pageserver/src/tenant/timeline.rs | 33 +++++++++++++------ .../src/tenant/timeline/eviction_task.rs | 16 ++++++--- 6 files changed, 70 insertions(+), 25 deletions(-) diff --git a/pageserver/src/consumption_metrics.rs b/pageserver/src/consumption_metrics.rs index 23cfc55b65..ca7b9650e8 100644 --- a/pageserver/src/consumption_metrics.rs +++ b/pageserver/src/consumption_metrics.rs @@ -5,7 +5,7 @@ //! use crate::context::{DownloadBehavior, RequestContext}; use crate::task_mgr::{self, TaskKind, BACKGROUND_RUNTIME}; -use crate::tenant::mgr; +use crate::tenant::{mgr, LogicalSizeCalculationCause}; use anyhow; use chrono::Utc; use consumption_metrics::{idempotency_key, Event, EventChunk, EventType, CHUNK_SIZE}; @@ -335,7 +335,9 @@ pub async fn calculate_synthetic_size_worker( if let Ok(tenant) = mgr::get_tenant(tenant_id, true).await { - if let Err(e) = tenant.calculate_synthetic_size(ctx).await { + if let Err(e) = tenant.calculate_synthetic_size( + LogicalSizeCalculationCause::ConsumptionMetricsSyntheticSize, + ctx).await { error!("failed to calculate synthetic size for tenant {}: {}", tenant_id, e); } } diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 3c87e6f79d..699014b574 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -25,7 +25,7 @@ use crate::tenant::config::TenantConfOpt; use crate::tenant::mgr::{TenantMapInsertError, TenantStateError}; use crate::tenant::size::ModelInputs; use crate::tenant::storage_layer::LayerAccessStatsReset; -use crate::tenant::{PageReconstructError, Timeline}; +use crate::tenant::{LogicalSizeCalculationCause, PageReconstructError, Timeline}; use crate::{config::PageServerConf, tenant::mgr}; use utils::{ auth::JwtAuth, @@ -534,7 +534,11 @@ async fn tenant_size_handler(request: Request) -> Result, A // this can be long operation let inputs = tenant - .gather_size_inputs(retention_period, &ctx) + .gather_size_inputs( + retention_period, + LogicalSizeCalculationCause::TenantSizeHandler, + &ctx, + ) .await .map_err(ApiError::InternalServerError)?; diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index c9d5a54f0b..1aabe20c57 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -98,7 +98,9 @@ mod timeline; pub mod size; pub(crate) use timeline::debug_assert_current_span_has_tenant_and_timeline_id; -pub use timeline::{LocalLayerInfoForDiskUsageEviction, PageReconstructError, Timeline}; +pub use timeline::{ + LocalLayerInfoForDiskUsageEviction, LogicalSizeCalculationCause, PageReconstructError, Timeline, +}; // re-export this function so that page_cache.rs can use it. pub use crate::tenant::ephemeral_file::writeback as writeback_ephemeral_file; @@ -2642,6 +2644,7 @@ impl Tenant { // `max_retention_period` overrides the cutoff that is used to calculate the size // (only if it is shorter than the real cutoff). max_retention_period: Option, + cause: LogicalSizeCalculationCause, ctx: &RequestContext, ) -> anyhow::Result { let logical_sizes_at_once = self @@ -2663,6 +2666,7 @@ impl Tenant { logical_sizes_at_once, max_retention_period, &mut shared_cache, + cause, ctx, ) .await @@ -2672,8 +2676,12 @@ impl Tenant { /// This is periodically called by background worker. /// result is cached in tenant struct #[instrument(skip_all, fields(tenant_id=%self.tenant_id))] - pub async fn calculate_synthetic_size(&self, ctx: &RequestContext) -> anyhow::Result { - let inputs = self.gather_size_inputs(None, ctx).await?; + pub async fn calculate_synthetic_size( + &self, + cause: LogicalSizeCalculationCause, + ctx: &RequestContext, + ) -> anyhow::Result { + let inputs = self.gather_size_inputs(None, cause, ctx).await?; let size = inputs.calculate()?; diff --git a/pageserver/src/tenant/size.rs b/pageserver/src/tenant/size.rs index 9b2e5dc4f1..ffcbdc1f1d 100644 --- a/pageserver/src/tenant/size.rs +++ b/pageserver/src/tenant/size.rs @@ -11,7 +11,7 @@ use tokio_util::sync::CancellationToken; use crate::context::RequestContext; use crate::pgdatadir_mapping::CalculateLogicalSizeError; -use super::Tenant; +use super::{LogicalSizeCalculationCause, Tenant}; use crate::tenant::Timeline; use utils::id::TimelineId; use utils::lsn::Lsn; @@ -126,6 +126,7 @@ pub(super) async fn gather_inputs( limit: &Arc, max_retention_period: Option, logical_size_cache: &mut HashMap<(TimelineId, Lsn), u64>, + cause: LogicalSizeCalculationCause, ctx: &RequestContext, ) -> anyhow::Result { // refresh is needed to update gc related pitr_cutoff and horizon_cutoff @@ -318,7 +319,15 @@ pub(super) async fn gather_inputs( // We left the 'size' field empty in all of the Segments so far. // Now find logical sizes for all of the points that might need or benefit from them. - fill_logical_sizes(&timelines, &mut segments, limit, logical_size_cache, ctx).await?; + fill_logical_sizes( + &timelines, + &mut segments, + limit, + logical_size_cache, + cause, + ctx, + ) + .await?; Ok(ModelInputs { segments, @@ -336,6 +345,7 @@ async fn fill_logical_sizes( segments: &mut [SegmentMeta], limit: &Arc, logical_size_cache: &mut HashMap<(TimelineId, Lsn), u64>, + cause: LogicalSizeCalculationCause, ctx: &RequestContext, ) -> anyhow::Result<()> { let timeline_hash: HashMap> = HashMap::from_iter( @@ -378,6 +388,7 @@ async fn fill_logical_sizes( parallel_size_calcs, timeline, lsn, + cause, ctx, cancel.child_token(), ) @@ -485,6 +496,7 @@ async fn calculate_logical_size( limit: Arc, timeline: Arc, lsn: utils::lsn::Lsn, + cause: LogicalSizeCalculationCause, ctx: RequestContext, cancel: CancellationToken, ) -> Result { @@ -493,7 +505,7 @@ async fn calculate_logical_size( .expect("global semaphore should not had been closed"); let size_res = timeline - .spawn_ondemand_logical_size_calculation(lsn, ctx, cancel) + .spawn_ondemand_logical_size_calculation(lsn, cause, ctx, cancel) .instrument(info_span!("spawn_ondemand_logical_size_calculation")) .await?; Ok(TimelineAtLsnSizeResult(timeline, lsn, size_res)) diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 17b1d55c1f..db5a4b97b8 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -49,7 +49,7 @@ use crate::tenant::{ use crate::config::PageServerConf; use crate::keyspace::{KeyPartitioning, KeySpace}; -use crate::metrics::{StorageTimeMetrics, TimelineMetrics, UNEXPECTED_ONDEMAND_DOWNLOADS}; +use crate::metrics::{TimelineMetrics, UNEXPECTED_ONDEMAND_DOWNLOADS}; use crate::pgdatadir_mapping::LsnForTimestamp; use crate::pgdatadir_mapping::{is_rel_fsm_block_key, is_rel_vm_block_key}; use crate::pgdatadir_mapping::{BlockNumber, CalculateLogicalSizeError}; @@ -437,6 +437,14 @@ impl std::fmt::Display for PageReconstructError { } } +#[derive(Clone, Copy)] +pub enum LogicalSizeCalculationCause { + Initial, + ConsumptionMetricsSyntheticSize, + EvictionTaskImitation, + TenantSizeHandler, +} + /// Public interface functions impl Timeline { /// Get the LSN where this branch was created @@ -1839,7 +1847,7 @@ impl Timeline { // to spawn_ondemand_logical_size_calculation. let cancel = CancellationToken::new(); let calculated_size = match self_clone - .logical_size_calculation_task(lsn, &background_ctx, cancel) + .logical_size_calculation_task(lsn, LogicalSizeCalculationCause::Initial, &background_ctx, cancel) .await { Ok(s) => s, @@ -1893,6 +1901,7 @@ impl Timeline { pub fn spawn_ondemand_logical_size_calculation( self: &Arc, lsn: Lsn, + cause: LogicalSizeCalculationCause, ctx: RequestContext, cancel: CancellationToken, ) -> oneshot::Receiver> { @@ -1915,7 +1924,7 @@ impl Timeline { false, async move { let res = self_clone - .logical_size_calculation_task(lsn, &ctx, cancel) + .logical_size_calculation_task(lsn, cause, &ctx, cancel) .await; let _ = sender.send(res).ok(); Ok(()) // Receiver is responsible for handling errors @@ -1929,6 +1938,7 @@ impl Timeline { async fn logical_size_calculation_task( self: &Arc, lsn: Lsn, + cause: LogicalSizeCalculationCause, ctx: &RequestContext, cancel: CancellationToken, ) -> Result { @@ -1941,12 +1951,7 @@ impl Timeline { let cancel = cancel.child_token(); let ctx = ctx.attached_child(); self_calculation - .calculate_logical_size( - lsn, - &self_calculation.metrics.logical_size_histo, - cancel, - &ctx, - ) + .calculate_logical_size(lsn, cause, cancel, &ctx) .await }); let timeline_state_cancellation = async { @@ -2001,7 +2006,7 @@ impl Timeline { pub async fn calculate_logical_size( &self, up_to_lsn: Lsn, - storage_time_metrics: &StorageTimeMetrics, + cause: LogicalSizeCalculationCause, cancel: CancellationToken, ctx: &RequestContext, ) -> Result { @@ -2035,6 +2040,14 @@ impl Timeline { if let Some(size) = self.current_logical_size.initialized_size(up_to_lsn) { return Ok(size); } + let storage_time_metrics = match cause { + LogicalSizeCalculationCause::Initial + | LogicalSizeCalculationCause::ConsumptionMetricsSyntheticSize + | LogicalSizeCalculationCause::TenantSizeHandler => &self.metrics.logical_size_histo, + LogicalSizeCalculationCause::EvictionTaskImitation => { + &self.metrics.imitate_logical_size_histo + } + }; let timer = storage_time_metrics.start_timer(); let logical_size = self .get_current_logical_size_non_incremental(up_to_lsn, cancel, ctx) diff --git a/pageserver/src/tenant/timeline/eviction_task.rs b/pageserver/src/tenant/timeline/eviction_task.rs index 5ea3d5b14d..558600692e 100644 --- a/pageserver/src/tenant/timeline/eviction_task.rs +++ b/pageserver/src/tenant/timeline/eviction_task.rs @@ -30,7 +30,7 @@ use crate::{ tenant::{ config::{EvictionPolicy, EvictionPolicyLayerAccessThreshold}, storage_layer::PersistentLayer, - Tenant, + LogicalSizeCalculationCause, Tenant, }, }; @@ -344,7 +344,7 @@ impl Timeline { let size = self .calculate_logical_size( lsn, - &self.metrics.imitate_logical_size_histo, + LogicalSizeCalculationCause::EvictionTaskImitation, cancel.clone(), ctx, ) @@ -419,9 +419,15 @@ impl Timeline { .inner(); let mut throwaway_cache = HashMap::new(); - let gather = - crate::tenant::size::gather_inputs(tenant, limit, None, &mut throwaway_cache, ctx) - .instrument(info_span!("gather_inputs")); + let gather = crate::tenant::size::gather_inputs( + tenant, + limit, + None, + &mut throwaway_cache, + LogicalSizeCalculationCause::EvictionTaskImitation, + ctx, + ) + .instrument(info_span!("gather_inputs")); tokio::select! { _ = cancel.cancelled() => {}