From 256058f2abb044e4deacd71c8743cd14203fdd43 Mon Sep 17 00:00:00 2001 From: John Spray Date: Mon, 26 Feb 2024 10:24:58 +0000 Subject: [PATCH] pageserver: only write out legacy tenant config if no generation (#6891) ## Problem Previously we always wrote out both legacy and modern tenant config files. The legacy write enabled rollbacks, but we are long past the point where that is needed. We still need the legacy format for situations where someone is running tenants without generations (that will be yanked as well eventually), but we can avoid writing it out at all if we do have a generation number set. We implicitly also avoid writing the legacy config if our mode is Secondary (secondary mode is newer than generations). ## Summary of changes - Make writing legacy tenant config conditional on there being no generation number set. --- pageserver/src/tenant.rs | 27 +++++++++++++++---------- test_runner/fixtures/neon_fixtures.py | 2 +- test_runner/regress/test_tenant_conf.py | 3 +-- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index c97f24c0fc..2362f19068 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -2573,19 +2573,24 @@ impl Tenant { legacy_config_path: &Utf8Path, location_conf: &LocationConf, ) -> anyhow::Result<()> { - // Forward compat: write out an old-style configuration that old versions can read, in case we roll back - Self::persist_tenant_config_legacy( - tenant_shard_id, - legacy_config_path, - &location_conf.tenant_conf, - ) - .await?; - if let LocationMode::Attached(attach_conf) = &location_conf.mode { - // Once we use LocationMode, generations are mandatory. If we aren't using generations, - // then drop out after writing legacy-style config. + // The modern-style LocationConf config file requires a generation to be set. In case someone + // is running a pageserver without the infrastructure to set generations, write out the legacy-style + // config file that only contains TenantConf. + // + // This will eventually be removed in https://github.com/neondatabase/neon/issues/5388 + if attach_conf.generation.is_none() { - tracing::debug!("Running without generations, not writing new-style LocationConf"); + tracing::info!( + "Running without generations, writing legacy-style tenant config file" + ); + Self::persist_tenant_config_legacy( + tenant_shard_id, + legacy_config_path, + &location_conf.tenant_conf, + ) + .await?; + return Ok(()); } } diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 441b64ebfc..6cb7656660 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -3812,7 +3812,7 @@ def pytest_addoption(parser: Parser): SMALL_DB_FILE_NAME_REGEX: re.Pattern = re.compile( # type: ignore[type-arg] - r"config|config-v1|heatmap-v1|metadata|.+\.(?:toml|pid|json|sql|conf)" + r"config-v1|heatmap-v1|metadata|.+\.(?:toml|pid|json|sql|conf)" ) diff --git a/test_runner/regress/test_tenant_conf.py b/test_runner/regress/test_tenant_conf.py index 2ed22cabc4..a2ffd200a6 100644 --- a/test_runner/regress/test_tenant_conf.py +++ b/test_runner/regress/test_tenant_conf.py @@ -299,8 +299,7 @@ def test_creating_tenant_conf_after_attach(neon_env_builder: NeonEnvBuilder): # tenant is created with defaults, as in without config file (tenant_id, timeline_id) = env.neon_cli.create_tenant() - config_path = env.pageserver.tenant_dir(tenant_id) / "config" - assert config_path.exists(), "config file is always initially created" + config_path = env.pageserver.tenant_dir(tenant_id) / "config-v1" http_client = env.pageserver.http_client()