diff --git a/libs/pageserver_api/src/config.rs b/libs/pageserver_api/src/config.rs index b12ef65780..4bf0214879 100644 --- a/libs/pageserver_api/src/config.rs +++ b/libs/pageserver_api/src/config.rs @@ -240,11 +240,7 @@ impl Default for EvictionOrder { #[serde(transparent)] pub struct MaxVectoredReadBytes(pub NonZeroUsize); -/// A tenant's calcuated configuration, which is the result of merging a -/// tenant's TenantConfOpt with the global TenantConf from PageServerConf. -/// -/// For storing and transmitting individual tenant's configuration, see -/// TenantConfOpt. +/// Tenant-level configuration values, used for various purposes. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(deny_unknown_fields, default)] pub struct TenantConfigToml { diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 4a8f75413c..42daa1d996 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -572,65 +572,126 @@ pub struct TenantConfigPatch { pub gc_compaction_ratio_percent: FieldPatch, } -/// An alternative representation of `pageserver::tenant::TenantConf` with -/// simpler types. -#[derive(Serialize, Deserialize, Debug, Default, Clone, Eq, PartialEq)] +/// Like [`crate::config::TenantConfigToml`], but preserves the information +/// about which parameters are set and which are not. +/// +/// Used in many places, including durably stored ones. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(default)] // this maps omitted fields in deserialization to None pub struct TenantConfig { + #[serde(skip_serializing_if = "Option::is_none")] pub checkpoint_distance: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub checkpoint_timeout: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_target_size: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub compaction_period: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_threshold: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_upper_limit: Option, - // defer parsing compaction_algorithm, like eviction_policy + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_algorithm: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_l0_first: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub compaction_l0_semaphore: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub l0_flush_delay_threshold: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub l0_flush_stall_threshold: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub l0_flush_wait_upload: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub gc_horizon: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub gc_period: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub image_creation_threshold: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub pitr_interval: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub walreceiver_connect_timeout: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub lagging_wal_timeout: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub max_lsn_wal_lag: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub eviction_policy: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub min_resident_size_override: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub evictions_low_residence_duration_metric_threshold: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub heatmap_period: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub lazy_slru_download: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub timeline_get_throttle: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub image_layer_creation_check_threshold: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub image_creation_preempt_threshold: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub lsn_lease_length: Option, - #[serde(default)] + + #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "humantime_serde")] pub lsn_lease_length_for_ts: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub timeline_offloading: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub wal_receiver_protocol_override: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub rel_size_v2_enabled: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub gc_compaction_enabled: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub gc_compaction_initial_threshold_kb: Option, + + #[serde(skip_serializing_if = "Option::is_none")] pub gc_compaction_ratio_percent: Option, } @@ -809,6 +870,110 @@ impl TenantConfig { gc_compaction_ratio_percent, }) } + + pub fn merge( + &self, + global_conf: crate::config::TenantConfigToml, + ) -> crate::config::TenantConfigToml { + crate::config::TenantConfigToml { + checkpoint_distance: self + .checkpoint_distance + .unwrap_or(global_conf.checkpoint_distance), + checkpoint_timeout: self + .checkpoint_timeout + .unwrap_or(global_conf.checkpoint_timeout), + compaction_target_size: self + .compaction_target_size + .unwrap_or(global_conf.compaction_target_size), + compaction_period: self + .compaction_period + .unwrap_or(global_conf.compaction_period), + compaction_threshold: self + .compaction_threshold + .unwrap_or(global_conf.compaction_threshold), + compaction_upper_limit: self + .compaction_upper_limit + .unwrap_or(global_conf.compaction_upper_limit), + compaction_algorithm: self + .compaction_algorithm + .as_ref() + .unwrap_or(&global_conf.compaction_algorithm) + .clone(), + compaction_l0_first: self + .compaction_l0_first + .unwrap_or(global_conf.compaction_l0_first), + compaction_l0_semaphore: self + .compaction_l0_semaphore + .unwrap_or(global_conf.compaction_l0_semaphore), + l0_flush_delay_threshold: self + .l0_flush_delay_threshold + .or(global_conf.l0_flush_delay_threshold), + l0_flush_stall_threshold: self + .l0_flush_stall_threshold + .or(global_conf.l0_flush_stall_threshold), + l0_flush_wait_upload: self + .l0_flush_wait_upload + .unwrap_or(global_conf.l0_flush_wait_upload), + gc_horizon: self.gc_horizon.unwrap_or(global_conf.gc_horizon), + gc_period: self.gc_period.unwrap_or(global_conf.gc_period), + image_creation_threshold: self + .image_creation_threshold + .unwrap_or(global_conf.image_creation_threshold), + pitr_interval: self.pitr_interval.unwrap_or(global_conf.pitr_interval), + walreceiver_connect_timeout: self + .walreceiver_connect_timeout + .unwrap_or(global_conf.walreceiver_connect_timeout), + lagging_wal_timeout: self + .lagging_wal_timeout + .unwrap_or(global_conf.lagging_wal_timeout), + max_lsn_wal_lag: self.max_lsn_wal_lag.unwrap_or(global_conf.max_lsn_wal_lag), + eviction_policy: self.eviction_policy.unwrap_or(global_conf.eviction_policy), + min_resident_size_override: self + .min_resident_size_override + .or(global_conf.min_resident_size_override), + evictions_low_residence_duration_metric_threshold: self + .evictions_low_residence_duration_metric_threshold + .unwrap_or(global_conf.evictions_low_residence_duration_metric_threshold), + heatmap_period: self.heatmap_period.unwrap_or(global_conf.heatmap_period), + lazy_slru_download: self + .lazy_slru_download + .unwrap_or(global_conf.lazy_slru_download), + timeline_get_throttle: self + .timeline_get_throttle + .clone() + .unwrap_or(global_conf.timeline_get_throttle), + image_layer_creation_check_threshold: self + .image_layer_creation_check_threshold + .unwrap_or(global_conf.image_layer_creation_check_threshold), + image_creation_preempt_threshold: self + .image_creation_preempt_threshold + .unwrap_or(global_conf.image_creation_preempt_threshold), + lsn_lease_length: self + .lsn_lease_length + .unwrap_or(global_conf.lsn_lease_length), + lsn_lease_length_for_ts: self + .lsn_lease_length_for_ts + .unwrap_or(global_conf.lsn_lease_length_for_ts), + timeline_offloading: self + .timeline_offloading + .unwrap_or(global_conf.timeline_offloading), + wal_receiver_protocol_override: self + .wal_receiver_protocol_override + .or(global_conf.wal_receiver_protocol_override), + rel_size_v2_enabled: self + .rel_size_v2_enabled + .unwrap_or(global_conf.rel_size_v2_enabled), + gc_compaction_enabled: self + .gc_compaction_enabled + .unwrap_or(global_conf.gc_compaction_enabled), + gc_compaction_initial_threshold_kb: self + .gc_compaction_initial_threshold_kb + .unwrap_or(global_conf.gc_compaction_initial_threshold_kb), + gc_compaction_ratio_percent: self + .gc_compaction_ratio_percent + .unwrap_or(global_conf.gc_compaction_ratio_percent), + } + } } /// The policy for the aux file storage. diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 562a16a14e..58e4722e6e 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -94,7 +94,7 @@ pub struct PageServerConf { pub remote_storage_config: Option, - pub default_tenant_conf: crate::tenant::config::TenantConf, + pub default_tenant_conf: pageserver_api::config::TenantConfigToml, /// Storage broker endpoints to connect to. pub broker_endpoint: Uri, diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index e8a32ca1ef..01ae96f89f 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -60,7 +60,7 @@ use crate::context::{DownloadBehavior, RequestContext, RequestContextBuilder}; use crate::deletion_queue::DeletionQueueClient; use crate::pgdatadir_mapping::LsnForTimestamp; use crate::task_mgr::TaskKind; -use crate::tenant::config::{LocationConf, TenantConfOpt}; +use crate::tenant::config::LocationConf; use crate::tenant::mgr::{ GetActiveTenantError, GetTenantError, TenantManager, TenantMapError, TenantMapInsertError, TenantSlot, TenantSlotError, TenantSlotUpsertError, TenantStateError, UpsertLocationError, @@ -1849,8 +1849,7 @@ async fn update_tenant_config_handler( let tenant_id = request_data.tenant_id; check_permission(&request, Some(tenant_id))?; - let new_tenant_conf = - TenantConfOpt::try_from(&request_data.config).map_err(ApiError::BadRequest)?; + let new_tenant_conf = request_data.config; let state = get_state(&request); @@ -1899,7 +1898,10 @@ async fn patch_tenant_config_handler( tenant.wait_to_become_active(ACTIVE_TENANT_TIMEOUT).await?; let updated = tenant - .update_tenant_config(|crnt| crnt.apply_patch(request_data.config.clone())) + .update_tenant_config(|crnt| { + crnt.apply_patch(request_data.config.clone()) + .map_err(anyhow::Error::new) + }) .map_err(ApiError::BadRequest)?; // This is a legacy API that only operates on attached tenants: the preferred diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 55b5704d67..fba6a49ccc 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -67,7 +67,7 @@ use utils::try_rcu::ArcSwapExt; use utils::zstd::{create_zst_tarball, extract_zst_tarball}; use utils::{backoff, completion, failpoint_support, fs_ext, pausable_failpoint}; -use self::config::{AttachedLocationConfig, AttachmentMode, LocationConf, TenantConf}; +use self::config::{AttachedLocationConfig, AttachmentMode, LocationConf}; use self::metadata::TimelineMetadata; use self::mgr::{GetActiveTenantError, GetTenantError}; use self::remote_timeline_client::upload::{upload_index_part, upload_tenant_manifest}; @@ -88,7 +88,7 @@ use crate::metrics::{ TENANT_SYNTHETIC_SIZE_METRIC, remove_tenant_metrics, }; use crate::task_mgr::TaskKind; -use crate::tenant::config::{LocationMode, TenantConfOpt}; +use crate::tenant::config::LocationMode; use crate::tenant::gc_result::GcResult; pub use crate::tenant::remote_timeline_client::index::IndexPart; use crate::tenant::remote_timeline_client::{ @@ -162,7 +162,7 @@ pub struct TenantSharedResources { /// in this struct. #[derive(Clone)] pub(super) struct AttachedTenantConf { - tenant_conf: TenantConfOpt, + tenant_conf: pageserver_api::models::TenantConfig, location: AttachedLocationConfig, /// The deadline before which we are blocked from GC so that /// leases have a chance to be renewed. @@ -170,7 +170,10 @@ pub(super) struct AttachedTenantConf { } impl AttachedTenantConf { - fn new(tenant_conf: TenantConfOpt, location: AttachedLocationConfig) -> Self { + fn new( + tenant_conf: pageserver_api::models::TenantConfig, + location: AttachedLocationConfig, + ) -> Self { // Sets a deadline before which we cannot proceed to GC due to lsn lease. // // We do this as the leases mapping are not persisted to disk. By delaying GC by lease @@ -251,7 +254,7 @@ pub struct Tenant { state: watch::Sender, // Overridden tenant-specific config parameters. - // We keep TenantConfOpt sturct here to preserve the information + // We keep pageserver_api::models::TenantConfig sturct here to preserve the information // about parameters that are not set. // This is necessary to allow global config updates. tenant_conf: Arc>, @@ -3702,17 +3705,14 @@ impl Tenant { /// create a Tenant in the same state. Do not use this in hot paths: it's for relatively /// rare external API calls, like a reconciliation at startup. pub(crate) fn get_location_conf(&self) -> models::LocationConfig { - let conf = self.tenant_conf.load(); + let attached_tenant_conf = self.tenant_conf.load(); - let location_config_mode = match conf.location.attach_mode { + let location_config_mode = match attached_tenant_conf.location.attach_mode { AttachmentMode::Single => models::LocationConfigMode::AttachedSingle, AttachmentMode::Multi => models::LocationConfigMode::AttachedMulti, AttachmentMode::Stale => models::LocationConfigMode::AttachedStale, }; - // We have a pageserver TenantConf, we need the API-facing TenantConfig. - let tenant_config: models::TenantConfig = conf.tenant_conf.clone().into(); - models::LocationConfig { mode: location_config_mode, generation: self.generation.into(), @@ -3720,7 +3720,7 @@ impl Tenant { shard_number: self.shard_identity.number.0, shard_count: self.shard_identity.count.literal(), shard_stripe_size: self.shard_identity.stripe_size.0, - tenant_conf: tenant_config, + tenant_conf: attached_tenant_conf.tenant_conf.clone(), } } @@ -3926,11 +3926,11 @@ enum ActivateTimelineArgs { } impl Tenant { - pub fn tenant_specific_overrides(&self) -> TenantConfOpt { + pub fn tenant_specific_overrides(&self) -> pageserver_api::models::TenantConfig { self.tenant_conf.load().tenant_conf.clone() } - pub fn effective_config(&self) -> TenantConf { + pub fn effective_config(&self) -> pageserver_api::config::TenantConfigToml { self.tenant_specific_overrides() .merge(self.conf.default_tenant_conf.clone()) } @@ -4072,10 +4072,14 @@ impl Tenant { } } - pub fn update_tenant_config anyhow::Result>( + pub fn update_tenant_config< + F: Fn( + pageserver_api::models::TenantConfig, + ) -> anyhow::Result, + >( &self, update: F, - ) -> anyhow::Result { + ) -> anyhow::Result { // Use read-copy-update in order to avoid overwriting the location config // state if this races with [`Tenant::set_new_location_config`]. Note that // this race is not possible if both request types come from the storage @@ -4122,7 +4126,7 @@ impl Tenant { fn get_pagestream_throttle_config( psconf: &'static PageServerConf, - overrides: &TenantConfOpt, + overrides: &pageserver_api::models::TenantConfig, ) -> throttle::Config { overrides .timeline_get_throttle @@ -4130,7 +4134,7 @@ impl Tenant { .unwrap_or(psconf.default_tenant_conf.timeline_get_throttle.clone()) } - pub(crate) fn tenant_conf_updated(&self, new_conf: &TenantConfOpt) { + pub(crate) fn tenant_conf_updated(&self, new_conf: &pageserver_api::models::TenantConfig) { let conf = Self::get_pagestream_throttle_config(self.conf, new_conf); self.pagestream_throttle.reconfigure(conf) } @@ -5492,7 +5496,7 @@ impl Tenant { Ok(()) } - pub(crate) fn get_tenant_conf(&self) -> TenantConfOpt { + pub(crate) fn get_tenant_conf(&self) -> pageserver_api::models::TenantConfig { self.tenant_conf.load().tenant_conf.clone() } @@ -5682,59 +5686,9 @@ pub(crate) mod harness { buf.freeze() } - impl From for TenantConfOpt { - fn from(tenant_conf: TenantConf) -> Self { - Self { - checkpoint_distance: Some(tenant_conf.checkpoint_distance), - checkpoint_timeout: Some(tenant_conf.checkpoint_timeout), - compaction_target_size: Some(tenant_conf.compaction_target_size), - compaction_period: Some(tenant_conf.compaction_period), - compaction_threshold: Some(tenant_conf.compaction_threshold), - compaction_upper_limit: Some(tenant_conf.compaction_upper_limit), - compaction_algorithm: Some(tenant_conf.compaction_algorithm), - compaction_l0_first: Some(tenant_conf.compaction_l0_first), - compaction_l0_semaphore: Some(tenant_conf.compaction_l0_semaphore), - l0_flush_delay_threshold: tenant_conf.l0_flush_delay_threshold, - l0_flush_stall_threshold: tenant_conf.l0_flush_stall_threshold, - l0_flush_wait_upload: Some(tenant_conf.l0_flush_wait_upload), - gc_horizon: Some(tenant_conf.gc_horizon), - gc_period: Some(tenant_conf.gc_period), - image_creation_threshold: Some(tenant_conf.image_creation_threshold), - pitr_interval: Some(tenant_conf.pitr_interval), - walreceiver_connect_timeout: Some(tenant_conf.walreceiver_connect_timeout), - lagging_wal_timeout: Some(tenant_conf.lagging_wal_timeout), - max_lsn_wal_lag: Some(tenant_conf.max_lsn_wal_lag), - eviction_policy: Some(tenant_conf.eviction_policy), - min_resident_size_override: tenant_conf.min_resident_size_override, - evictions_low_residence_duration_metric_threshold: Some( - tenant_conf.evictions_low_residence_duration_metric_threshold, - ), - heatmap_period: Some(tenant_conf.heatmap_period), - lazy_slru_download: Some(tenant_conf.lazy_slru_download), - timeline_get_throttle: Some(tenant_conf.timeline_get_throttle), - image_layer_creation_check_threshold: Some( - tenant_conf.image_layer_creation_check_threshold, - ), - image_creation_preempt_threshold: Some( - tenant_conf.image_creation_preempt_threshold, - ), - lsn_lease_length: Some(tenant_conf.lsn_lease_length), - lsn_lease_length_for_ts: Some(tenant_conf.lsn_lease_length_for_ts), - timeline_offloading: Some(tenant_conf.timeline_offloading), - wal_receiver_protocol_override: tenant_conf.wal_receiver_protocol_override, - rel_size_v2_enabled: Some(tenant_conf.rel_size_v2_enabled), - gc_compaction_enabled: Some(tenant_conf.gc_compaction_enabled), - gc_compaction_initial_threshold_kb: Some( - tenant_conf.gc_compaction_initial_threshold_kb, - ), - gc_compaction_ratio_percent: Some(tenant_conf.gc_compaction_ratio_percent), - } - } - } - pub struct TenantHarness { pub conf: &'static PageServerConf, - pub tenant_conf: TenantConf, + pub tenant_conf: pageserver_api::models::TenantConfig, pub tenant_shard_id: TenantShardId, pub generation: Generation, pub shard: ShardIndex, @@ -5761,7 +5715,7 @@ pub(crate) mod harness { impl TenantHarness { pub async fn create_custom( test_name: &'static str, - tenant_conf: TenantConf, + tenant_conf: pageserver_api::models::TenantConfig, tenant_id: TenantId, shard_identity: ShardIdentity, generation: Generation, @@ -5814,10 +5768,10 @@ pub(crate) mod harness { pub async fn create(test_name: &'static str) -> anyhow::Result { // Disable automatic GC and compaction to make the unit tests more deterministic. // The tests perform them manually if needed. - let tenant_conf = TenantConf { - gc_period: Duration::ZERO, - compaction_period: Duration::ZERO, - ..TenantConf::default() + let tenant_conf = pageserver_api::models::TenantConfig { + gc_period: Some(Duration::ZERO), + compaction_period: Some(Duration::ZERO), + ..Default::default() }; let tenant_id = TenantId::generate(); let shard = ShardIdentity::unsharded(); @@ -5857,7 +5811,7 @@ pub(crate) mod harness { TenantState::Attaching, self.conf, AttachedTenantConf::try_from(LocationConf::attached_single( - TenantConfOpt::from(self.tenant_conf.clone()), + self.tenant_conf.clone(), self.generation, &ShardParameters::default(), )) @@ -6941,14 +6895,14 @@ mod tests { // ``` #[tokio::test] async fn test_get_vectored_key_gap() -> anyhow::Result<()> { - let tenant_conf = TenantConf { + let tenant_conf = pageserver_api::models::TenantConfig { // Make compaction deterministic - gc_period: Duration::ZERO, - compaction_period: Duration::ZERO, + gc_period: Some(Duration::ZERO), + compaction_period: Some(Duration::ZERO), // Encourage creation of L1 layers - checkpoint_distance: 16 * 1024, - compaction_target_size: 8 * 1024, - ..TenantConf::default() + checkpoint_distance: Some(16 * 1024), + compaction_target_size: Some(8 * 1024), + ..Default::default() }; let harness = TenantHarness::create_custom( @@ -7254,9 +7208,9 @@ mod tests { compaction_algorithm: CompactionAlgorithm, ) -> anyhow::Result<()> { let mut harness = TenantHarness::create(name).await?; - harness.tenant_conf.compaction_algorithm = CompactionAlgorithmSettings { + harness.tenant_conf.compaction_algorithm = Some(CompactionAlgorithmSettings { kind: compaction_algorithm, - }; + }); let (tenant, ctx) = harness.load().await; let tline = tenant .create_test_timeline(TIMELINE_ID, Lsn(0x10), DEFAULT_PG_VERSION, &ctx) @@ -7623,9 +7577,9 @@ mod tests { compaction_algorithm: CompactionAlgorithm, ) -> anyhow::Result<()> { let mut harness = TenantHarness::create(name).await?; - harness.tenant_conf.compaction_algorithm = CompactionAlgorithmSettings { + harness.tenant_conf.compaction_algorithm = Some(CompactionAlgorithmSettings { kind: compaction_algorithm, - }; + }); let (tenant, ctx) = harness.load().await; let tline = tenant .create_test_timeline(TIMELINE_ID, Lsn(0x08), DEFAULT_PG_VERSION, &ctx) diff --git a/pageserver/src/tenant/config.rs b/pageserver/src/tenant/config.rs index 4308db84e5..bf82fc8df8 100644 --- a/pageserver/src/tenant/config.rs +++ b/pageserver/src/tenant/config.rs @@ -8,19 +8,11 @@ //! We cannot use global or default config instead, because wrong settings //! may lead to a data loss. //! -use std::num::NonZeroU64; -use std::time::Duration; -pub(crate) use pageserver_api::config::TenantConfigToml as TenantConf; -use pageserver_api::models::{ - self, CompactionAlgorithmSettings, EvictionPolicy, TenantConfigPatch, -}; +use pageserver_api::models; use pageserver_api::shard::{ShardCount, ShardIdentity, ShardNumber, ShardStripeSize}; -use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; -use serde_json::Value; use utils::generation::Generation; -use utils::postgres_client::PostgresClientProtocol; #[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq, Eq)] pub(crate) enum AttachmentMode { @@ -74,7 +66,7 @@ pub(crate) struct LocationConf { pub(crate) shard: ShardIdentity, /// The pan-cluster tenant configuration, the same on all locations - pub(crate) tenant_conf: TenantConfOpt, + pub(crate) tenant_conf: pageserver_api::models::TenantConfig, } impl std::fmt::Debug for LocationConf { @@ -140,7 +132,7 @@ impl LocationConf { /// implies it is in AttachmentMode::Single, which used to be the only /// possible state. This function should eventually be removed. pub(crate) fn attached_single( - tenant_conf: TenantConfOpt, + tenant_conf: pageserver_api::models::TenantConfig, generation: Generation, shard_params: &models::ShardParameters, ) -> Self { @@ -174,7 +166,7 @@ impl LocationConf { } pub(crate) fn try_from(conf: &'_ models::LocationConfig) -> anyhow::Result { - let tenant_conf = TenantConfOpt::try_from(&conf.tenant_conf)?; + let tenant_conf = conf.tenant_conf.clone(); fn get_generation(conf: &'_ models::LocationConfig) -> Result { conf.generation @@ -250,509 +242,19 @@ impl Default for LocationConf { generation: Generation::none(), attach_mode: AttachmentMode::Single, }), - tenant_conf: TenantConfOpt::default(), + tenant_conf: pageserver_api::models::TenantConfig::default(), shard: ShardIdentity::unsharded(), } } } -/// Same as TenantConf, but this struct preserves the information about -/// which parameters are set and which are not. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] -pub struct TenantConfOpt { - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub checkpoint_distance: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub checkpoint_timeout: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_target_size: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub compaction_period: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_upper_limit: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_algorithm: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_l0_first: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub compaction_l0_semaphore: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub l0_flush_delay_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub l0_flush_stall_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub l0_flush_wait_upload: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub gc_horizon: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub gc_period: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub image_creation_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub pitr_interval: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub walreceiver_connect_timeout: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub lagging_wal_timeout: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub max_lsn_wal_lag: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub eviction_policy: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub min_resident_size_override: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub evictions_low_residence_duration_metric_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub heatmap_period: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub lazy_slru_download: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub timeline_get_throttle: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub image_layer_creation_check_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub image_creation_preempt_threshold: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub lsn_lease_length: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(with = "humantime_serde")] - #[serde(default)] - pub lsn_lease_length_for_ts: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - #[serde(default)] - pub timeline_offloading: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub wal_receiver_protocol_override: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub rel_size_v2_enabled: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub gc_compaction_enabled: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub gc_compaction_initial_threshold_kb: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - pub gc_compaction_ratio_percent: Option, -} - -impl TenantConfOpt { - pub fn merge(&self, global_conf: TenantConf) -> TenantConf { - TenantConf { - checkpoint_distance: self - .checkpoint_distance - .unwrap_or(global_conf.checkpoint_distance), - checkpoint_timeout: self - .checkpoint_timeout - .unwrap_or(global_conf.checkpoint_timeout), - compaction_target_size: self - .compaction_target_size - .unwrap_or(global_conf.compaction_target_size), - compaction_period: self - .compaction_period - .unwrap_or(global_conf.compaction_period), - compaction_threshold: self - .compaction_threshold - .unwrap_or(global_conf.compaction_threshold), - compaction_upper_limit: self - .compaction_upper_limit - .unwrap_or(global_conf.compaction_upper_limit), - compaction_algorithm: self - .compaction_algorithm - .as_ref() - .unwrap_or(&global_conf.compaction_algorithm) - .clone(), - compaction_l0_first: self - .compaction_l0_first - .unwrap_or(global_conf.compaction_l0_first), - compaction_l0_semaphore: self - .compaction_l0_semaphore - .unwrap_or(global_conf.compaction_l0_semaphore), - l0_flush_delay_threshold: self - .l0_flush_delay_threshold - .or(global_conf.l0_flush_delay_threshold), - l0_flush_stall_threshold: self - .l0_flush_stall_threshold - .or(global_conf.l0_flush_stall_threshold), - l0_flush_wait_upload: self - .l0_flush_wait_upload - .unwrap_or(global_conf.l0_flush_wait_upload), - gc_horizon: self.gc_horizon.unwrap_or(global_conf.gc_horizon), - gc_period: self.gc_period.unwrap_or(global_conf.gc_period), - image_creation_threshold: self - .image_creation_threshold - .unwrap_or(global_conf.image_creation_threshold), - pitr_interval: self.pitr_interval.unwrap_or(global_conf.pitr_interval), - walreceiver_connect_timeout: self - .walreceiver_connect_timeout - .unwrap_or(global_conf.walreceiver_connect_timeout), - lagging_wal_timeout: self - .lagging_wal_timeout - .unwrap_or(global_conf.lagging_wal_timeout), - max_lsn_wal_lag: self.max_lsn_wal_lag.unwrap_or(global_conf.max_lsn_wal_lag), - eviction_policy: self.eviction_policy.unwrap_or(global_conf.eviction_policy), - min_resident_size_override: self - .min_resident_size_override - .or(global_conf.min_resident_size_override), - evictions_low_residence_duration_metric_threshold: self - .evictions_low_residence_duration_metric_threshold - .unwrap_or(global_conf.evictions_low_residence_duration_metric_threshold), - heatmap_period: self.heatmap_period.unwrap_or(global_conf.heatmap_period), - lazy_slru_download: self - .lazy_slru_download - .unwrap_or(global_conf.lazy_slru_download), - timeline_get_throttle: self - .timeline_get_throttle - .clone() - .unwrap_or(global_conf.timeline_get_throttle), - image_layer_creation_check_threshold: self - .image_layer_creation_check_threshold - .unwrap_or(global_conf.image_layer_creation_check_threshold), - image_creation_preempt_threshold: self - .image_creation_preempt_threshold - .unwrap_or(global_conf.image_creation_preempt_threshold), - lsn_lease_length: self - .lsn_lease_length - .unwrap_or(global_conf.lsn_lease_length), - lsn_lease_length_for_ts: self - .lsn_lease_length_for_ts - .unwrap_or(global_conf.lsn_lease_length_for_ts), - timeline_offloading: self - .timeline_offloading - .unwrap_or(global_conf.timeline_offloading), - wal_receiver_protocol_override: self - .wal_receiver_protocol_override - .or(global_conf.wal_receiver_protocol_override), - rel_size_v2_enabled: self - .rel_size_v2_enabled - .unwrap_or(global_conf.rel_size_v2_enabled), - gc_compaction_enabled: self - .gc_compaction_enabled - .unwrap_or(global_conf.gc_compaction_enabled), - gc_compaction_initial_threshold_kb: self - .gc_compaction_initial_threshold_kb - .unwrap_or(global_conf.gc_compaction_initial_threshold_kb), - gc_compaction_ratio_percent: self - .gc_compaction_ratio_percent - .unwrap_or(global_conf.gc_compaction_ratio_percent), - } - } - - pub fn apply_patch(self, patch: TenantConfigPatch) -> anyhow::Result { - let Self { - mut checkpoint_distance, - mut checkpoint_timeout, - mut compaction_target_size, - mut compaction_period, - mut compaction_threshold, - mut compaction_upper_limit, - mut compaction_algorithm, - mut compaction_l0_first, - mut compaction_l0_semaphore, - mut l0_flush_delay_threshold, - mut l0_flush_stall_threshold, - mut l0_flush_wait_upload, - mut gc_horizon, - mut gc_period, - mut image_creation_threshold, - mut pitr_interval, - mut walreceiver_connect_timeout, - mut lagging_wal_timeout, - mut max_lsn_wal_lag, - mut eviction_policy, - mut min_resident_size_override, - mut evictions_low_residence_duration_metric_threshold, - mut heatmap_period, - mut lazy_slru_download, - mut timeline_get_throttle, - mut image_layer_creation_check_threshold, - mut image_creation_preempt_threshold, - mut lsn_lease_length, - mut lsn_lease_length_for_ts, - mut timeline_offloading, - mut wal_receiver_protocol_override, - mut rel_size_v2_enabled, - mut gc_compaction_enabled, - mut gc_compaction_initial_threshold_kb, - mut gc_compaction_ratio_percent, - } = self; - - patch.checkpoint_distance.apply(&mut checkpoint_distance); - patch - .checkpoint_timeout - .map(|v| humantime::parse_duration(&v))? - .apply(&mut checkpoint_timeout); - patch - .compaction_target_size - .apply(&mut compaction_target_size); - patch - .compaction_period - .map(|v| humantime::parse_duration(&v))? - .apply(&mut compaction_period); - patch.compaction_threshold.apply(&mut compaction_threshold); - patch - .compaction_upper_limit - .apply(&mut compaction_upper_limit); - patch.compaction_algorithm.apply(&mut compaction_algorithm); - patch.compaction_l0_first.apply(&mut compaction_l0_first); - patch - .compaction_l0_semaphore - .apply(&mut compaction_l0_semaphore); - patch - .l0_flush_delay_threshold - .apply(&mut l0_flush_delay_threshold); - patch - .l0_flush_stall_threshold - .apply(&mut l0_flush_stall_threshold); - patch.l0_flush_wait_upload.apply(&mut l0_flush_wait_upload); - patch.gc_horizon.apply(&mut gc_horizon); - patch - .gc_period - .map(|v| humantime::parse_duration(&v))? - .apply(&mut gc_period); - patch - .image_creation_threshold - .apply(&mut image_creation_threshold); - patch - .pitr_interval - .map(|v| humantime::parse_duration(&v))? - .apply(&mut pitr_interval); - patch - .walreceiver_connect_timeout - .map(|v| humantime::parse_duration(&v))? - .apply(&mut walreceiver_connect_timeout); - patch - .lagging_wal_timeout - .map(|v| humantime::parse_duration(&v))? - .apply(&mut lagging_wal_timeout); - patch.max_lsn_wal_lag.apply(&mut max_lsn_wal_lag); - patch.eviction_policy.apply(&mut eviction_policy); - patch - .min_resident_size_override - .apply(&mut min_resident_size_override); - patch - .evictions_low_residence_duration_metric_threshold - .map(|v| humantime::parse_duration(&v))? - .apply(&mut evictions_low_residence_duration_metric_threshold); - patch - .heatmap_period - .map(|v| humantime::parse_duration(&v))? - .apply(&mut heatmap_period); - patch.lazy_slru_download.apply(&mut lazy_slru_download); - patch - .timeline_get_throttle - .apply(&mut timeline_get_throttle); - patch - .image_layer_creation_check_threshold - .apply(&mut image_layer_creation_check_threshold); - patch - .image_creation_preempt_threshold - .apply(&mut image_creation_preempt_threshold); - patch - .lsn_lease_length - .map(|v| humantime::parse_duration(&v))? - .apply(&mut lsn_lease_length); - patch - .lsn_lease_length_for_ts - .map(|v| humantime::parse_duration(&v))? - .apply(&mut lsn_lease_length_for_ts); - patch.timeline_offloading.apply(&mut timeline_offloading); - patch - .wal_receiver_protocol_override - .apply(&mut wal_receiver_protocol_override); - patch.rel_size_v2_enabled.apply(&mut rel_size_v2_enabled); - patch - .gc_compaction_enabled - .apply(&mut gc_compaction_enabled); - patch - .gc_compaction_initial_threshold_kb - .apply(&mut gc_compaction_initial_threshold_kb); - patch - .gc_compaction_ratio_percent - .apply(&mut gc_compaction_ratio_percent); - - Ok(Self { - checkpoint_distance, - checkpoint_timeout, - compaction_target_size, - compaction_period, - compaction_threshold, - compaction_upper_limit, - compaction_algorithm, - compaction_l0_first, - compaction_l0_semaphore, - l0_flush_delay_threshold, - l0_flush_stall_threshold, - l0_flush_wait_upload, - gc_horizon, - gc_period, - image_creation_threshold, - pitr_interval, - walreceiver_connect_timeout, - lagging_wal_timeout, - max_lsn_wal_lag, - eviction_policy, - min_resident_size_override, - evictions_low_residence_duration_metric_threshold, - heatmap_period, - lazy_slru_download, - timeline_get_throttle, - image_layer_creation_check_threshold, - image_creation_preempt_threshold, - lsn_lease_length, - lsn_lease_length_for_ts, - timeline_offloading, - wal_receiver_protocol_override, - rel_size_v2_enabled, - gc_compaction_enabled, - gc_compaction_initial_threshold_kb, - gc_compaction_ratio_percent, - }) - } -} - -impl TryFrom<&'_ models::TenantConfig> for TenantConfOpt { - type Error = anyhow::Error; - - fn try_from(request_data: &'_ models::TenantConfig) -> Result { - // Convert the request_data to a JSON Value - let json_value: Value = serde_json::to_value(request_data)?; - - // Create a Deserializer from the JSON Value - let deserializer = json_value.into_deserializer(); - - // Use serde_path_to_error to deserialize the JSON Value into TenantConfOpt - let tenant_conf: TenantConfOpt = serde_path_to_error::deserialize(deserializer)?; - - Ok(tenant_conf) - } -} - -/// This is a conversion from our internal tenant config object to the one used -/// in external APIs. -impl From for models::TenantConfig { - // TODO(vlad): These are now the same, but they have different serialization logic. - // Can we merge them? - fn from(value: TenantConfOpt) -> Self { - Self { - checkpoint_distance: value.checkpoint_distance, - checkpoint_timeout: value.checkpoint_timeout, - compaction_algorithm: value.compaction_algorithm, - compaction_target_size: value.compaction_target_size, - compaction_period: value.compaction_period, - compaction_threshold: value.compaction_threshold, - compaction_upper_limit: value.compaction_upper_limit, - compaction_l0_first: value.compaction_l0_first, - compaction_l0_semaphore: value.compaction_l0_semaphore, - l0_flush_delay_threshold: value.l0_flush_delay_threshold, - l0_flush_stall_threshold: value.l0_flush_stall_threshold, - l0_flush_wait_upload: value.l0_flush_wait_upload, - gc_horizon: value.gc_horizon, - gc_period: value.gc_period, - image_creation_threshold: value.image_creation_threshold, - pitr_interval: value.pitr_interval, - walreceiver_connect_timeout: value.walreceiver_connect_timeout, - lagging_wal_timeout: value.lagging_wal_timeout, - max_lsn_wal_lag: value.max_lsn_wal_lag, - eviction_policy: value.eviction_policy, - min_resident_size_override: value.min_resident_size_override, - evictions_low_residence_duration_metric_threshold: value - .evictions_low_residence_duration_metric_threshold, - heatmap_period: value.heatmap_period, - lazy_slru_download: value.lazy_slru_download, - timeline_get_throttle: value.timeline_get_throttle, - image_layer_creation_check_threshold: value.image_layer_creation_check_threshold, - image_creation_preempt_threshold: value.image_creation_preempt_threshold, - lsn_lease_length: value.lsn_lease_length, - lsn_lease_length_for_ts: value.lsn_lease_length_for_ts, - timeline_offloading: value.timeline_offloading, - wal_receiver_protocol_override: value.wal_receiver_protocol_override, - rel_size_v2_enabled: value.rel_size_v2_enabled, - gc_compaction_enabled: value.gc_compaction_enabled, - gc_compaction_initial_threshold_kb: value.gc_compaction_initial_threshold_kb, - gc_compaction_ratio_percent: value.gc_compaction_ratio_percent, - } - } -} - #[cfg(test)] mod tests { - use models::TenantConfig; - - use super::*; - #[test] - fn de_serializing_pageserver_config_omits_empty_values() { - let small_conf = TenantConfOpt { + fn serde_roundtrip_tenant_conf_opt() { + let small_conf = pageserver_api::models::TenantConfig { gc_horizon: Some(42), - ..TenantConfOpt::default() + ..Default::default() }; let toml_form = toml_edit::ser::to_string(&small_conf).unwrap(); @@ -763,19 +265,4 @@ mod tests { assert_eq!(json_form, "{\"gc_horizon\":42}"); assert_eq!(small_conf, serde_json::from_str(&json_form).unwrap()); } - - #[test] - fn test_try_from_models_tenant_config_success() { - let tenant_config = models::TenantConfig { - lagging_wal_timeout: Some(Duration::from_secs(5)), - ..TenantConfig::default() - }; - - let tenant_conf_opt = TenantConfOpt::try_from(&tenant_config).unwrap(); - - assert_eq!( - tenant_conf_opt.lagging_wal_timeout, - Some(Duration::from_secs(5)) - ); - } } diff --git a/pageserver/src/tenant/secondary.rs b/pageserver/src/tenant/secondary.rs index 8f8622c796..a378961620 100644 --- a/pageserver/src/tenant/secondary.rs +++ b/pageserver/src/tenant/secondary.rs @@ -20,7 +20,7 @@ use utils::sync::gate::Gate; use self::downloader::{SecondaryDetail, downloader_task}; use self::heatmap_uploader::heatmap_uploader_task; use super::GetTenantError; -use super::config::{SecondaryLocationConfig, TenantConfOpt}; +use super::config::SecondaryLocationConfig; use super::mgr::TenantManager; use super::span::debug_assert_current_span_has_tenant_id; use super::storage_layer::LayerName; @@ -98,11 +98,11 @@ pub(crate) struct SecondaryTenant { pub(crate) gate: Gate, - // Secondary mode does not need the full shard identity or the TenantConfOpt. However, + // Secondary mode does not need the full shard identity or the pageserver_api::models::TenantConfig. However, // storing these enables us to report our full LocationConf, enabling convenient reconciliation // by the control plane (see [`Self::get_location_conf`]) shard_identity: ShardIdentity, - tenant_conf: std::sync::Mutex, + tenant_conf: std::sync::Mutex, // Internal state used by the Downloader. detail: std::sync::Mutex, @@ -121,7 +121,7 @@ impl SecondaryTenant { pub(crate) fn new( tenant_shard_id: TenantShardId, shard_identity: ShardIdentity, - tenant_conf: TenantConfOpt, + tenant_conf: pageserver_api::models::TenantConfig, config: &SecondaryLocationConfig, ) -> Arc { let tenant_id = tenant_shard_id.tenant_id.to_string(); @@ -177,7 +177,7 @@ impl SecondaryTenant { self.detail.lock().unwrap().config = config.clone(); } - pub(crate) fn set_tenant_conf(&self, config: &TenantConfOpt) { + pub(crate) fn set_tenant_conf(&self, config: &pageserver_api::models::TenantConfig) { *(self.tenant_conf.lock().unwrap()) = config.clone(); } @@ -197,7 +197,7 @@ impl SecondaryTenant { shard_number: self.tenant_shard_id.shard_number.0, shard_count: self.tenant_shard_id.shard_count.literal(), shard_stripe_size: self.shard_identity.stripe_size.0, - tenant_conf: tenant_conf.into(), + tenant_conf, } } diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 2e6cee036c..b211eb5416 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -1148,7 +1148,6 @@ mod test { use super::{ImageLayerIterator, ImageLayerWriter}; use crate::DEFAULT_PG_VERSION; use crate::context::RequestContext; - use crate::tenant::config::TenantConf; use crate::tenant::harness::{TIMELINE_ID, TenantHarness}; use crate::tenant::storage_layer::{Layer, ResidentLayer}; use crate::tenant::vectored_blob_io::StreamingVectoredReadPlanner; @@ -1156,10 +1155,10 @@ mod test { #[tokio::test] async fn image_layer_rewrite() { - let tenant_conf = TenantConf { - gc_period: Duration::ZERO, - compaction_period: Duration::ZERO, - ..TenantConf::default() + let tenant_conf = pageserver_api::models::TenantConfig { + gc_period: Some(Duration::ZERO), + compaction_period: Some(Duration::ZERO), + ..Default::default() }; let tenant_id = TenantId::generate(); let mut gen_ = Generation::new(0xdead0001); diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index ce2b8c4f1a..a4b59603d1 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -84,7 +84,6 @@ use self::eviction_task::EvictionTaskTimelineState; use self::layer_manager::LayerManager; use self::logical_size::LogicalSize; use self::walreceiver::{WalReceiver, WalReceiverConf}; -use super::config::TenantConf; use super::remote_timeline_client::index::{GcCompactionState, IndexPart}; use super::remote_timeline_client::{RemoteTimelineClient, WaitCompletionError}; use super::secondary::heatmap::HeatMapLayer; @@ -111,7 +110,7 @@ use crate::pgdatadir_mapping::{ MAX_AUX_FILE_V2_DELTAS, MetricsUpdate, }; use crate::task_mgr::TaskKind; -use crate::tenant::config::{AttachmentMode, TenantConfOpt}; +use crate::tenant::config::AttachmentMode; use crate::tenant::gc_result::GcResult; use crate::tenant::layer_map::{LayerMap, SearchResult}; use crate::tenant::metadata::TimelineMetadata; @@ -536,11 +535,11 @@ impl GcInfo { /// between time-based and space-based retention for observability and consumption metrics purposes. #[derive(Debug, Clone)] pub(crate) struct GcCutoffs { - /// Calculated from the [`TenantConf::gc_horizon`], this LSN indicates how much + /// Calculated from the [`pageserver_api::models::TenantConfig::gc_horizon`], this LSN indicates how much /// history we must keep to retain a specified number of bytes of WAL. pub(crate) space: Lsn, - /// Calculated from [`TenantConf::pitr_interval`], this LSN indicates how much + /// Calculated from [`pageserver_api::models::TenantConfig::pitr_interval`], this LSN indicates how much /// history we must keep to enable reading back at least the PITR interval duration. pub(crate) time: Lsn, } @@ -2598,8 +2597,8 @@ impl Timeline { } fn get_evictions_low_residence_duration_metric_threshold( - tenant_conf: &TenantConfOpt, - default_tenant_conf: &TenantConf, + tenant_conf: &pageserver_api::models::TenantConfig, + default_tenant_conf: &pageserver_api::config::TenantConfigToml, ) -> Duration { tenant_conf .evictions_low_residence_duration_metric_threshold diff --git a/test_runner/regress/test_attach_tenant_config.py b/test_runner/regress/test_attach_tenant_config.py index b56fcd3500..8360072a2d 100644 --- a/test_runner/regress/test_attach_tenant_config.py +++ b/test_runner/regress/test_attach_tenant_config.py @@ -5,11 +5,12 @@ from dataclasses import dataclass import pytest from fixtures.common_types import TenantId +from fixtures.log_helper import log from fixtures.neon_fixtures import ( NeonEnv, NeonEnvBuilder, ) -from fixtures.pageserver.http import TenantConfig +from fixtures.pageserver.http import PageserverHttpClient, TenantConfig from fixtures.remote_storage import LocalFsStorage, RemoteStorageKind from fixtures.utils import wait_until @@ -159,7 +160,6 @@ def test_fully_custom_config(positive_env: NeonEnv): "evictions_low_residence_duration_metric_threshold": "2days", "gc_horizon": 23 * (1024 * 1024), "gc_period": "2h 13m", - "heatmap_period": "10m", "image_creation_threshold": 7, "pitr_interval": "1m", "lagging_wal_timeout": "23m", @@ -182,7 +182,7 @@ def test_fully_custom_config(positive_env: NeonEnv): "type": "interpreted", "args": {"format": "bincode", "compression": {"zstd": {"level": 1}}}, }, - "rel_size_v2_enabled": True, + "rel_size_v2_enabled": False, # test suite enables it by default as of https://github.com/neondatabase/neon/issues/11081, so, custom config means disabling it "gc_compaction_enabled": True, "gc_compaction_initial_threshold_kb": 1024000, "gc_compaction_ratio_percent": 200, @@ -190,36 +190,66 @@ def test_fully_custom_config(positive_env: NeonEnv): } vps_http = env.storage_controller.pageserver_api() + ps_http = env.pageserver.http_client() - initial_tenant_config = vps_http.tenant_config(env.initial_tenant) - assert [ - (key, val) - for key, val in initial_tenant_config.tenant_specific_overrides.items() - if val is not None - ] == [] + def get_config(client: PageserverHttpClient, tenant_id): + ignored_fields = [ + # storcon overrides this during reconciles, and + # this test triggers reconciles when we change the + # tenant config via vps_http + "heatmap_period" + ] + config = client.tenant_config(tenant_id) + for field in ignored_fields: + config.effective_config.pop(field, None) + config.tenant_specific_overrides.pop(field, None) + return config + + # storcon returns its db state in GET tenant_config in both fields + # https://github.com/neondatabase/neon/issues/9621 + initial_tenant_config = get_config(vps_http, env.initial_tenant) + assert initial_tenant_config.tenant_specific_overrides == {} + assert initial_tenant_config.tenant_specific_overrides == initial_tenant_config.effective_config + + # pageserver has built-in defaults for all config options + # also self-test that our fully_custom_config covers all of them + initial_tenant_config = get_config(ps_http, env.initial_tenant) + assert initial_tenant_config.tenant_specific_overrides == {} assert set(initial_tenant_config.effective_config.keys()) == set( fully_custom_config.keys() ), "ensure we cover all config options" + # create a new tenant to test overrides (tenant_id, _) = env.create_tenant() vps_http.set_tenant_config(tenant_id, fully_custom_config) - our_tenant_config = vps_http.tenant_config(tenant_id) - assert our_tenant_config.tenant_specific_overrides == fully_custom_config - assert set(our_tenant_config.effective_config.keys()) == set( - fully_custom_config.keys() - ), "ensure we cover all config options" - assert ( - { - k: initial_tenant_config.effective_config[k] != our_tenant_config.effective_config[k] - for k in fully_custom_config.keys() - } - == {k: True for k in fully_custom_config.keys()} - ), "ensure our custom config has different values than the default config for all config options, so we know we overrode everything" - env.pageserver.tenant_detach(tenant_id) - env.pageserver.tenant_attach(tenant_id, config=fully_custom_config) + for iteration in ["first", "after-reattach"]: + log.info(f"iteration: {iteration}") - assert vps_http.tenant_config(tenant_id).tenant_specific_overrides == fully_custom_config - assert set(vps_http.tenant_config(tenant_id).effective_config.keys()) == set( - fully_custom_config.keys() - ), "ensure we cover all config options" + # validate that overrides for all fields are returned by storcon + our_tenant_config = get_config(vps_http, tenant_id) + assert our_tenant_config.tenant_specific_overrides == fully_custom_config + assert our_tenant_config.tenant_specific_overrides == our_tenant_config.effective_config + + # validate that overrides for all fields reached pageserver + our_tenant_config = get_config(ps_http, tenant_id) + assert our_tenant_config.tenant_specific_overrides == fully_custom_config + assert our_tenant_config.tenant_specific_overrides == our_tenant_config.effective_config + + # some more self-validation: assert that none of the values in our + # fully custom config are the same as the default values + assert set(our_tenant_config.effective_config.keys()) == set( + fully_custom_config.keys() + ), "ensure we cover all config options" + assert ( + { + k: initial_tenant_config.effective_config[k] + != our_tenant_config.effective_config[k] + for k in fully_custom_config.keys() + } + == {k: True for k in fully_custom_config.keys()} + ), "ensure our custom config has different values than the default config for all config options, so we know we overrode everything" + + # ensure customizations survive reattach + env.pageserver.tenant_detach(tenant_id) + env.pageserver.tenant_attach(tenant_id, config=fully_custom_config) diff --git a/test_runner/regress/test_storage_controller.py b/test_runner/regress/test_storage_controller.py index 05eb4301b0..3822ac8e88 100644 --- a/test_runner/regress/test_storage_controller.py +++ b/test_runner/regress/test_storage_controller.py @@ -1374,8 +1374,9 @@ def test_storage_controller_tenant_conf(neon_env_builder: NeonEnvBuilder): # vs. pageserver API calls, because pageserver has defaults. http.set_tenant_config(tenant_id, {}) readback_controller = http.tenant_config(tenant_id) - assert readback_controller.effective_config["pitr_interval"] is None - assert readback_controller.tenant_specific_overrides["pitr_interval"] is None + + assert "pitr_interval" not in readback_controller.effective_config.keys() + assert "pitr_interval" not in readback_controller.tenant_specific_overrides.keys() readback_ps = env.pageservers[0].http_client().tenant_config(tenant_id) assert readback_ps.effective_config["pitr_interval"] == default_value assert "pitr_interval" not in readback_ps.tenant_specific_overrides diff --git a/test_runner/regress/test_threshold_based_eviction.py b/test_runner/regress/test_threshold_based_eviction.py index c87b520366..b16448ef00 100644 --- a/test_runner/regress/test_threshold_based_eviction.py +++ b/test_runner/regress/test_threshold_based_eviction.py @@ -54,7 +54,12 @@ def test_threshold_based_eviction( ps_http = env.pageserver.http_client() vps_http = env.storage_controller.pageserver_api() - assert vps_http.tenant_config(tenant_id).effective_config["eviction_policy"] is None + # check config on pageserver, set via storcon; https://github.com/neondatabase/neon/issues/9621 + + assert ps_http.tenant_config(tenant_id).tenant_specific_overrides == {} + assert ps_http.tenant_config(tenant_id).effective_config["eviction_policy"] == { + "kind": "NoEviction" + } eviction_threshold = 10 eviction_period = 2 @@ -68,7 +73,7 @@ def test_threshold_based_eviction( }, }, ) - assert vps_http.tenant_config(tenant_id).effective_config["eviction_policy"] == { + assert ps_http.tenant_config(tenant_id).effective_config["eviction_policy"] == { "kind": "LayerAccessThreshold", "threshold": f"{eviction_threshold}s", "period": f"{eviction_period}s", @@ -77,7 +82,7 @@ def test_threshold_based_eviction( # restart because changing tenant config is not instant env.pageserver.restart() - assert vps_http.tenant_config(tenant_id).effective_config["eviction_policy"] == { + assert ps_http.tenant_config(tenant_id).effective_config["eviction_policy"] == { "kind": "LayerAccessThreshold", "threshold": f"{eviction_threshold}s", "period": f"{eviction_period}s",