diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index f8c011a0a579b..e2698da739c50 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -432,8 +432,9 @@ jobs: strategy: fail-fast: false matrix: - build_type: [ debug, release ] - pg_version: [ v14, v15, v16 ] + build_type: [ debug ] + pg_version: [ v15 ] + pageserver_compaction_algorithm_kind: [ "Legacy", "Tiered" ] steps: - name: Checkout uses: actions/checkout@v4 @@ -461,6 +462,9 @@ jobs: PAGESERVER_GET_VECTORED_IMPL: vectored PAGESERVER_GET_IMPL: vectored PAGESERVER_VALIDATE_VEC_GET: true + PAGESERVER_DEFAULT_TENANT_CONFIG_COMPACTION_ALGORITHM: 'kind="${{ matrix.pageserver_compaction_algorithm_kind }}"' + # catch the tests that override `tenant_config` as a whole without specifying the compaction algorithm `kind` + NEON_PAGESERVER_PANIC_ON_UNSPECIFIED_COMPACTION_ALGORITHM: true # Temporary disable this step until we figure out why it's so flaky # Ref https://github.com/neondatabase/neon/issues/4540 diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 9311dab33cd05..fb418380edb46 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -451,6 +451,8 @@ impl EvictionPolicy { )] #[strum(serialize_all = "kebab-case")] pub enum CompactionAlgorithm { + #[strum(disabled)] + NotSpecified, Legacy, Tiered, } diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index b0afb6414be21..177ee5cd41093 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -5,7 +5,7 @@ //! See also `settings.md` for better description on every parameter. use anyhow::{anyhow, bail, ensure, Context, Result}; -use pageserver_api::shard::TenantShardId; +use pageserver_api::{models::CompactionAlgorithm, shard::TenantShardId}; use remote_storage::{RemotePath, RemoteStorageConfig}; use serde; use serde::de::IntoDeserializer; @@ -15,7 +15,7 @@ use utils::crashsafe::path_with_suffix_extension; use utils::id::ConnectionId; use utils::logging::SecretString; -use once_cell::sync::OnceCell; +use once_cell::sync::{Lazy, OnceCell}; use reqwest::Url; use std::num::NonZeroUsize; use std::str::FromStr; @@ -1067,6 +1067,19 @@ impl PageServerConf { conf.default_tenant_conf = t_conf.merge(TenantConf::default()); + { + const VAR_NAME: &str = "NEON_PAGESERVER_PANIC_ON_UNSPECIFIED_COMPACTION_ALGORITHM"; + static VAR: Lazy> = Lazy::new(|| utils::env::var(VAR_NAME)); + if VAR.unwrap_or(false) + && conf.default_tenant_conf.compaction_algorithm.kind + == CompactionAlgorithm::NotSpecified + { + panic!( + "Unspecified compaction algorithm in default tenant configuration. \ + Set the algorithm explicitly in the pageserver.toml's `tenant_config` field or unset the environment variable {VAR_NAME}"); + } + } + Ok(conf) } diff --git a/pageserver/src/tenant/config.rs b/pageserver/src/tenant/config.rs index 342d70595468a..33a445099db3f 100644 --- a/pageserver/src/tenant/config.rs +++ b/pageserver/src/tenant/config.rs @@ -40,8 +40,6 @@ pub mod defaults { pub const DEFAULT_COMPACTION_PERIOD: &str = "20 s"; pub const DEFAULT_COMPACTION_THRESHOLD: usize = 10; - pub const DEFAULT_COMPACTION_ALGORITHM: super::CompactionAlgorithm = - super::CompactionAlgorithm::Legacy; pub const DEFAULT_GC_HORIZON: u64 = 64 * 1024 * 1024; @@ -554,7 +552,13 @@ impl Default for TenantConf { .expect("cannot parse default compaction period"), compaction_threshold: DEFAULT_COMPACTION_THRESHOLD, compaction_algorithm: CompactionAlgorithmSettings { - kind: DEFAULT_COMPACTION_ALGORITHM, + kind: if cfg!(test) { + // Rust tests rely on a valid implicit default (TODO: fix this) + CompactionAlgorithm::Legacy + } else { + // Python tests are subject to NotSpecified handling + CompactionAlgorithm::NotSpecified + }, }, gc_horizon: DEFAULT_GC_HORIZON, gc_period: humantime::parse_duration(DEFAULT_GC_PERIOD) diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index d4f6e25843b7b..848924cdfade3 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -1700,6 +1700,9 @@ impl Timeline { } match self.get_compaction_algorithm_settings().kind { + CompactionAlgorithm::NotSpecified => { + unreachable!("should panic earlier when we construct the default tenant conf") + } CompactionAlgorithm::Tiered => self.compact_tiered(cancel, ctx).await, CompactionAlgorithm::Legacy => self.compact_legacy(cancel, flags, ctx).await, }