From 0a2e90ab65ff6f2ecf18a64de09e4174e876261c Mon Sep 17 00:00:00 2001 From: John Spray Date: Sun, 7 Jan 2024 20:57:42 +0000 Subject: [PATCH] pageserver: move EvictionPolicy into models/ This makes the config type more convenient to work with: it was previously carrying a serde_json::Value to force the contained types to be opaque, which doesn't make a lot of sense for an external interface. --- Cargo.lock | 1 + libs/pageserver_api/Cargo.toml | 1 + libs/pageserver_api/src/models.rs | 32 +++++++++++++++---- pageserver/src/tenant/config.rs | 26 +-------------- pageserver/src/tenant/timeline.rs | 6 ++-- .../src/tenant/timeline/eviction_task.rs | 6 ++-- 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4dd195a895..81540a1915 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3252,6 +3252,7 @@ dependencies = [ "const_format", "enum-map", "hex", + "humantime-serde", "postgres_ffi", "rand 0.8.5", "serde", diff --git a/libs/pageserver_api/Cargo.toml b/libs/pageserver_api/Cargo.toml index 4146597d8d..96c6c10d3e 100644 --- a/libs/pageserver_api/Cargo.toml +++ b/libs/pageserver_api/Cargo.toml @@ -19,6 +19,7 @@ strum.workspace = true strum_macros.workspace = true hex.workspace = true thiserror.workspace = true +humantime-serde.workspace = true workspace_hack.workspace = true diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index dea925b468..83412d4aae 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -4,7 +4,7 @@ use std::{ collections::HashMap, io::Read, num::{NonZeroU64, NonZeroUsize}, - time::SystemTime, + time::{Duration, SystemTime}, }; use byteorder::{BigEndian, ReadBytesExt}; @@ -232,17 +232,37 @@ pub struct TenantConfig { pub lagging_wal_timeout: Option, pub max_lsn_wal_lag: Option, pub trace_read_requests: Option, - // We defer the parsing of the eviction_policy field to the request handler. - // Otherwise we'd have to move the types for eviction policy into this package. - // We might do that once the eviction feature has stabilizied. - // For now, this field is not even documented in the openapi_spec.yml. - pub eviction_policy: Option, + pub eviction_policy: Option, pub min_resident_size_override: Option, pub evictions_low_residence_duration_metric_threshold: Option, pub gc_feedback: Option, pub heatmap_period: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind")] +pub enum EvictionPolicy { + NoEviction, + LayerAccessThreshold(EvictionPolicyLayerAccessThreshold), +} + +impl EvictionPolicy { + pub fn discriminant_str(&self) -> &'static str { + match self { + EvictionPolicy::NoEviction => "NoEviction", + EvictionPolicy::LayerAccessThreshold(_) => "LayerAccessThreshold", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct EvictionPolicyLayerAccessThreshold { + #[serde(with = "humantime_serde")] + pub period: Duration, + #[serde(with = "humantime_serde")] + pub threshold: Duration, +} + /// A flattened analog of a `pagesever::tenant::LocationMode`, which /// lists out all possible states (and the virtual "Detached" state) /// in a flat form rather than using rust-style enums. diff --git a/pageserver/src/tenant/config.rs b/pageserver/src/tenant/config.rs index 2d4cd350d7..e856047607 100644 --- a/pageserver/src/tenant/config.rs +++ b/pageserver/src/tenant/config.rs @@ -9,7 +9,7 @@ //! may lead to a data loss. //! use anyhow::bail; -use pageserver_api::models; +use pageserver_api::models::{self, EvictionPolicy}; use pageserver_api::shard::{ShardCount, ShardIdentity, ShardNumber, ShardStripeSize}; use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; @@ -428,30 +428,6 @@ pub struct TenantConfOpt { pub heatmap_period: Option, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "kind")] -pub enum EvictionPolicy { - NoEviction, - LayerAccessThreshold(EvictionPolicyLayerAccessThreshold), -} - -impl EvictionPolicy { - pub fn discriminant_str(&self) -> &'static str { - match self { - EvictionPolicy::NoEviction => "NoEviction", - EvictionPolicy::LayerAccessThreshold(_) => "LayerAccessThreshold", - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct EvictionPolicyLayerAccessThreshold { - #[serde(with = "humantime_serde")] - pub period: Duration, - #[serde(with = "humantime_serde")] - pub threshold: Duration, -} - impl TenantConfOpt { pub fn merge(&self, global_conf: TenantConf) -> TenantConf { TenantConf { diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 24a92859b7..4cf5c687f7 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -15,8 +15,8 @@ use fail::fail_point; use itertools::Itertools; use pageserver_api::{ models::{ - DownloadRemoteLayersTaskInfo, DownloadRemoteLayersTaskSpawnRequest, LayerMapInfo, - TimelineState, + DownloadRemoteLayersTaskInfo, DownloadRemoteLayersTaskSpawnRequest, EvictionPolicy, + LayerMapInfo, TimelineState, }, shard::{ShardIdentity, TenantShardId}, }; @@ -68,7 +68,7 @@ use crate::metrics::{ use crate::pgdatadir_mapping::LsnForTimestamp; use crate::pgdatadir_mapping::{is_inherited_key, is_rel_fsm_block_key, is_rel_vm_block_key}; use crate::pgdatadir_mapping::{BlockNumber, CalculateLogicalSizeError}; -use crate::tenant::config::{EvictionPolicy, TenantConfOpt}; +use crate::tenant::config::TenantConfOpt; use pageserver_api::reltag::RelTag; use pageserver_api::shard::ShardIndex; diff --git a/pageserver/src/tenant/timeline/eviction_task.rs b/pageserver/src/tenant/timeline/eviction_task.rs index ea5f5f5fa7..6a5ed6ffe1 100644 --- a/pageserver/src/tenant/timeline/eviction_task.rs +++ b/pageserver/src/tenant/timeline/eviction_task.rs @@ -20,6 +20,7 @@ use std::{ time::{Duration, SystemTime}, }; +use pageserver_api::models::{EvictionPolicy, EvictionPolicyLayerAccessThreshold}; use tokio::time::Instant; use tokio_util::sync::CancellationToken; use tracing::{debug, error, info, info_span, instrument, warn, Instrument}; @@ -29,10 +30,7 @@ use crate::{ pgdatadir_mapping::CollectKeySpaceError, task_mgr::{self, TaskKind, BACKGROUND_RUNTIME}, tenant::{ - config::{EvictionPolicy, EvictionPolicyLayerAccessThreshold}, - tasks::BackgroundLoopKind, - timeline::EvictionError, - LogicalSizeCalculationCause, Tenant, + tasks::BackgroundLoopKind, timeline::EvictionError, LogicalSizeCalculationCause, Tenant, }, };