pageserver: forward & backward compat load/store of config

pageserver/src/config.rs

@@ -36,8 +36,8 @@ use crate::tenant::{
     TIMELINES_SEGMENT_NAME,
 };
 use crate::{
-    IGNORED_TENANT_FILE_NAME, METADATA_FILE_NAME, TENANT_CONFIG_NAME, TIMELINE_DELETE_MARK_SUFFIX,
-    TIMELINE_UNINIT_MARK_SUFFIX,
+    IGNORED_TENANT_FILE_NAME, METADATA_FILE_NAME, TENANT_CONFIG_NAME, TENANT_LOCATION_CONFIG_NAME,
+    TIMELINE_DELETE_MARK_SUFFIX, TIMELINE_UNINIT_MARK_SUFFIX,
 };
 
 pub mod defaults {
@@ -595,10 +595,18 @@ impl PageServerConf {
 
     /// Points to a place in pageserver's local directory,
     /// where certain tenant's tenantconf file should be located.
+    ///
+    /// Legacy: superseded by tenant_location_config_path.  Eventually
+    /// remove this function.
     pub fn tenant_config_path(&self, tenant_id: &TenantId) -> PathBuf {
         self.tenant_path(tenant_id).join(TENANT_CONFIG_NAME)
     }
 
+    pub fn tenant_location_config_path(&self, tenant_id: &TenantId) -> PathBuf {
+        self.tenant_path(tenant_id)
+            .join(TENANT_LOCATION_CONFIG_NAME)
+    }
+
     pub fn timelines_path(&self, tenant_id: &TenantId) -> PathBuf {
         self.tenant_path(tenant_id).join(TIMELINES_SEGMENT_NAME)
     }

pageserver/src/lib.rs

@@ -106,6 +106,10 @@ pub const METADATA_FILE_NAME: &str = "metadata";
 /// Full path: `tenants/<tenant_id>/config`.
 pub const TENANT_CONFIG_NAME: &str = "config";
 
+/// Per-tenant configuration file.
+/// Full path: `tenants/<tenant_id>/config`.
+pub const TENANT_LOCATION_CONFIG_NAME: &str = "config-v1";
+
 /// A suffix used for various temporary files. Any temporary files found in the
 /// data directory at pageserver startup can be automatically removed.
 pub const TEMP_FILE_SUFFIX: &str = "___temp";

pageserver/src/tenant.rs

@@ -530,14 +530,14 @@ impl Tenant {
         ctx: &RequestContext,
     ) -> anyhow::Result<Arc<Tenant>> {
         // TODO dedup with spawn_load
-        let tenant_conf =
+        let location_conf =
             Self::load_tenant_config(conf, &tenant_id).context("load tenant config")?;
 
         let wal_redo_manager = Arc::new(PostgresRedoManager::new(conf, tenant_id));
         let tenant = Arc::new(Tenant::new(
             TenantState::Attaching,
             conf,
-            tenant_conf,
+            location_conf.tenant_conf,
             wal_redo_manager,
             tenant_id,
             generation,
@@ -891,7 +891,7 @@ impl Tenant {
         let tenant = Tenant::new(
             TenantState::Loading,
             conf,
-            tenant_conf,
+            tenant_conf.tenant_conf,
             wal_redo_manager,
             tenant_id,
             generation,
@@ -2330,51 +2330,108 @@ impl Tenant {
     pub(super) fn load_tenant_config(
         conf: &'static PageServerConf,
         tenant_id: &TenantId,
-    ) -> anyhow::Result<TenantConfOpt> {
-        let target_config_path = conf.tenant_config_path(tenant_id);
-        let target_config_display = target_config_path.display();
-
-        info!("loading tenantconf from {target_config_display}");
-
-        // FIXME If the config file is not found, assume that we're attaching
-        // a detached tenant and config is passed via attach command.
-        // https://github.com/neondatabase/neon/issues/1555
-        // OR: we're loading after incomplete deletion that managed to remove config.
-        if !target_config_path.exists() {
-            info!("tenant config not found in {target_config_display}");
-            return Ok(TenantConfOpt::default());
+    ) -> anyhow::Result<LocationConf> {
+        let legacy_config_path = conf.tenant_config_path(tenant_id);
+        let config_path = conf.tenant_location_config_path(tenant_id);
+
+        if std::path::Path::exists(&config_path) {
+            // New-style config takes precedence
+            let deserialized = Self::read_config(&legacy_config_path)?;
+            Ok(toml_edit::de::from_document::<LocationConf>(deserialized)?)
+        } else if std::path::Path::exists(&legacy_config_path) {
+            // Upgrade path: found an old-style configuration only
+            let deserialized = Self::read_config(&legacy_config_path)?;
+            let display_path = legacy_config_path.display();
+
+            let mut tenant_conf = TenantConfOpt::default();
+            for (key, item) in deserialized.iter() {
+                match key {
+                    "tenant_config" => {
+                        tenant_conf = PageServerConf::parse_toml_tenant_conf(item).with_context(|| {
+                            format!("Failed to parse config from file '{display_path}' as pageserver config")
+                        })?;
+                    }
+                    _ => bail!(
+                        "config file {display_path} has unrecognized pageserver option '{key}'"
+                    ),
+                }
+            }
+
+            Ok(LocationConf::new(tenant_conf, Generation::none()))
+        } else {
+            // FIXME If the config file is not found, assume that we're attaching
+            // a detached tenant and config is passed via attach command.
+            // https://github.com/neondatabase/neon/issues/1555
+            // OR: we're loading after incomplete deletion that managed to remove config.
+            info!(
+                "tenant config not found in {} or {}",
+                config_path.display(),
+                legacy_config_path.display()
+            );
+            Ok(LocationConf::default())
         }
+    }
+
+    fn read_config(path: &Path) -> anyhow::Result<toml_edit::Document> {
+        let path_display = path.display();
+        info!("loading tenant configuration from {path_display}");
 
         // load and parse file
-        let config = fs::read_to_string(&target_config_path).with_context(|| {
-            format!("Failed to load config from path '{target_config_display}'")
-        })?;
+        let config = fs::read_to_string(path)
+            .with_context(|| format!("Failed to load config from path '{path_display}'"))?;
 
-        let toml = config.parse::<toml_edit::Document>().with_context(|| {
-            format!("Failed to parse config from file '{target_config_display}' as toml file")
-        })?;
+        config.parse::<toml_edit::Document>().with_context(|| {
+            format!("Failed to parse config from file '{path_display}' as toml file")
+        })
+    }
 
-        let mut tenant_conf = TenantConfOpt::default();
-        for (key, item) in toml.iter() {
-            match key {
-                "tenant_config" => {
-                    tenant_conf = PageServerConf::parse_toml_tenant_conf(item).with_context(|| {
-                        format!("Failed to parse config from file '{target_config_display}' as pageserver config")
-                    })?;
-                }
-                _ => bail!("config file {target_config_display} has unrecognized pageserver option '{key}'"),
+    #[tracing::instrument(skip_all, fields(%tenant_id))]
+    pub(super) async fn persist_tenant_config(
+        tenant_id: &TenantId,
+        config_path: &Path,
+        legacy_config_path: &Path,
+        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_id,
+            legacy_config_path,
+            location_conf.tenant_conf,
+        )
+        .await?;
 
-            }
-        }
+        // imitate a try-block with a closure
+        info!("persisting tenantconf to {}", config_path.display());
+
+        let mut conf_content = r#"# This file contains a specific per-tenant's config.
+#  It is read in case of pageserver restart.
+
+[tenant_config]
+"#
+        .to_string();
+
+        // Convert the config to a toml file.
+        conf_content += &toml_edit::ser::to_string(&location_conf)?;
+
+        let conf_content = conf_content.as_bytes();
 
-        Ok(tenant_conf)
+        let temp_path = path_with_suffix_extension(config_path, TEMP_FILE_SUFFIX);
+        VirtualFile::crashsafe_overwrite(config_path, &temp_path, conf_content)
+            .await
+            .with_context(|| {
+                format!(
+                    "write tenant {tenant_id} config to {}",
+                    config_path.display()
+                )
+            })?;
+        Ok(())
     }
 
     #[tracing::instrument(skip_all, fields(%tenant_id))]
-    pub(super) async fn persist_tenant_config(
+    pub(super) async fn persist_tenant_config_legacy(
         tenant_id: &TenantId,
         target_config_path: &Path,
-        location_conf: LocationConf,
+        tenant_conf: TenantConfOpt,
     ) -> anyhow::Result<()> {
         // imitate a try-block with a closure
         info!("persisting tenantconf to {}", target_config_path.display());
@@ -2387,7 +2444,7 @@ impl Tenant {
         .to_string();
 
         // Convert the config to a toml file.
-        conf_content += &toml_edit::ser::to_string(&location_conf)?;
+        conf_content += &toml_edit::ser::to_string(&tenant_conf)?;
 
         let conf_content = conf_content.as_bytes();
 
@@ -3170,14 +3227,26 @@ async fn try_create_target_tenant_dir(
         temporary_tenant_dir,
     )
     .with_context(|| format!("resolve tenant {tenant_id} temporary timelines dir"))?;
-    let temporary_tenant_config_path = rebase_directory(
+    let temporary_legacy_tenant_config_path = rebase_directory(
         &conf.tenant_config_path(tenant_id),
         target_tenant_directory,
         temporary_tenant_dir,
     )
     .with_context(|| format!("resolve tenant {tenant_id} temporary config path"))?;
+    let temporary_tenant_config_path = rebase_directory(
+        &conf.tenant_location_config_path(tenant_id),
+        target_tenant_directory,
+        temporary_tenant_dir,
+    )
+    .with_context(|| format!("resolve tenant {tenant_id} temporary config path"))?;
 
-    Tenant::persist_tenant_config(tenant_id, &temporary_tenant_config_path, location_conf).await?;
+    Tenant::persist_tenant_config(
+        tenant_id,
+        &temporary_tenant_config_path,
+        &temporary_legacy_tenant_config_path,
+        location_conf,
+    )
+    .await?;
 
     crashsafe::create_dir(&temporary_tenant_timelines_dir).with_context(|| {
         format!(

pageserver/src/tenant/config.rs

@@ -84,9 +84,9 @@ pub(crate) enum LocationMode {
 pub(crate) struct LocationConf {
     /// The location-specific part of the configuration, describes the operating
     /// mode of this pageserver for this tenant.
-    mode: LocationMode,
+    pub(crate) mode: LocationMode,
     /// The pan-cluster tenant configuration, the same on all locations
-    tenant_conf: TenantConfOpt,
+    pub(crate) tenant_conf: TenantConfOpt,
 }
 
 impl LocationConf {
@@ -101,6 +101,20 @@ impl LocationConf {
     }
 }
 
+impl Default for LocationConf {
+    // TODO: this should be removed once tenant loading can guarantee that we are never
+    // loading from a directory without a configuration.
+    fn default() -> Self {
+        Self {
+            mode: LocationMode::Attached(AttachedLocationConfig {
+                generation: Generation::none(),
+                mode: AttachmentMode::Single,
+            }),
+            tenant_conf: TenantConfOpt::default(),
+        }
+    }
+}
+
 /// A tenant's calcuated configuration, which is the result of merging a
 /// tenant's TenantConfOpt with the global TenantConf from PageServerConf.
 ///

pageserver/src/tenant/mgr.rs

@@ -489,8 +489,9 @@ pub async fn set_new_tenant_config(
     let tenant = get_tenant(tenant_id, true).await?;
 
     let location_conf = LocationConf::new(new_tenant_conf, tenant.generation);
-    let tenant_config_path = conf.tenant_config_path(&tenant_id);
-    Tenant::persist_tenant_config(&tenant_id, &tenant_config_path, location_conf)
+    let legacy_config_path = conf.tenant_config_path(&tenant_id);
+    let config_path = conf.tenant_location_config_path(&tenant_id);
+    Tenant::persist_tenant_config(&tenant_id, &config_path, &legacy_config_path, location_conf)
         .await
         .map_err(SetNewTenantConfigError::Persist)?;
     tenant.set_new_tenant_config(new_tenant_conf);