From 88f39c11d47ae28d44d7dcea4995a5bdedea6512 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 4 May 2023 17:10:40 +0200 Subject: [PATCH] refactor: the code that builds TenantConfOpt from mgmt API requests (#4152) - extract code that builds TenantConfOpt from requests into a From<> impl - move map_err(ApiError::BadRequest) into callers --- pageserver/src/http/routes.rs | 176 +------------------------------- pageserver/src/tenant/config.rs | 175 +++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 174 deletions(-) diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index b1251123b2..58ee7367ca 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -691,16 +691,6 @@ pub fn html_response(status: StatusCode, data: String) -> Result, Ok(response) } -// Helper function to standardize the error messages we produce on bad durations -// -// Intended to be used with anyhow's `with_context`, e.g.: -// -// let value = result.with_context(bad_duration("name", &value))?; -// -fn bad_duration<'a>(field_name: &'static str, value: &'a str) -> impl 'a + Fn() -> String { - move || format!("Cannot parse `{field_name}` duration {value:?}") -} - async fn tenant_create_handler(mut request: Request) -> Result, ApiError> { check_permission(&request, None)?; @@ -708,91 +698,7 @@ async fn tenant_create_handler(mut request: Request) -> Result(field_name: &'static str, value: &'a str) -> impl 'a + Fn() -> String { + move || format!("Cannot parse `{field_name}` duration {value:?}") +} + +impl TryFrom<&'_ TenantCreateRequest> for TenantConfOpt { + type Error = anyhow::Error; + + fn try_from(request_data: &TenantCreateRequest) -> Result { + let mut tenant_conf = TenantConfOpt::default(); + + if let Some(gc_period) = &request_data.gc_period { + tenant_conf.gc_period = Some( + humantime::parse_duration(gc_period) + .with_context(bad_duration("gc_period", gc_period))?, + ); + } + tenant_conf.gc_horizon = request_data.gc_horizon; + tenant_conf.image_creation_threshold = request_data.image_creation_threshold; + + if let Some(pitr_interval) = &request_data.pitr_interval { + tenant_conf.pitr_interval = Some( + humantime::parse_duration(pitr_interval) + .with_context(bad_duration("pitr_interval", pitr_interval))?, + ); + } + + if let Some(walreceiver_connect_timeout) = &request_data.walreceiver_connect_timeout { + tenant_conf.walreceiver_connect_timeout = Some( + humantime::parse_duration(walreceiver_connect_timeout).with_context( + bad_duration("walreceiver_connect_timeout", walreceiver_connect_timeout), + )?, + ); + } + if let Some(lagging_wal_timeout) = &request_data.lagging_wal_timeout { + tenant_conf.lagging_wal_timeout = Some( + humantime::parse_duration(lagging_wal_timeout) + .with_context(bad_duration("lagging_wal_timeout", lagging_wal_timeout))?, + ); + } + if let Some(max_lsn_wal_lag) = request_data.max_lsn_wal_lag { + tenant_conf.max_lsn_wal_lag = Some(max_lsn_wal_lag); + } + if let Some(trace_read_requests) = request_data.trace_read_requests { + tenant_conf.trace_read_requests = Some(trace_read_requests); + } + + tenant_conf.checkpoint_distance = request_data.checkpoint_distance; + if let Some(checkpoint_timeout) = &request_data.checkpoint_timeout { + tenant_conf.checkpoint_timeout = Some( + humantime::parse_duration(checkpoint_timeout) + .with_context(bad_duration("checkpoint_timeout", checkpoint_timeout))?, + ); + } + + tenant_conf.compaction_target_size = request_data.compaction_target_size; + tenant_conf.compaction_threshold = request_data.compaction_threshold; + + if let Some(compaction_period) = &request_data.compaction_period { + tenant_conf.compaction_period = Some( + humantime::parse_duration(compaction_period) + .with_context(bad_duration("compaction_period", compaction_period))?, + ); + } + + if let Some(eviction_policy) = &request_data.eviction_policy { + tenant_conf.eviction_policy = Some( + serde::Deserialize::deserialize(eviction_policy) + .context("parse field `eviction_policy`")?, + ); + } + + tenant_conf.min_resident_size_override = request_data.min_resident_size_override; + + if let Some(evictions_low_residence_duration_metric_threshold) = + &request_data.evictions_low_residence_duration_metric_threshold + { + tenant_conf.evictions_low_residence_duration_metric_threshold = Some( + humantime::parse_duration(evictions_low_residence_duration_metric_threshold) + .with_context(bad_duration( + "evictions_low_residence_duration_metric_threshold", + evictions_low_residence_duration_metric_threshold, + ))?, + ); + } + + Ok(tenant_conf) + } +} + +impl TryFrom<&'_ TenantConfigRequest> for TenantConfOpt { + type Error = anyhow::Error; + + fn try_from(request_data: &TenantConfigRequest) -> Result { + let mut tenant_conf = TenantConfOpt::default(); + if let Some(gc_period) = &request_data.gc_period { + tenant_conf.gc_period = Some( + humantime::parse_duration(gc_period) + .with_context(bad_duration("gc_period", gc_period))?, + ); + } + tenant_conf.gc_horizon = request_data.gc_horizon; + tenant_conf.image_creation_threshold = request_data.image_creation_threshold; + + if let Some(pitr_interval) = &request_data.pitr_interval { + tenant_conf.pitr_interval = Some( + humantime::parse_duration(pitr_interval) + .with_context(bad_duration("pitr_interval", pitr_interval))?, + ); + } + if let Some(walreceiver_connect_timeout) = &request_data.walreceiver_connect_timeout { + tenant_conf.walreceiver_connect_timeout = Some( + humantime::parse_duration(walreceiver_connect_timeout).with_context( + bad_duration("walreceiver_connect_timeout", walreceiver_connect_timeout), + )?, + ); + } + if let Some(lagging_wal_timeout) = &request_data.lagging_wal_timeout { + tenant_conf.lagging_wal_timeout = Some( + humantime::parse_duration(lagging_wal_timeout) + .with_context(bad_duration("lagging_wal_timeout", lagging_wal_timeout))?, + ); + } + tenant_conf.max_lsn_wal_lag = request_data.max_lsn_wal_lag; + tenant_conf.trace_read_requests = request_data.trace_read_requests; + + tenant_conf.checkpoint_distance = request_data.checkpoint_distance; + if let Some(checkpoint_timeout) = &request_data.checkpoint_timeout { + tenant_conf.checkpoint_timeout = Some( + humantime::parse_duration(checkpoint_timeout) + .with_context(bad_duration("checkpoint_timeout", checkpoint_timeout))?, + ); + } + tenant_conf.compaction_target_size = request_data.compaction_target_size; + tenant_conf.compaction_threshold = request_data.compaction_threshold; + + if let Some(compaction_period) = &request_data.compaction_period { + tenant_conf.compaction_period = Some( + humantime::parse_duration(compaction_period) + .with_context(bad_duration("compaction_period", compaction_period))?, + ); + } + + if let Some(eviction_policy) = &request_data.eviction_policy { + tenant_conf.eviction_policy = Some( + serde::Deserialize::deserialize(eviction_policy) + .context("parse field `eviction_policy`")?, + ); + } + + tenant_conf.min_resident_size_override = request_data.min_resident_size_override; + + if let Some(evictions_low_residence_duration_metric_threshold) = + &request_data.evictions_low_residence_duration_metric_threshold + { + tenant_conf.evictions_low_residence_duration_metric_threshold = Some( + humantime::parse_duration(evictions_low_residence_duration_metric_threshold) + .with_context(bad_duration( + "evictions_low_residence_duration_metric_threshold", + evictions_low_residence_duration_metric_threshold, + ))?, + ); + } + + Ok(tenant_conf) + } +} + #[cfg(test)] mod tests { use super::*;