diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 55e9c48421..6b0403c8ab 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -9,7 +9,6 @@ use std::{ collections::HashMap, io::{BufRead, Read}, num::{NonZeroU64, NonZeroUsize}, - str::FromStr, sync::atomic::AtomicUsize, time::{Duration, SystemTime}, }; @@ -334,14 +333,28 @@ pub struct TenantConfig { /// Unset -> V1 /// -> V2 /// -> CrossValidation -> V2 -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive( + Eq, + PartialEq, + Debug, + Copy, + Clone, + strum_macros::EnumString, + strum_macros::Display, + serde_with::DeserializeFromStr, + serde_with::SerializeDisplay, +)] +#[strum(serialize_all = "kebab-case")] pub enum AuxFilePolicy { /// V1 aux file policy: store everything in AUX_FILE_KEY + #[strum(ascii_case_insensitive)] V1, /// V2 aux file policy: store in the AUX_FILE keyspace + #[strum(ascii_case_insensitive)] V2, /// Cross validation runs both formats on the write path and does validation /// on the read path. + #[strum(ascii_case_insensitive)] CrossValidation, } @@ -407,23 +420,6 @@ impl AuxFilePolicy { } } -impl FromStr for AuxFilePolicy { - type Err = anyhow::Error; - - fn from_str(s: &str) -> Result { - let s = s.to_lowercase(); - if s == "v1" { - Ok(Self::V1) - } else if s == "v2" { - Ok(Self::V2) - } else if s == "crossvalidation" || s == "cross_validation" { - Ok(Self::CrossValidation) - } else { - anyhow::bail!("cannot parse {} to aux file policy", s) - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind")] pub enum EvictionPolicy { @@ -1405,6 +1401,7 @@ impl PagestreamBeMessage { #[cfg(test)] mod tests { use serde_json::json; + use std::str::FromStr; use super::*; @@ -1667,4 +1664,14 @@ mod tests { AuxFilePolicy::V2 )); } + + #[test] + fn test_aux_parse() { + assert_eq!(AuxFilePolicy::from_str("V2").unwrap(), AuxFilePolicy::V2); + assert_eq!(AuxFilePolicy::from_str("v2").unwrap(), AuxFilePolicy::V2); + assert_eq!( + AuxFilePolicy::from_str("cross-validation").unwrap(), + AuxFilePolicy::CrossValidation + ); + } } diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index b02054a702..796ae7217b 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -1625,7 +1625,7 @@ class NeonCli(AbstractNeonCli): args.extend(["-c", "switch_aux_file_policy:v1"]) if aux_file_v2 is AuxFileStore.CrossValidation: - args.extend(["-c", "switch_aux_file_policy:cross_validation"]) + args.extend(["-c", "switch_aux_file_policy:cross-validation"]) if set_default: args.append("--set-default") diff --git a/test_runner/fixtures/utils.py b/test_runner/fixtures/utils.py index 70263245e7..22bb43c580 100644 --- a/test_runner/fixtures/utils.py +++ b/test_runner/fixtures/utils.py @@ -495,9 +495,9 @@ def assert_no_errors(log_file, service, allowed_errors): @enum.unique class AuxFileStore(str, enum.Enum): - V1 = "V1" - V2 = "V2" - CrossValidation = "CrossValidation" + V1 = "v1" + V2 = "v2" + CrossValidation = "cross-validation" def __repr__(self) -> str: return f"'aux-{self.value}'" diff --git a/test_runner/regress/test_attach_tenant_config.py b/test_runner/regress/test_attach_tenant_config.py index 2ec375271c..531679c7af 100644 --- a/test_runner/regress/test_attach_tenant_config.py +++ b/test_runner/regress/test_attach_tenant_config.py @@ -190,7 +190,7 @@ def test_fully_custom_config(positive_env: NeonEnv): "trace_read_requests": True, "walreceiver_connect_timeout": "13m", "image_layer_creation_check_threshold": 1, - "switch_aux_file_policy": "CrossValidation", + "switch_aux_file_policy": "cross-validation", } ps_http = env.pageserver.http_client() diff --git a/test_runner/regress/test_aux_files.py b/test_runner/regress/test_aux_files.py index be9c41a867..5328aef156 100644 --- a/test_runner/regress/test_aux_files.py +++ b/test_runner/regress/test_aux_files.py @@ -1,5 +1,6 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import ( + AuxFileStore, NeonEnvBuilder, logical_replication_sync, ) @@ -14,7 +15,7 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg): timeline_id = env.initial_timeline tenant_config = client.tenant_config(tenant_id).effective_config - tenant_config["switch_aux_file_policy"] = "V2" + tenant_config["switch_aux_file_policy"] = AuxFileStore.V2 client.set_tenant_config(tenant_id, tenant_config) # aux file v2 is enabled on the write path, so for now, it should be unset (or null) assert ( @@ -49,7 +50,10 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg): with env.pageserver.http_client() as client: # aux file v2 flag should be enabled at this point - assert client.timeline_detail(tenant_id, timeline_id)["last_aux_file_policy"] == "V2" + assert ( + client.timeline_detail(tenant_id, timeline_id)["last_aux_file_policy"] + == AuxFileStore.V2 + ) with env.pageserver.http_client() as client: tenant_config = client.tenant_config(tenant_id).effective_config tenant_config["switch_aux_file_policy"] = "V1" @@ -59,7 +63,7 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg): client.timeline_detail(tenant_id=tenant_id, timeline_id=timeline_id)[ "last_aux_file_policy" ] - == "V2" + == AuxFileStore.V2 ) env.pageserver.restart() with env.pageserver.http_client() as client: @@ -68,5 +72,5 @@ def test_aux_v2_config_switch(neon_env_builder: NeonEnvBuilder, vanilla_pg): client.timeline_detail(tenant_id=tenant_id, timeline_id=timeline_id)[ "last_aux_file_policy" ] - == "V2" + == AuxFileStore.V2 )