From aa367e5d82914915efef17abdf4891a19a23bad3 Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Thu, 27 Feb 2025 08:46:51 +0200 Subject: [PATCH] Add lazy_slru_download_threshold parameter to page server config --- compute_tools/src/compute.rs | 28 ++++++++++++++++++++++------ libs/compute_api/src/spec.rs | 4 ++-- libs/pageserver_api/src/config.rs | 3 +++ pageserver/src/basebackup.rs | 18 ++++++++++++------ pageserver/src/config.rs | 6 ++++++ pageserver/src/page_service.rs | 7 ++++--- pageserver/src/tenant/timeline.rs | 15 +++++++++++++++ 7 files changed, 64 insertions(+), 17 deletions(-) diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 818563c6ee..29b0225419 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -897,13 +897,29 @@ impl ComputeNode { let mut client = config.connect(NoTls)?; let pageserver_connect_micros = start_time.elapsed().as_micros() as u64; - let replica = if spec.spec.mode != ComputeMode::Primary { " --replica" } else { "" }; - let lazy_slru_downloand = if spec.features.contains(&ComputeFeature::LazySlruDownload) { " --lazy-slru-download" } else { "" }; + let replica = if spec.spec.mode != ComputeMode::Primary { + " --replica" + } else { + "" + }; + let lazy_slru_download = if spec + .spec + .features + .contains(&ComputeFeature::LazySlruDownload) + { + " --lazy-slru-download" + } else { + "" + }; let basebackup_cmd = match lsn { - Lsn(0) => format!("basebackup {} {} --gzip{}{}", - spec.tenant_id, spec.timeline_id, replica, lazy_slru_dpownload) - _ => format!("basebackup {} {} {} --gzip{}{}", - spec.tenant_id, spec.timeline_id, lsn, replica, lazy_slru_dpownload) + Lsn(0) => format!( + "basebackup {} {} --gzip{}{}", + spec.tenant_id, spec.timeline_id, replica, lazy_slru_download + ), + _ => format!( + "basebackup {} {} {} --gzip{}{}", + spec.tenant_id, spec.timeline_id, lsn, replica, lazy_slru_download + ), }; let copyreader = client.copy_out(basebackup_cmd.as_str())?; diff --git a/libs/compute_api/src/spec.rs b/libs/compute_api/src/spec.rs index ea85e4cc52..158d057bbe 100644 --- a/libs/compute_api/src/spec.rs +++ b/libs/compute_api/src/spec.rs @@ -183,8 +183,8 @@ pub enum ComputeFeature { /// track short-lived connections as user activity. ActivityMonitorExperimental, - /// Download larger SLU files on demand - LazySlruDownload, + /// Download larger SLU files on demand + LazySlruDownload, /// This is a special feature flag that is used to represent unknown feature flags. /// Basically all unknown to enum flags are represented as this one. See unit test diff --git a/libs/pageserver_api/src/config.rs b/libs/pageserver_api/src/config.rs index 53b68afb0f..f7405434b9 100644 --- a/libs/pageserver_api/src/config.rs +++ b/libs/pageserver_api/src/config.rs @@ -181,6 +181,7 @@ pub struct ConfigToml { pub generate_unarchival_heatmap: Option, pub tracing: Option, pub enable_tls_page_service_api: bool, + pub lazy_slru_download_threshold: usize, } #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] @@ -537,6 +538,7 @@ pub mod defaults { pub const DEFAULT_SSL_KEY_FILE: &str = "server.key"; pub const DEFAULT_SSL_CERT_FILE: &str = "server.crt"; + pub const DEFAULT_LAZY_SLRU_DOWNLOAD_THRESHOLD: usize = 128; } impl Default for ConfigToml { @@ -655,6 +657,7 @@ impl Default for ConfigToml { generate_unarchival_heatmap: None, tracing: None, enable_tls_page_service_api: false, + lazy_slru_download_threshold: DEFAULT_LAZY_SLRU_DOWNLOAD_THRESHOLD, } } } diff --git a/pageserver/src/basebackup.rs b/pageserver/src/basebackup.rs index 26571f111e..462cb9b40c 100644 --- a/pageserver/src/basebackup.rs +++ b/pageserver/src/basebackup.rs @@ -80,7 +80,7 @@ pub async fn send_basebackup_tarball<'a, W>( prev_lsn: Option, full_backup: bool, replica: bool, - lazy_slru_download: bool, + lazy_slru_download_enabled: bool, ctx: &'a RequestContext, ) -> Result<(), BasebackupError> where @@ -132,8 +132,8 @@ where }; info!( - "taking basebackup lsn={}, prev_lsn={} (full_backup={}, replica={}, lazy_slru_download={})", - backup_lsn, prev_lsn, full_backup, replica, lazy_slru_download + "taking basebackup lsn={}, prev_lsn={} (full_backup={}, replica={}, lazy_slru_download_enabled={})", + backup_lsn, prev_lsn, full_backup, replica, lazy_slru_download_enabled ); let basebackup = Basebackup { @@ -143,7 +143,7 @@ where prev_record_lsn: prev_lsn, full_backup, replica, - lazy_slru_dpownload, + lazy_slru_download_enabled, ctx, io_concurrency: IoConcurrency::spawn_from_conf( timeline.conf, @@ -172,7 +172,7 @@ where prev_record_lsn: Lsn, full_backup: bool, replica: bool, - lazy_slru_download: bool, + lazy_slru_download_enabled: bool, ctx: &'a RequestContext, io_concurrency: IoConcurrency, } @@ -311,7 +311,9 @@ where self.timeline.pg_version, )?; - let lazy_slru_download = (self.self.lazy_slru_download || timeline.get_lazy_slru_download()) && !self.full_backup; + let lazy_slru_download = self.lazy_slru_download_enabled + && self.timeline.get_lazy_slru_download() + && !self.full_backup; let pgversion = self.timeline.pg_version; let subdirs = dispatch_pgversion!(pgversion, &pgv::bindings::PGDATA_SUBDIRS[..]); @@ -362,6 +364,10 @@ where .get_vectored(query, self.io_concurrency.clone(), self.ctx) .await?; + if blocks.len() > self.timeline.conf.lazy_slru_download_threshold { + self.timeline.set_lazy_slru_download(true); + } + for (key, block) in blocks { let block = block?; slru_builder.add_block(&key, block).await?; diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 26ae6af70e..f3c8aea50e 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -224,6 +224,10 @@ pub struct PageServerConf { /// Does not force TLS: the client negotiates TLS usage during the handshake. /// Uses key and certificate from ssl_key_file/ssl_cert_file. pub enable_tls_page_service_api: bool, + /// + /// Size of SLRU object in blocks which triggers on-demand download rarther than including it in basebackup + /// + pub lazy_slru_download_threshold: usize, } /// Token for authentication to safekeepers @@ -397,6 +401,7 @@ impl PageServerConf { generate_unarchival_heatmap, tracing, enable_tls_page_service_api, + lazy_slru_download_threshold, } = config_toml; let mut conf = PageServerConf { @@ -501,6 +506,7 @@ impl PageServerConf { } None => Vec::new(), }, + lazy_slru_download_threshold, }; // ------------------------------------------------------------ diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index c3d1ba2b31..4794f084f3 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -2726,7 +2726,7 @@ impl BaseBackupCmd { lsn, gzip, replica, - lazy_slru_download, + lazy_slru_download, }) } } @@ -2941,7 +2941,7 @@ where lsn, gzip, replica, - lazy_slru_download, + lazy_slru_download, }) => { tracing::Span::current() .record("tenant_id", field::display(tenant_id)) @@ -3001,6 +3001,7 @@ where true, false, false, + false, &ctx, ) .await?; @@ -3220,7 +3221,7 @@ mod tests { lsn: Some(Lsn::from_str("0/16ABCDE").unwrap()), gzip: true, replica: true, - lazy_slru_download: false + lazy_slru_download: false }) ); let cmd = PageServiceCmd::parse(&format!("fullbackup {tenant_id} {timeline_id}")).unwrap(); diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 613834dc88..e79e167ca5 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -285,6 +285,11 @@ pub struct Timeline { // them yet. disk_consistent_lsn: AtomicLsn, + // + // Flag indicating that SLRU for this timeline should be loaded on demand rathert than included in basesbackup + // + lazy_slru_download: AtomicBool, + // Parent timeline that this timeline was branched from, and the LSN // of the branch point. ancestor_timeline: Option>, @@ -2491,6 +2496,9 @@ impl Timeline { } pub(crate) fn get_lazy_slru_download(&self) -> bool { + if self.lazy_slru_download.load(AtomicOrdering::Relaxed) { + return true; + } let tenant_conf = self.tenant_conf.load(); tenant_conf .tenant_conf @@ -2498,6 +2506,7 @@ impl Timeline { .unwrap_or(self.conf.default_tenant_conf.lazy_slru_download) } +<<<<<<< HEAD /// Checks if a get page request should get perf tracing /// /// The configuration priority is: tenant config override, default tenant config, @@ -2521,6 +2530,11 @@ impl Timeline { } None => false, } +======= + pub(crate) fn set_lazy_slru_download(&self, enabled: bool) { + self.lazy_slru_download + .store(enabled, AtomicOrdering::Relaxed); +>>>>>>> 7f7f353dc (Add lazy_slru_download_threshold parameter to page server config) } fn get_checkpoint_distance(&self) -> u64 { @@ -2898,6 +2912,7 @@ impl Timeline { prev: metadata.prev_record_lsn().unwrap_or(Lsn(0)), }), disk_consistent_lsn: AtomicLsn::new(disk_consistent_lsn.0), + lazy_slru_download: AtomicBool::new(false), gc_compaction_state: ArcSwap::new(Arc::new(gc_compaction_state)),