From 8c7136b057e80fb2d48feec7ca04088408318096 Mon Sep 17 00:00:00 2001 From: Konstantin Knizhnik Date: Mon, 11 Mar 2024 22:13:11 +0200 Subject: [PATCH] Add compress_image_layer property to TenantConfig --- control_plane/src/pageserver.rs | 10 +++++++ libs/pageserver_api/src/models.rs | 1 + pageserver/src/config.rs | 8 ++++- pageserver/src/tenant.rs | 8 +++++ pageserver/src/tenant/config.rs | 10 +++++++ .../src/tenant/storage_layer/image_layer.rs | 30 +++++++++++-------- pageserver/src/tenant/timeline.rs | 18 +++++------ pageserver/src/tenant/timeline/compaction.rs | 9 +----- 8 files changed, 63 insertions(+), 31 deletions(-) diff --git a/control_plane/src/pageserver.rs b/control_plane/src/pageserver.rs index 06ec942895..b9e226ebcd 100644 --- a/control_plane/src/pageserver.rs +++ b/control_plane/src/pageserver.rs @@ -384,6 +384,11 @@ impl PageServerNode { .map(|x| x.parse::()) .transpose() .context("Failed to parse 'trace_read_requests' as bool")?, + compress_image_layer: settings + .remove("compress_image_layer") + .map(|x| x.parse::()) + .transpose() + .context("Failed to parse 'compress_image_layer' as bool")?, eviction_policy: settings .remove("eviction_policy") .map(serde_json::from_str) @@ -496,6 +501,11 @@ impl PageServerNode { .map(|x| x.parse::()) .transpose() .context("Failed to parse 'trace_read_requests' as bool")?, + compress_image_layer: settings + .remove("compress_image_layer") + .map(|x| x.parse::()) + .transpose() + .context("Failed to parse 'compress_image_layer' as bool")?, eviction_policy: settings .remove("eviction_policy") .map(serde_json::from_str) diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 3aa84f8903..259c080483 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -294,6 +294,7 @@ pub struct TenantConfig { pub lagging_wal_timeout: Option, pub max_lsn_wal_lag: Option, pub trace_read_requests: Option, + pub compress_image_layer: Option, pub eviction_policy: Option, pub min_resident_size_override: Option, pub evictions_low_residence_duration_metric_threshold: Option, diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 845b20c8db..74707090f6 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -1538,6 +1538,7 @@ broker_endpoint = '{broker_endpoint}' let broker_endpoint = "http://127.0.0.1:7777"; let trace_read_requests = true; + let compress_image_layer = true; let config_string = format!( r#"{ALL_BASE_VALUES_TOML} @@ -1545,7 +1546,8 @@ pg_distrib_dir='{pg_distrib_dir}' broker_endpoint = '{broker_endpoint}' [tenant_config] -trace_read_requests = {trace_read_requests}"#, +trace_read_requests = {trace_read_requests}, +compress_image_layer = {compress_image_layer}"#, ); let toml = config_string.parse()?; @@ -1555,6 +1557,10 @@ trace_read_requests = {trace_read_requests}"#, conf.default_tenant_conf.trace_read_requests, trace_read_requests, "Tenant config from pageserver config file should be parsed and udpated values used as defaults for all tenants", ); + assert_eq!( + conf.default_tenant_conf.compress_image_layer, compress_image_layer, + "Tenant config from pageserver config file should be parsed and udpated values used as defaults for all tenants", + ); Ok(()) } diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index f0996328c0..94b5b00194 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -2288,6 +2288,13 @@ impl Tenant { .unwrap_or(self.conf.default_tenant_conf.trace_read_requests) } + pub fn get_compress_image_layer(&self) -> bool { + let tenant_conf = self.tenant_conf.read().unwrap().tenant_conf.clone(); + tenant_conf + .compress_image_layer + .unwrap_or(self.conf.default_tenant_conf.compress_image_layer) + } + pub fn get_min_resident_size_override(&self) -> Option { let tenant_conf = self.tenant_conf.read().unwrap().tenant_conf.clone(); tenant_conf @@ -3637,6 +3644,7 @@ pub(crate) mod harness { lagging_wal_timeout: Some(tenant_conf.lagging_wal_timeout), max_lsn_wal_lag: Some(tenant_conf.max_lsn_wal_lag), trace_read_requests: Some(tenant_conf.trace_read_requests), + compress_image_layer: Some(tenant_conf.compress_image_layer), eviction_policy: Some(tenant_conf.eviction_policy), min_resident_size_override: tenant_conf.min_resident_size_override, evictions_low_residence_duration_metric_threshold: Some( diff --git a/pageserver/src/tenant/config.rs b/pageserver/src/tenant/config.rs index 57fc444cdd..bcb1368911 100644 --- a/pageserver/src/tenant/config.rs +++ b/pageserver/src/tenant/config.rs @@ -345,6 +345,7 @@ pub struct TenantConf { /// to avoid eager reconnects. pub max_lsn_wal_lag: NonZeroU64, pub trace_read_requests: bool, + pub compress_image_layer: bool, pub eviction_policy: EvictionPolicy, pub min_resident_size_override: Option, // See the corresponding metric's help string. @@ -429,6 +430,10 @@ pub struct TenantConfOpt { #[serde(default)] pub trace_read_requests: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] + pub compress_image_layer: Option, + #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] pub eviction_policy: Option, @@ -492,6 +497,9 @@ impl TenantConfOpt { trace_read_requests: self .trace_read_requests .unwrap_or(global_conf.trace_read_requests), + compress_image_layer: self + .compress_image_layer + .unwrap_or(global_conf.compress_image_layer), eviction_policy: self.eviction_policy.unwrap_or(global_conf.eviction_policy), min_resident_size_override: self .min_resident_size_override @@ -538,6 +546,7 @@ impl Default for TenantConf { max_lsn_wal_lag: NonZeroU64::new(DEFAULT_MAX_WALRECEIVER_LSN_WAL_LAG) .expect("cannot parse default max walreceiver Lsn wal lag"), trace_read_requests: false, + compress_image_layer: false, eviction_policy: EvictionPolicy::NoEviction, min_resident_size_override: None, evictions_low_residence_duration_metric_threshold: humantime::parse_duration( @@ -612,6 +621,7 @@ impl From for models::TenantConfig { lagging_wal_timeout: value.lagging_wal_timeout.map(humantime), max_lsn_wal_lag: value.max_lsn_wal_lag, trace_read_requests: value.trace_read_requests, + compress_image_layer: value.compress_image_layer, eviction_policy: value.eviction_policy, min_resident_size_override: value.min_resident_size_override, evictions_low_residence_duration_metric_threshold: value diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 0ca356646a..d7a000e549 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -598,6 +598,7 @@ struct ImageLayerWriterInner { timeline_id: TimelineId, tenant_shard_id: TenantShardId, key_range: Range, + compression: bool, lsn: Lsn, blob_writer: BlobWriter, @@ -609,16 +610,17 @@ impl ImageLayerWriterInner { /// Start building a new image layer. /// async fn new( - conf: &'static PageServerConf, - timeline_id: TimelineId, - tenant_shard_id: TenantShardId, + timeline: &Arc, key_range: &Range, lsn: Lsn, ) -> anyhow::Result { + let timeline_id = timeline.timeline_id; + let tenant_shard_id = timeline.tenant_shard_id; + let compression = timeline.get_image_layer_compression(); // Create the file initially with a temporary filename. // We'll atomically rename it to the final name when we're done. let path = ImageLayer::temp_path_for( - conf, + timeline.conf, timeline_id, tenant_shard_id, &ImageFileName { @@ -645,11 +647,12 @@ impl ImageLayerWriterInner { let tree_builder = DiskBtreeBuilder::new(block_buf); let writer = Self { - conf, + conf: timeline.conf, path, timeline_id, tenant_shard_id, key_range: key_range.clone(), + compression, lsn, tree: tree_builder, blob_writer, @@ -665,7 +668,13 @@ impl ImageLayerWriterInner { /// async fn put_image(&mut self, key: Key, img: Bytes) -> anyhow::Result<()> { ensure!(self.key_range.contains(&key)); - let off = self.blob_writer.write_compressed_blob(img).await?; + let off = if self.compression { + self.blob_writer.write_compressed_blob(img).await? + } else { + let (_img, res) = self.blob_writer.write_blob(img).await; + // TODO: re-use the buffer for `img` further upstack + res? + }; let mut keybuf: [u8; KEY_SIZE] = [0u8; KEY_SIZE]; key.write_to_byte_slice(&mut keybuf); self.tree.append(&keybuf, off)?; @@ -770,17 +779,12 @@ impl ImageLayerWriter { /// Start building a new image layer. /// pub async fn new( - conf: &'static PageServerConf, - timeline_id: TimelineId, - tenant_shard_id: TenantShardId, + timeline: &Arc, key_range: &Range, lsn: Lsn, ) -> anyhow::Result { Ok(Self { - inner: Some( - ImageLayerWriterInner::new(conf, timeline_id, tenant_shard_id, key_range, lsn) - .await?, - ), + inner: Some(ImageLayerWriterInner::new(timeline, key_range, lsn).await?), }) } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index d507a19de9..ce255e216f 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -182,7 +182,7 @@ pub(crate) struct AuxFilesState { } pub struct Timeline { - conf: &'static PageServerConf, + pub(crate) conf: &'static PageServerConf, tenant_conf: Arc>, myself: Weak, @@ -1515,6 +1515,13 @@ impl Timeline { .unwrap_or(default_tenant_conf.evictions_low_residence_duration_metric_threshold) } + pub fn get_image_layer_compression(&self) -> bool { + let tenant_conf = self.tenant_conf.read().unwrap().tenant_conf.clone(); + tenant_conf + .compress_image_layer + .unwrap_or(self.conf.default_tenant_conf.compress_image_layer) + } + pub(super) fn tenant_conf_updated(&self) { // NB: Most tenant conf options are read by background loops, so, // changes will automatically be picked up. @@ -3458,14 +3465,7 @@ impl Timeline { continue; } - let mut image_layer_writer = ImageLayerWriter::new( - self.conf, - self.timeline_id, - self.tenant_shard_id, - &img_range, - lsn, - ) - .await?; + let mut image_layer_writer = ImageLayerWriter::new(self, &img_range, lsn).await?; fail_point!("image-layer-writer-fail-before-finish", |_| { Err(CreateImageLayersError::Other(anyhow::anyhow!( diff --git a/pageserver/src/tenant/timeline/compaction.rs b/pageserver/src/tenant/timeline/compaction.rs index 74b75dabf0..7d2634e8e9 100644 --- a/pageserver/src/tenant/timeline/compaction.rs +++ b/pageserver/src/tenant/timeline/compaction.rs @@ -997,14 +997,7 @@ impl TimelineAdaptor { ) -> Result<(), PageReconstructError> { let timer = self.timeline.metrics.create_images_time_histo.start_timer(); - let mut image_layer_writer = ImageLayerWriter::new( - self.timeline.conf, - self.timeline.timeline_id, - self.timeline.tenant_shard_id, - key_range, - lsn, - ) - .await?; + let mut image_layer_writer = ImageLayerWriter::new(&self.timeline, key_range, lsn).await?; fail_point!("image-layer-writer-fail-before-finish", |_| { Err(PageReconstructError::Other(anyhow::anyhow!(