From 4d3b15fc3e834c4506d79240939f8fb69caebcf0 Mon Sep 17 00:00:00 2001 From: Anastasia Lubennikova Date: Fri, 8 Apr 2022 14:43:46 +0300 Subject: [PATCH] Remove fields that are present in TenantConf from global PageserverConf to avoid confusion --- pageserver/src/config.rs | 135 --------------------------- pageserver/src/http/routes.rs | 4 +- pageserver/src/layered_repository.rs | 15 ++- pageserver/src/page_service.rs | 26 ++++-- pageserver/src/repository.rs | 7 +- pageserver/src/tenant_config.rs | 49 ++++++++-- pageserver/src/tenant_mgr.rs | 2 +- pageserver/src/timelines.rs | 2 +- 8 files changed, 80 insertions(+), 160 deletions(-) diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index bb4d805726..b05a63d94e 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -27,22 +27,6 @@ pub mod defaults { pub const DEFAULT_HTTP_LISTEN_PORT: u16 = 9898; pub const DEFAULT_HTTP_LISTEN_ADDR: &str = formatcp!("127.0.0.1:{DEFAULT_HTTP_LISTEN_PORT}"); - // FIXME: This current value is very low. I would imagine something like 1 GB or 10 GB - // would be more appropriate. But a low value forces the code to be exercised more, - // which is good for now to trigger bugs. - // This parameter actually determines L0 layer file size. - pub const DEFAULT_CHECKPOINT_DISTANCE: u64 = 256 * 1024 * 1024; - - // Target file size, when creating image and delta layers. - // This parameter determines L1 layer file size. - pub const DEFAULT_COMPACTION_TARGET_SIZE: u64 = 128 * 1024 * 1024; - - pub const DEFAULT_COMPACTION_PERIOD: &str = "1 s"; - - pub const DEFAULT_GC_HORIZON: u64 = 64 * 1024 * 1024; - pub const DEFAULT_GC_PERIOD: &str = "100 s"; - pub const DEFAULT_PITR_INTERVAL: &str = "30 days"; - pub const DEFAULT_WAIT_LSN_TIMEOUT: &str = "60 s"; pub const DEFAULT_WAL_REDO_TIMEOUT: &str = "60 s"; @@ -63,14 +47,6 @@ pub mod defaults { #listen_pg_addr = '{DEFAULT_PG_LISTEN_ADDR}' #listen_http_addr = '{DEFAULT_HTTP_LISTEN_ADDR}' -#checkpoint_distance = {DEFAULT_CHECKPOINT_DISTANCE} # in bytes -#compaction_target_size = {DEFAULT_COMPACTION_TARGET_SIZE} # in bytes -#compaction_period = '{DEFAULT_COMPACTION_PERIOD}' - -#gc_period = '{DEFAULT_GC_PERIOD}' -#gc_horizon = {DEFAULT_GC_HORIZON} -#pitr_interval = '{DEFAULT_PITR_INTERVAL}' - #wait_lsn_timeout = '{DEFAULT_WAIT_LSN_TIMEOUT}' #wal_redo_timeout = '{DEFAULT_WAL_REDO_TIMEOUT}' @@ -96,23 +72,6 @@ pub struct PageServerConf { /// Example (default): 127.0.0.1:9898 pub listen_http_addr: String, - // Flush out an inmemory layer, if it's holding WAL older than this - // This puts a backstop on how much WAL needs to be re-digested if the - // page server crashes. - // This parameter actually determines L0 layer file size. - pub checkpoint_distance: u64, - - // Target file size, when creating image and delta layers. - // This parameter determines L1 layer file size. - pub compaction_target_size: u64, - - // How often to check if there's compaction work to be done. - pub compaction_period: Duration, - - pub gc_horizon: u64, - pub gc_period: Duration, - pub pitr_interval: Duration, - // Timeout when waiting for WAL receiver to catch up to an LSN given in a GetPage@LSN call. pub wait_lsn_timeout: Duration, // How long to wait for WAL redo to complete. @@ -161,15 +120,6 @@ struct PageServerConfigBuilder { listen_http_addr: BuilderValue, - checkpoint_distance: BuilderValue, - - compaction_target_size: BuilderValue, - compaction_period: BuilderValue, - - gc_horizon: BuilderValue, - gc_period: BuilderValue, - pitr_interval: BuilderValue, - wait_lsn_timeout: BuilderValue, wal_redo_timeout: BuilderValue, @@ -198,15 +148,6 @@ impl Default for PageServerConfigBuilder { Self { listen_pg_addr: Set(DEFAULT_PG_LISTEN_ADDR.to_string()), listen_http_addr: Set(DEFAULT_HTTP_LISTEN_ADDR.to_string()), - checkpoint_distance: Set(DEFAULT_CHECKPOINT_DISTANCE), - compaction_target_size: Set(DEFAULT_COMPACTION_TARGET_SIZE), - compaction_period: Set(humantime::parse_duration(DEFAULT_COMPACTION_PERIOD) - .expect("cannot parse default compaction period")), - gc_horizon: Set(DEFAULT_GC_HORIZON), - gc_period: Set(humantime::parse_duration(DEFAULT_GC_PERIOD) - .expect("cannot parse default gc period")), - pitr_interval: Set(humantime::parse_duration(DEFAULT_PITR_INTERVAL) - .expect("cannot parse default PITR interval")), wait_lsn_timeout: Set(humantime::parse_duration(DEFAULT_WAIT_LSN_TIMEOUT) .expect("cannot parse default wait lsn timeout")), wal_redo_timeout: Set(humantime::parse_duration(DEFAULT_WAL_REDO_TIMEOUT) @@ -235,30 +176,6 @@ impl PageServerConfigBuilder { self.listen_http_addr = BuilderValue::Set(listen_http_addr) } - pub fn checkpoint_distance(&mut self, checkpoint_distance: u64) { - self.checkpoint_distance = BuilderValue::Set(checkpoint_distance) - } - - pub fn compaction_target_size(&mut self, compaction_target_size: u64) { - self.compaction_target_size = BuilderValue::Set(compaction_target_size) - } - - pub fn compaction_period(&mut self, compaction_period: Duration) { - self.compaction_period = BuilderValue::Set(compaction_period) - } - - pub fn gc_horizon(&mut self, gc_horizon: u64) { - self.gc_horizon = BuilderValue::Set(gc_horizon) - } - - pub fn gc_period(&mut self, gc_period: Duration) { - self.gc_period = BuilderValue::Set(gc_period) - } - - pub fn pitr_interval(&mut self, pitr_interval: Duration) { - self.pitr_interval = BuilderValue::Set(pitr_interval) - } - pub fn wait_lsn_timeout(&mut self, wait_lsn_timeout: Duration) { self.wait_lsn_timeout = BuilderValue::Set(wait_lsn_timeout) } @@ -314,22 +231,6 @@ impl PageServerConfigBuilder { listen_http_addr: self .listen_http_addr .ok_or(anyhow::anyhow!("missing listen_http_addr"))?, - checkpoint_distance: self - .checkpoint_distance - .ok_or(anyhow::anyhow!("missing checkpoint_distance"))?, - compaction_target_size: self - .compaction_target_size - .ok_or(anyhow::anyhow!("missing compaction_target_size"))?, - compaction_period: self - .compaction_period - .ok_or(anyhow::anyhow!("missing compaction_period"))?, - gc_horizon: self - .gc_horizon - .ok_or(anyhow::anyhow!("missing gc_horizon"))?, - gc_period: self.gc_period.ok_or(anyhow::anyhow!("missing gc_period"))?, - pitr_interval: self - .pitr_interval - .ok_or(anyhow::anyhow!("missing pitr_interval"))?, wait_lsn_timeout: self .wait_lsn_timeout .ok_or(anyhow::anyhow!("missing wait_lsn_timeout"))?, @@ -461,14 +362,6 @@ impl PageServerConf { match key { "listen_pg_addr" => builder.listen_pg_addr(parse_toml_string(key, item)?), "listen_http_addr" => builder.listen_http_addr(parse_toml_string(key, item)?), - "checkpoint_distance" => builder.checkpoint_distance(parse_toml_u64(key, item)?), - "compaction_target_size" => { - builder.compaction_target_size(parse_toml_u64(key, item)?) - } - "compaction_period" => builder.compaction_period(parse_toml_duration(key, item)?), - "gc_horizon" => builder.gc_horizon(parse_toml_u64(key, item)?), - "gc_period" => builder.gc_period(parse_toml_duration(key, item)?), - "pitr_interval" => builder.pitr_interval(parse_toml_duration(key, item)?), "wait_lsn_timeout" => builder.wait_lsn_timeout(parse_toml_duration(key, item)?), "wal_redo_timeout" => builder.wal_redo_timeout(parse_toml_duration(key, item)?), "initial_superuser_name" => builder.superuser(parse_toml_string(key, item)?), @@ -601,12 +494,6 @@ impl PageServerConf { pub fn dummy_conf(repo_dir: PathBuf) -> Self { PageServerConf { id: ZNodeId(0), - checkpoint_distance: defaults::DEFAULT_CHECKPOINT_DISTANCE, - compaction_target_size: 4 * 1024 * 1024, - compaction_period: Duration::from_secs(10), - gc_horizon: defaults::DEFAULT_GC_HORIZON, - gc_period: Duration::from_secs(10), - pitr_interval: Duration::from_secs(60 * 60), wait_lsn_timeout: Duration::from_secs(60), wal_redo_timeout: Duration::from_secs(60), page_cache_size: defaults::DEFAULT_PAGE_CACHE_SIZE, @@ -673,16 +560,6 @@ mod tests { listen_pg_addr = '127.0.0.1:64000' listen_http_addr = '127.0.0.1:9898' -checkpoint_distance = 111 # in bytes - -compaction_target_size = 111 # in bytes -compaction_period = '111 s' - -gc_period = '222 s' -gc_horizon = 222 - -pitr_interval = '30 days' - wait_lsn_timeout = '111 s' wal_redo_timeout = '111 s' @@ -714,12 +591,6 @@ id = 10 id: ZNodeId(10), listen_pg_addr: defaults::DEFAULT_PG_LISTEN_ADDR.to_string(), listen_http_addr: defaults::DEFAULT_HTTP_LISTEN_ADDR.to_string(), - checkpoint_distance: defaults::DEFAULT_CHECKPOINT_DISTANCE, - compaction_target_size: defaults::DEFAULT_COMPACTION_TARGET_SIZE, - compaction_period: humantime::parse_duration(defaults::DEFAULT_COMPACTION_PERIOD)?, - gc_horizon: defaults::DEFAULT_GC_HORIZON, - gc_period: humantime::parse_duration(defaults::DEFAULT_GC_PERIOD)?, - pitr_interval: humantime::parse_duration(defaults::DEFAULT_PITR_INTERVAL)?, wait_lsn_timeout: humantime::parse_duration(defaults::DEFAULT_WAIT_LSN_TIMEOUT)?, wal_redo_timeout: humantime::parse_duration(defaults::DEFAULT_WAL_REDO_TIMEOUT)?, superuser: defaults::DEFAULT_SUPERUSER.to_string(), @@ -760,14 +631,8 @@ id = 10 id: ZNodeId(10), listen_pg_addr: "127.0.0.1:64000".to_string(), listen_http_addr: "127.0.0.1:9898".to_string(), - checkpoint_distance: 111, - compaction_target_size: 111, - compaction_period: Duration::from_secs(111), - gc_horizon: 222, - gc_period: Duration::from_secs(222), wait_lsn_timeout: Duration::from_secs(111), wal_redo_timeout: Duration::from_secs(111), - pitr_interval: Duration::from_secs(30 * 24 * 60 * 60), superuser: "zzzz".to_string(), page_cache_size: 444, max_file_descriptors: 333, diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index fa8f405fed..6277481666 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -291,8 +291,7 @@ async fn tenant_create_handler(mut request: Request) -> Result) -> Result) -> Result<()> { let last_lsn = self.get_last_record_lsn(); + let repo = tenant_mgr::get_repository_for_tenant(self.tenantid)?; + let tenant_conf = repo.get_tenant_conf(); let distance = last_lsn.widening_sub(self.last_freeze_at.load()); - if distance >= self.conf.checkpoint_distance.into() { + if distance >= tenant_conf.checkpoint_distance.into() { self.freeze_inmem_layer(true); self.last_freeze_at.store(last_lsn); } @@ -1648,13 +1650,18 @@ impl LayeredTimeline { // above. Rewrite it. let _compaction_cs = self.compaction_cs.lock().unwrap(); - let target_file_size = self.conf.checkpoint_distance; + let repo = tenant_mgr::get_repository_for_tenant(self.tenantid)?; + let tenant_conf = repo.get_tenant_conf(); + + let target_file_size = tenant_conf.checkpoint_distance; // Define partitioning schema if needed if let Ok(pgdir) = tenant_mgr::get_timeline_for_tenant_load(self.tenantid, self.timelineid) { - let (partitioning, lsn) = - pgdir.repartition(self.get_last_record_lsn(), self.conf.compaction_target_size)?; + let (partitioning, lsn) = pgdir.repartition( + self.get_last_record_lsn(), + tenant_conf.compaction_target_size, + )?; let timer = self.create_images_time_histo.start_timer(); // 2. Create new image layers for partitions that have been modified // "enough". diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 2ffc8dcc73..bd51d25534 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -680,7 +680,7 @@ impl postgres_backend::Handler for PageServerHandler { ensure!(params.len() == 1, "invalid param number for config command"); let tenantid = ZTenantId::from_str(params[0])?; let repo = tenant_mgr::get_repository_for_tenant(tenantid)?; - let conf = repo.get_tenant_conf(); + let tenant_conf = repo.get_tenant_conf(); pgb.write_message_noflush(&BeMessage::RowDescription(&[ RowDescriptor::int8_col(b"checkpoint_distance"), RowDescriptor::int8_col(b"compaction_target_size"), @@ -690,12 +690,18 @@ impl postgres_backend::Handler for PageServerHandler { RowDescriptor::int8_col(b"pitr_interval"), ]))? .write_message_noflush(&BeMessage::DataRow(&[ - Some(conf.checkpoint_distance.to_string().as_bytes()), - Some(conf.compaction_target_size.to_string().as_bytes()), - Some(conf.compaction_period.as_secs().to_string().as_bytes()), - Some(conf.gc_horizon.to_string().as_bytes()), - Some(conf.gc_period.as_secs().to_string().as_bytes()), - Some(conf.pitr_interval.as_secs().to_string().as_bytes()), + Some(tenant_conf.checkpoint_distance.to_string().as_bytes()), + Some(tenant_conf.compaction_target_size.to_string().as_bytes()), + Some( + tenant_conf + .compaction_period + .as_secs() + .to_string() + .as_bytes(), + ), + Some(tenant_conf.gc_horizon.to_string().as_bytes()), + Some(tenant_conf.gc_period.as_secs().to_string().as_bytes()), + Some(tenant_conf.pitr_interval.as_secs().to_string().as_bytes()), ]))? .write_message(&BeMessage::CommandComplete(b"SELECT 1"))?; } else if query_string.starts_with("do_gc ") { @@ -715,10 +721,14 @@ impl postgres_backend::Handler for PageServerHandler { let tenantid = ZTenantId::from_str(caps.get(1).unwrap().as_str())?; let timelineid = ZTimelineId::from_str(caps.get(2).unwrap().as_str())?; + + let repo = tenant_mgr::get_repository_for_tenant(tenantid)?; + let tenant_conf = repo.get_tenant_conf(); + let gc_horizon: u64 = caps .get(4) .map(|h| h.as_str().parse()) - .unwrap_or(Ok(self.conf.gc_horizon))?; + .unwrap_or(Ok(tenant_conf.gc_horizon))?; let repo = tenant_mgr::get_repository_for_tenant(tenantid)?; let result = repo.gc_iteration(Some(timelineid), gc_horizon, Duration::ZERO, true)?; diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 5db9f7e4e7..28769d849c 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -460,6 +460,7 @@ pub mod repo_harness { pub struct RepoHarness<'a> { pub conf: &'static PageServerConf, + pub tenant_conf: TenantConf, pub tenant_id: ZTenantId, pub lock_guard: ( @@ -491,12 +492,15 @@ pub mod repo_harness { // OK in a test. let conf: &'static PageServerConf = Box::leak(Box::new(conf)); + let tenant_conf = TenantConf::dummy_conf(); + let tenant_id = ZTenantId::generate(); fs::create_dir_all(conf.tenant_path(&tenant_id))?; fs::create_dir_all(conf.timelines_path(&tenant_id))?; Ok(Self { conf, + tenant_conf, tenant_id, lock_guard, }) @@ -511,8 +515,7 @@ pub mod repo_harness { let repo = LayeredRepository::new( self.conf, - // Use default TenantConf in tests - TenantConf::from(self.conf), + self.tenant_conf, walredo_mgr, self.tenant_id, RemoteIndex::empty(), diff --git a/pageserver/src/tenant_config.rs b/pageserver/src/tenant_config.rs index 9d9f6a05f4..036c8a0c00 100644 --- a/pageserver/src/tenant_config.rs +++ b/pageserver/src/tenant_config.rs @@ -19,6 +19,24 @@ use zenith_utils::zid::ZTenantId; pub const TENANT_CONFIG_NAME: &str = "config"; +pub mod defaults { + // FIXME: This current value is very low. I would imagine something like 1 GB or 10 GB + // would be more appropriate. But a low value forces the code to be exercised more, + // which is good for now to trigger bugs. + // This parameter actually determines L0 layer file size. + pub const DEFAULT_CHECKPOINT_DISTANCE: u64 = 256 * 1024 * 1024; + + // Target file size, when creating image and delta layers. + // This parameter determines L1 layer file size. + pub const DEFAULT_COMPACTION_TARGET_SIZE: u64 = 128 * 1024 * 1024; + + pub const DEFAULT_COMPACTION_PERIOD: &str = "1 s"; + + pub const DEFAULT_GC_HORIZON: u64 = 64 * 1024 * 1024; + pub const DEFAULT_GC_PERIOD: &str = "100 s"; + pub const DEFAULT_PITR_INTERVAL: &str = "30 days"; +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct TenantConf { pub checkpoint_distance: u64, @@ -137,14 +155,19 @@ impl TenantConfFile { } impl TenantConf { - pub fn from(conf: &PageServerConf) -> TenantConf { + pub fn default() -> TenantConf { + use defaults::*; + TenantConf { - gc_period: conf.gc_period, - gc_horizon: conf.gc_horizon, - pitr_interval: conf.pitr_interval, - checkpoint_distance: conf.checkpoint_distance, - compaction_period: conf.compaction_period, - compaction_target_size: conf.compaction_target_size, + checkpoint_distance: DEFAULT_CHECKPOINT_DISTANCE, + compaction_target_size: DEFAULT_COMPACTION_TARGET_SIZE, + compaction_period: humantime::parse_duration(DEFAULT_COMPACTION_PERIOD) + .expect("cannot parse default compaction period"), + gc_horizon: DEFAULT_GC_HORIZON, + gc_period: humantime::parse_duration(DEFAULT_GC_PERIOD) + .expect("cannot parse default gc period"), + pitr_interval: humantime::parse_duration(DEFAULT_PITR_INTERVAL) + .expect("cannot parse default PITR interval"), } } @@ -153,6 +176,18 @@ impl TenantConf { pub fn tenantconf_path(conf: &'static PageServerConf, tenantid: ZTenantId) -> PathBuf { conf.tenant_path(&tenantid).join(TENANT_CONFIG_NAME) } + + #[cfg(test)] + pub fn dummy_conf() -> Self { + TenantConf { + checkpoint_distance: defaults::DEFAULT_CHECKPOINT_DISTANCE, + compaction_target_size: 4 * 1024 * 1024, + compaction_period: Duration::from_secs(10), + gc_horizon: defaults::DEFAULT_GC_HORIZON, + gc_period: Duration::from_secs(10), + pitr_interval: Duration::from_secs(60 * 60), + } + } } #[cfg(test)] diff --git a/pageserver/src/tenant_mgr.rs b/pageserver/src/tenant_mgr.rs index 9f729788ed..82d75f1699 100644 --- a/pageserver/src/tenant_mgr.rs +++ b/pageserver/src/tenant_mgr.rs @@ -298,7 +298,7 @@ pub fn get_timeline_for_tenant_load( .get_timeline_load(timelineid) .with_context(|| format!("Timeline {} not found for tenant {}", timelineid, tenantid))?; - let repartition_distance = tenant.repo.conf.checkpoint_distance / 10; + let repartition_distance = tenant.repo.get_tenant_conf().checkpoint_distance / 10; let page_tline = Arc::new(DatadirTimelineImpl::new(tline, repartition_distance)); page_tline.init_logical_size()?; diff --git a/pageserver/src/timelines.rs b/pageserver/src/timelines.rs index 19d03495f8..650743107e 100644 --- a/pageserver/src/timelines.rs +++ b/pageserver/src/timelines.rs @@ -150,7 +150,7 @@ pub fn init_pageserver( if let Some(tenant_id) = create_tenant { println!("initializing tenantid {}", tenant_id); - let repo = create_repo(conf, TenantConf::from(conf), tenant_id, CreateRepo::Dummy) + let repo = create_repo(conf, TenantConf::default(), tenant_id, CreateRepo::Dummy) .context("failed to create repo")?; let new_timeline_id = initial_timeline_id.unwrap_or_else(ZTimelineId::generate); bootstrap_timeline(conf, tenant_id, new_timeline_id, repo.as_ref())