Remove CLI flag from pageserver binary as we're using config field instead

Co-Authored-By: John Spray <john@neon.tech>

Cargo.lock

@@ -2837,7 +2837,6 @@ dependencies = [
  "utils",
  "uuid",
  "workspace_hack",
- "x509-cert",
 ]
 
 [[package]]

compute_tools/src/bin/compute_ctl.rs

@@ -29,12 +29,13 @@
 //! ```sh
 //! compute_ctl -D /var/db/postgres/compute \
 //!             -C 'postgresql://cloud_admin@localhost/postgres' \
-//!             -c /var/db/postgres/configs/config.json \
+//!             -S /var/db/postgres/specs/current.json \
 //!             -b /usr/local/bin/postgres \
 //!             -r http://pg-ext-s3-gateway \
 //! ```
 use std::ffi::OsString;
 use std::fs::File;
+use std::path::Path;
 use std::process::exit;
 use std::sync::mpsc;
 use std::thread;
@@ -42,7 +43,8 @@ use std::time::Duration;
 
 use anyhow::{Context, Result};
 use clap::Parser;
-use compute_api::responses::ComputeConfig;
+use compute_api::responses::ComputeCtlConfig;
+use compute_api::spec::ComputeSpec;
 use compute_tools::compute::{
     BUILD_TAG, ComputeNode, ComputeNodeParams, forward_termination_signal,
 };
@@ -116,10 +118,8 @@ struct Cli {
     #[arg(long)]
     pub set_disk_quota_for_fs: Option<String>,
 
-    // TODO(tristan957): remove alias after compatibility tests are no longer
-    // an issue
-    #[arg(short = 'c', long, alias = "spec-path")]
-    pub config: Option<OsString>,
+    #[arg(short = 'S', long, group = "spec-path")]
+    pub spec_path: Option<OsString>,
 
     #[arg(short = 'i', long, group = "compute-id")]
     pub compute_id: String,
@@ -127,9 +127,8 @@ struct Cli {
     #[arg(
         short = 'p',
         long,
-        conflicts_with = "config",
-        value_name = "CONTROL_PLANE_API_BASE_URL",
-        requires = "compute-id"
+        conflicts_with = "spec-path",
+        value_name = "CONTROL_PLANE_API_BASE_URL"
     )]
     pub control_plane_uri: Option<String>,
 }
@@ -155,7 +154,7 @@ fn main() -> Result<()> {
 
     let connstr = Url::parse(&cli.connstr).context("cannot parse connstr as a URL")?;
 
-    let cli_spec = get_config(&cli)?;
+    let cli_spec = try_spec_from_cli(&cli)?;
 
     let compute_node = ComputeNode::new(
         ComputeNodeParams {
@@ -202,17 +201,27 @@ async fn init() -> Result<()> {
     Ok(())
 }
 
-fn get_config(cli: &Cli) -> Result<ComputeConfig> {
-    // First, read the config from the path if provided
-    if let Some(ref config) = cli.config {
-        let file = File::open(config)?;
-        return Ok(serde_json::from_reader(&file)?);
+fn try_spec_from_cli(cli: &Cli) -> Result<CliSpecParams> {
+    // First, read spec from the path if provided
+    if let Some(ref spec_path) = cli.spec_path {
+        let file = File::open(Path::new(spec_path))?;
+        return Ok(CliSpecParams {
+            spec: Some(serde_json::from_reader(file)?),
+            compute_ctl_config: ComputeCtlConfig::default(),
+        });
     }
 
-    // If the config wasn't provided in the CLI arguments, then retrieve it from
+    if cli.control_plane_uri.is_none() {
+        panic!("must specify --control-plane-uri");
+    };
+
+    // If the spec wasn't provided in the CLI arguments, then retrieve it from
     // the control plane
-    match get_config_from_control_plane(cli.control_plane_uri.as_ref().unwrap(), &cli.compute_id) {
-        Ok(config) => Ok(config),
+    match get_spec_from_control_plane(cli.control_plane_uri.as_ref().unwrap(), &cli.compute_id) {
+        Ok(resp) => Ok(CliSpecParams {
+            spec: resp.0,
+            compute_ctl_config: resp.1,
+        }),
         Err(e) => {
             error!(
                 "cannot get response from control plane: {}\n\
@@ -224,6 +233,13 @@ fn get_config(cli: &Cli) -> Result<ComputeConfig> {
     }
 }
 
+struct CliSpecParams {
+    /// If a spec was provided via CLI or file, the [`ComputeSpec`]
+    spec: Option<ComputeSpec>,
+    #[allow(dead_code)]
+    compute_ctl_config: ComputeCtlConfig,
+}
+
 fn deinit_and_exit(exit_code: Option<i32>) -> ! {
     // Shutdown trace pipeline gracefully, so that it has a chance to send any
     // pending traces before we exit. Shutting down OTEL tracing provider may

compute_tools/src/compute.rs

@@ -523,14 +523,11 @@ impl ComputeNode {
 
         let pspec = compute_state.pspec.as_ref().expect("spec must be set");
         info!(
-            "starting compute for project {}, operation {}, tenant {}, timeline {}, project {}, branch {}, endpoint {}, features {:?}, spec.remote_extensions {:?}",
+            "starting compute for project {}, operation {}, tenant {}, timeline {}, features {:?}, spec.remote_extensions {:?}",
             pspec.spec.cluster.cluster_id.as_deref().unwrap_or("None"),
             pspec.spec.operation_uuid.as_deref().unwrap_or("None"),
             pspec.tenant_id,
             pspec.timeline_id,
-            pspec.spec.project_id.as_deref().unwrap_or("None"),
-            pspec.spec.branch_id.as_deref().unwrap_or("None"),
-            pspec.spec.endpoint_id.as_deref().unwrap_or("None"),
             pspec.spec.features,
             pspec.spec.remote_extensions,
         );
@@ -634,23 +631,19 @@ impl ComputeNode {
             });
         }
 
-        // Configure and start rsyslog for compliance audit logging
-        match pspec.spec.audit_log_level {
-            ComputeAudit::Hipaa | ComputeAudit::Extended | ComputeAudit::Full => {
-                let remote_endpoint =
-                    std::env::var("AUDIT_LOGGING_ENDPOINT").unwrap_or("".to_string());
-                if remote_endpoint.is_empty() {
-                    anyhow::bail!("AUDIT_LOGGING_ENDPOINT is empty");
-                }
-
-                let log_directory_path = Path::new(&self.params.pgdata).join("log");
-                let log_directory_path = log_directory_path.to_string_lossy().to_string();
-                configure_audit_rsyslog(log_directory_path.clone(), "hipaa", &remote_endpoint)?;
-
-                // Launch a background task to clean up the audit logs
-                launch_pgaudit_gc(log_directory_path);
+        // Configure and start rsyslog for HIPAA if necessary
+        if let ComputeAudit::Hipaa = pspec.spec.audit_log_level {
+            let remote_endpoint = std::env::var("AUDIT_LOGGING_ENDPOINT").unwrap_or("".to_string());
+            if remote_endpoint.is_empty() {
+                anyhow::bail!("AUDIT_LOGGING_ENDPOINT is empty");
             }
-            _ => {}
+
+            let log_directory_path = Path::new(&self.params.pgdata).join("log");
+            let log_directory_path = log_directory_path.to_string_lossy().to_string();
+            configure_audit_rsyslog(log_directory_path.clone(), "hipaa", &remote_endpoint)?;
+
+            // Launch a background task to clean up the audit logs
+            launch_pgaudit_gc(log_directory_path);
         }
 
         // Configure and start rsyslog for Postgres logs export

compute_tools/src/config.rs

@@ -89,15 +89,6 @@ pub fn write_postgres_conf(
             escape_conf_value(&s.to_string())
         )?;
     }
-    if let Some(s) = &spec.project_id {
-        writeln!(file, "neon.project_id={}", escape_conf_value(s))?;
-    }
-    if let Some(s) = &spec.branch_id {
-        writeln!(file, "neon.branch_id={}", escape_conf_value(s))?;
-    }
-    if let Some(s) = &spec.endpoint_id {
-        writeln!(file, "neon.endpoint_id={}", escape_conf_value(s))?;
-    }
 
     // tls
     if let Some(tls_config) = tls_config {
@@ -178,7 +169,7 @@ pub fn write_postgres_conf(
     // and don't allow the user or the control plane admin to change them.
     match spec.audit_log_level {
         ComputeAudit::Disabled => {}
-        ComputeAudit::Log | ComputeAudit::Base => {
+        ComputeAudit::Log => {
             writeln!(file, "# Managed by compute_ctl base audit settings: start")?;
             writeln!(file, "pgaudit.log='ddl,role'")?;
             // Disable logging of catalog queries to reduce the noise
@@ -202,20 +193,16 @@ pub fn write_postgres_conf(
             }
             writeln!(file, "# Managed by compute_ctl base audit settings: end")?;
         }
-        ComputeAudit::Hipaa | ComputeAudit::Extended | ComputeAudit::Full => {
+        ComputeAudit::Hipaa => {
             writeln!(
                 file,
                 "# Managed by compute_ctl compliance audit settings: begin"
             )?;
-            // Enable logging of parameters.
-            // This is very verbose and may contain sensitive data.
-            if spec.audit_log_level == ComputeAudit::Full {
-                writeln!(file, "pgaudit.log_parameter=on")?;
-                writeln!(file, "pgaudit.log='all'")?;
-            } else {
-                writeln!(file, "pgaudit.log_parameter=off")?;
-                writeln!(file, "pgaudit.log='all, -misc'")?;
-            }
+            // This log level is very verbose
+            // but this is necessary for HIPAA compliance.
+            // Exclude 'misc' category, because it doesn't contain anythig relevant.
+            writeln!(file, "pgaudit.log='all, -misc'")?;
+            writeln!(file, "pgaudit.log_parameter=on")?;
             // Disable logging of catalog queries
             // The catalog doesn't contain sensitive data, so we don't need to audit it.
             writeln!(file, "pgaudit.log_catalog=off")?;

compute_tools/src/metrics.rs

@@ -19,13 +19,13 @@ pub(crate) static INSTALLED_EXTENSIONS: Lazy<UIntGaugeVec> = Lazy::new(|| {
 // but for all our APIs we defined a 'slug'/method/operationId in the OpenAPI spec.
 // And it's fair to call it a 'RPC' (Remote Procedure Call).
 pub enum CPlaneRequestRPC {
-    GetConfig,
+    GetSpec,
 }
 
 impl CPlaneRequestRPC {
     pub fn as_str(&self) -> &str {
         match self {
-            CPlaneRequestRPC::GetConfig => "GetConfig",
+            CPlaneRequestRPC::GetSpec => "GetSpec",
         }
     }
 }

compute_tools/src/spec.rs

@@ -3,8 +3,9 @@ use std::path::Path;
 
 use anyhow::{Result, anyhow, bail};
 use compute_api::responses::{
-    ComputeConfig, ControlPlaneComputeStatus, ControlPlaneConfigResponse,
+    ComputeCtlConfig, ControlPlaneComputeStatus, ControlPlaneSpecResponse,
 };
+use compute_api::spec::ComputeSpec;
 use reqwest::StatusCode;
 use tokio_postgres::Client;
 use tracing::{error, info, instrument};
@@ -20,7 +21,7 @@ use crate::params::PG_HBA_ALL_MD5;
 fn do_control_plane_request(
     uri: &str,
     jwt: &str,
-) -> Result<ControlPlaneConfigResponse, (bool, String, String)> {
+) -> Result<ControlPlaneSpecResponse, (bool, String, String)> {
     let resp = reqwest::blocking::Client::new()
         .get(uri)
         .header("Authorization", format!("Bearer {}", jwt))
@@ -28,14 +29,14 @@ fn do_control_plane_request(
         .map_err(|e| {
             (
                 true,
-                format!("could not perform request to control plane: {:?}", e),
+                format!("could not perform spec request to control plane: {:?}", e),
                 UNKNOWN_HTTP_STATUS.to_string(),
             )
         })?;
 
     let status = resp.status();
     match status {
-        StatusCode::OK => match resp.json::<ControlPlaneConfigResponse>() {
+        StatusCode::OK => match resp.json::<ControlPlaneSpecResponse>() {
             Ok(spec_resp) => Ok(spec_resp),
             Err(e) => Err((
                 true,
@@ -68,35 +69,40 @@ fn do_control_plane_request(
     }
 }
 
-/// Request config from the control-plane by compute_id. If
-/// `NEON_CONTROL_PLANE_TOKEN` env variable is set, it will be used for
-/// authorization.
-pub fn get_config_from_control_plane(base_uri: &str, compute_id: &str) -> Result<ComputeConfig> {
+/// Request spec from the control-plane by compute_id. If `NEON_CONTROL_PLANE_TOKEN`
+/// env variable is set, it will be used for authorization.
+pub fn get_spec_from_control_plane(
+    base_uri: &str,
+    compute_id: &str,
+) -> Result<(Option<ComputeSpec>, ComputeCtlConfig)> {
     let cp_uri = format!("{base_uri}/compute/api/v2/computes/{compute_id}/spec");
-    let jwt: String = std::env::var("NEON_CONTROL_PLANE_TOKEN").unwrap_or_default();
+    let jwt: String = match std::env::var("NEON_CONTROL_PLANE_TOKEN") {
+        Ok(v) => v,
+        Err(_) => "".to_string(),
+    };
     let mut attempt = 1;
 
-    info!("getting config from control plane: {}", cp_uri);
+    info!("getting spec from control plane: {}", cp_uri);
 
     // Do 3 attempts to get spec from the control plane using the following logic:
     // - network error -> then retry
     // - compute id is unknown or any other error -> bail out
     // - no spec for compute yet (Empty state) -> return Ok(None)
-    // - got config -> return Ok(Some(config))
+    // - got spec -> return Ok(Some(spec))
     while attempt < 4 {
         let result = match do_control_plane_request(&cp_uri, &jwt) {
-            Ok(config_resp) => {
+            Ok(spec_resp) => {
                 CPLANE_REQUESTS_TOTAL
                     .with_label_values(&[
-                        CPlaneRequestRPC::GetConfig.as_str(),
+                        CPlaneRequestRPC::GetSpec.as_str(),
                         &StatusCode::OK.to_string(),
                     ])
                     .inc();
-                match config_resp.status {
-                    ControlPlaneComputeStatus::Empty => Ok(config_resp.into()),
+                match spec_resp.status {
+                    ControlPlaneComputeStatus::Empty => Ok((None, spec_resp.compute_ctl_config)),
                     ControlPlaneComputeStatus::Attached => {
-                        if config_resp.spec.is_some() {
-                            Ok(config_resp.into())
+                        if let Some(spec) = spec_resp.spec {
+                            Ok((Some(spec), spec_resp.compute_ctl_config))
                         } else {
                             bail!("compute is attached, but spec is empty")
                         }
@@ -105,7 +111,7 @@ pub fn get_config_from_control_plane(base_uri: &str, compute_id: &str) -> Result
             }
             Err((retry, msg, status)) => {
                 CPLANE_REQUESTS_TOTAL
-                    .with_label_values(&[CPlaneRequestRPC::GetConfig.as_str(), &status])
+                    .with_label_values(&[CPlaneRequestRPC::GetSpec.as_str(), &status])
                     .inc();
                 if retry {
                     Err(anyhow!(msg))
@@ -116,7 +122,7 @@ pub fn get_config_from_control_plane(base_uri: &str, compute_id: &str) -> Result
         };
 
         if let Err(e) = &result {
-            error!("attempt {} to get config failed with: {}", attempt, e);
+            error!("attempt {} to get spec failed with: {}", attempt, e);
         } else {
             return result;
         }
@@ -127,13 +133,13 @@ pub fn get_config_from_control_plane(base_uri: &str, compute_id: &str) -> Result
 
     // All attempts failed, return error.
     Err(anyhow::anyhow!(
-        "Exhausted all attempts to retrieve the config from the control plane"
+        "Exhausted all attempts to retrieve the spec from the control plane"
     ))
 }
 
 /// Check `pg_hba.conf` and update if needed to allow external connections.
 pub fn update_pg_hba(pgdata_path: &Path) -> Result<()> {
-    // XXX: consider making it a part of config.json
+    // XXX: consider making it a part of spec.json
     let pghba_path = pgdata_path.join("pg_hba.conf");
 
     if config::line_in_file(&pghba_path, PG_HBA_ALL_MD5)? {
@@ -147,7 +153,7 @@ pub fn update_pg_hba(pgdata_path: &Path) -> Result<()> {
 
 /// Create a standby.signal file
 pub fn add_standby_signal(pgdata_path: &Path) -> Result<()> {
-    // XXX: consider making it a part of config.json
+    // XXX: consider making it a part of spec.json
     let signalfile = pgdata_path.join("standby.signal");
 
     if !signalfile.exists() {

compute_tools/src/spec_apply.rs

@@ -278,12 +278,12 @@ impl ComputeNode {
             // so that all config operations are audit logged.
             match spec.audit_log_level
             {
-                ComputeAudit::Hipaa | ComputeAudit::Extended | ComputeAudit::Full => {
+                ComputeAudit::Hipaa => {
                     phases.push(CreatePgauditExtension);
                     phases.push(CreatePgauditlogtofileExtension);
                     phases.push(DisablePostgresDBPgAudit);
                 }
-                ComputeAudit::Log | ComputeAudit::Base => {
+                ComputeAudit::Log => {
                     phases.push(CreatePgauditExtension);
                     phases.push(DisablePostgresDBPgAudit);
                 }

control_plane/src/endpoint.rs

@@ -29,7 +29,7 @@
 //!     compute.log               - log output of `compute_ctl` and `postgres`
 //!     endpoint.json             - serialized `EndpointConf` struct
 //!     postgresql.conf           - postgresql settings
-//!     config.json                 - passed to `compute_ctl`
+//!     spec.json                 - passed to `compute_ctl`
 //!     pgdata/
 //!         postgresql.conf       - copy of postgresql.conf created by `compute_ctl`
 //!         zenith.signal
@@ -46,9 +46,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 
 use anyhow::{Context, Result, anyhow, bail};
 use compute_api::requests::ConfigurationRequest;
-use compute_api::responses::{
-    ComputeConfig, ComputeCtlConfig, ComputeStatus, ComputeStatusResponse,
-};
+use compute_api::responses::{ComputeCtlConfig, ComputeStatus, ComputeStatusResponse};
 use compute_api::spec::{
     Cluster, ComputeAudit, ComputeFeature, ComputeMode, ComputeSpec, Database, PgIdent,
     RemoteExtSpec, Role,
@@ -621,101 +619,87 @@ impl Endpoint {
             remote_extensions = None;
         };
 
-        // Create config file
-        let config = {
-            let mut spec = ComputeSpec {
-                skip_pg_catalog_updates: self.skip_pg_catalog_updates,
-                format_version: 1.0,
-                operation_uuid: None,
-                features: self.features.clone(),
-                swap_size_bytes: None,
-                disk_quota_bytes: None,
-                disable_lfc_resizing: None,
-                cluster: Cluster {
-                    cluster_id: None, // project ID: not used
-                    name: None,       // project name: not used
-                    state: None,
-                    roles: if create_test_user {
-                        vec![Role {
-                            name: PgIdent::from_str("test").unwrap(),
-                            encrypted_password: None,
-                            options: None,
-                        }]
-                    } else {
-                        Vec::new()
-                    },
-                    databases: if create_test_user {
-                        vec![Database {
-                            name: PgIdent::from_str("neondb").unwrap(),
-                            owner: PgIdent::from_str("test").unwrap(),
-                            options: None,
-                            restrict_conn: false,
-                            invalid: false,
-                        }]
-                    } else {
-                        Vec::new()
-                    },
-                    settings: None,
-                    postgresql_conf: Some(postgresql_conf.clone()),
-                },
-                delta_operations: None,
-                tenant_id: Some(self.tenant_id),
-                timeline_id: Some(self.timeline_id),
-                project_id: None,
-                branch_id: None,
-                endpoint_id: Some(self.endpoint_id.clone()),
-                mode: self.mode,
-                pageserver_connstring: Some(pageserver_connstring),
-                safekeepers_generation: safekeepers_generation.map(|g| g.into_inner()),
-                safekeeper_connstrings,
-                storage_auth_token: auth_token.clone(),
-                remote_extensions,
-                pgbouncer_settings: None,
-                shard_stripe_size: Some(shard_stripe_size),
-                local_proxy_config: None,
-                reconfigure_concurrency: self.reconfigure_concurrency,
-                drop_subscriptions_before_start: self.drop_subscriptions_before_start,
-                audit_log_level: ComputeAudit::Disabled,
-                logs_export_host: None::<String>,
-            };
-
-            // this strange code is needed to support respec() in tests
-            if self.cluster.is_some() {
-                debug!("Cluster is already set in the endpoint spec, using it");
-                spec.cluster = self.cluster.clone().unwrap();
-
-                debug!("spec.cluster {:?}", spec.cluster);
-
-                // fill missing fields again
-                if create_test_user {
-                    spec.cluster.roles.push(Role {
+        // Create spec file
+        let mut spec = ComputeSpec {
+            skip_pg_catalog_updates: self.skip_pg_catalog_updates,
+            format_version: 1.0,
+            operation_uuid: None,
+            features: self.features.clone(),
+            swap_size_bytes: None,
+            disk_quota_bytes: None,
+            disable_lfc_resizing: None,
+            cluster: Cluster {
+                cluster_id: None, // project ID: not used
+                name: None,       // project name: not used
+                state: None,
+                roles: if create_test_user {
+                    vec![Role {
                         name: PgIdent::from_str("test").unwrap(),
                         encrypted_password: None,
                         options: None,
-                    });
-                    spec.cluster.databases.push(Database {
+                    }]
+                } else {
+                    Vec::new()
+                },
+                databases: if create_test_user {
+                    vec![Database {
                         name: PgIdent::from_str("neondb").unwrap(),
                         owner: PgIdent::from_str("test").unwrap(),
                         options: None,
                         restrict_conn: false,
                         invalid: false,
-                    });
-                }
-                spec.cluster.postgresql_conf = Some(postgresql_conf);
-            }
-
-            ComputeConfig {
-                spec: Some(spec),
-                compute_ctl_config: ComputeCtlConfig::default(),
-            }
+                    }]
+                } else {
+                    Vec::new()
+                },
+                settings: None,
+                postgresql_conf: Some(postgresql_conf.clone()),
+            },
+            delta_operations: None,
+            tenant_id: Some(self.tenant_id),
+            timeline_id: Some(self.timeline_id),
+            mode: self.mode,
+            pageserver_connstring: Some(pageserver_connstring),
+            safekeepers_generation: safekeepers_generation.map(|g| g.into_inner()),
+            safekeeper_connstrings,
+            storage_auth_token: auth_token.clone(),
+            remote_extensions,
+            pgbouncer_settings: None,
+            shard_stripe_size: Some(shard_stripe_size),
+            local_proxy_config: None,
+            reconfigure_concurrency: self.reconfigure_concurrency,
+            drop_subscriptions_before_start: self.drop_subscriptions_before_start,
+            audit_log_level: ComputeAudit::Disabled,
+            logs_export_host: None::<String>,
         };
 
-        // TODO(tristan957): Remove the write to spec.json after compatibility
-        // tests work themselves out
+        // this strange code is needed to support respec() in tests
+        if self.cluster.is_some() {
+            debug!("Cluster is already set in the endpoint spec, using it");
+            spec.cluster = self.cluster.clone().unwrap();
+
+            debug!("spec.cluster {:?}", spec.cluster);
+
+            // fill missing fields again
+            if create_test_user {
+                spec.cluster.roles.push(Role {
+                    name: PgIdent::from_str("test").unwrap(),
+                    encrypted_password: None,
+                    options: None,
+                });
+                spec.cluster.databases.push(Database {
+                    name: PgIdent::from_str("neondb").unwrap(),
+                    owner: PgIdent::from_str("test").unwrap(),
+                    options: None,
+                    restrict_conn: false,
+                    invalid: false,
+                });
+            }
+            spec.cluster.postgresql_conf = Some(postgresql_conf);
+        }
+
         let spec_path = self.endpoint_path().join("spec.json");
-        std::fs::write(spec_path, serde_json::to_string_pretty(&config.spec)?)?;
-        let config_path = self.endpoint_path().join("config.json");
-        std::fs::write(config_path, serde_json::to_string_pretty(&config)?)?;
+        std::fs::write(spec_path, serde_json::to_string_pretty(&spec)?)?;
 
         // Open log file. We'll redirect the stdout and stderr of `compute_ctl` to it.
         let logfile = std::fs::OpenOptions::new()
@@ -723,16 +707,6 @@ impl Endpoint {
             .append(true)
             .open(self.endpoint_path().join("compute.log"))?;
 
-        // TODO(tristan957): Remove when compatibility tests are no longer an
-        // issue
-        let old_compute_ctl = {
-            let mut cmd = Command::new(self.env.neon_distrib_dir.join("compute_ctl"));
-            let help_output = cmd.arg("--help").output()?;
-            let help_output = String::from_utf8_lossy(&help_output.stdout);
-
-            !help_output.contains("--config")
-        };
-
         // Launch compute_ctl
         let conn_str = self.connstr("cloud_admin", "postgres");
         println!("Starting postgres node at '{}'", conn_str);
@@ -751,18 +725,9 @@ impl Endpoint {
         ])
         .args(["--pgdata", self.pgdata().to_str().unwrap()])
         .args(["--connstr", &conn_str])
-        // TODO(tristan957): Change this to --config when compatibility tests
-        // are no longer an issue
         .args([
             "--spec-path",
-            self.endpoint_path()
-                .join(if old_compute_ctl {
-                    "spec.json"
-                } else {
-                    "config.json"
-                })
-                .to_str()
-                .unwrap(),
+            self.endpoint_path().join("spec.json").to_str().unwrap(),
         ])
         .args([
             "--pgbin",
@@ -905,12 +870,10 @@ impl Endpoint {
         stripe_size: Option<ShardStripeSize>,
         safekeepers: Option<Vec<NodeId>>,
     ) -> Result<()> {
-        let (mut spec, compute_ctl_config) = {
-            let config_path = self.endpoint_path().join("config.json");
-            let file = std::fs::File::open(config_path)?;
-            let config: ComputeConfig = serde_json::from_reader(file)?;
-
-            (config.spec.unwrap(), config.compute_ctl_config)
+        let mut spec: ComputeSpec = {
+            let spec_path = self.endpoint_path().join("spec.json");
+            let file = std::fs::File::open(spec_path)?;
+            serde_json::from_reader(file)?
         };
 
         let postgresql_conf = self.read_postgresql_conf()?;
@@ -960,7 +923,7 @@ impl Endpoint {
             .body(
                 serde_json::to_string(&ConfigurationRequest {
                     spec,
-                    compute_ctl_config,
+                    compute_ctl_config: ComputeCtlConfig::default(),
                 })
                 .unwrap(),
             )

control_plane/src/pageserver.rs

@@ -318,7 +318,7 @@ impl PageServerNode {
                 self.conf.id, datadir,
             )
         })?;
-        let args = vec!["-D", datadir_path_str];
+        let args = vec!["-D", datadir_path_str, "--dev"];
 
         background_process::start_process(
             "pageserver",

control_plane/src/safekeeper.rs

@@ -162,6 +162,7 @@ impl SafekeeperNode {
             listen_http,
             "--availability-zone".to_owned(),
             availability_zone,
+            "--dev".to_owned(),
         ];
         if let Some(pg_tenant_only_port) = self.conf.pg_tenant_only_port {
             let listen_pg_tenant_only = format!("{}:{}", self.listen_addr, pg_tenant_only_port);

docker-compose/compute_wrapper/shell/compute.sh

@@ -11,8 +11,8 @@ generate_id() {
 
 PG_VERSION=${PG_VERSION:-14}
 
-CONFIG_FILE_ORG=/var/db/postgres/configs/config.json
-CONFIG_FILE=/tmp/config.json
+SPEC_FILE_ORG=/var/db/postgres/specs/spec.json
+SPEC_FILE=/tmp/spec.json
 
 echo "Waiting pageserver become ready."
 while ! nc -z pageserver 6400; do
@@ -20,7 +20,7 @@ while ! nc -z pageserver 6400; do
 done
 echo "Page server is ready."
 
-cp ${CONFIG_FILE_ORG} ${CONFIG_FILE}
+cp ${SPEC_FILE_ORG} ${SPEC_FILE}
 
  if [ -n "${TENANT_ID:-}" ] && [ -n "${TIMELINE_ID:-}" ]; then
    tenant_id=${TENANT_ID}
@@ -73,27 +73,17 @@ else
   ulid_extension=ulid
 fi
 echo "Adding pgx_ulid"
-shared_libraries=$(jq -r '.spec.cluster.settings[] | select(.name=="shared_preload_libraries").value' ${CONFIG_FILE})
-sed -i "s/${shared_libraries}/${shared_libraries},${ulid_extension}/" ${CONFIG_FILE}
+shared_libraries=$(jq -r '.cluster.settings[] | select(.name=="shared_preload_libraries").value' ${SPEC_FILE})
+sed -i "s/${shared_libraries}/${shared_libraries},${ulid_extension}/" ${SPEC_FILE}
 echo "Overwrite tenant id and timeline id in spec file"
-sed -i "s/TENANT_ID/${tenant_id}/" ${CONFIG_FILE}
-sed -i "s/TIMELINE_ID/${timeline_id}/" ${CONFIG_FILE}
+sed -i "s/TENANT_ID/${tenant_id}/" ${SPEC_FILE}
+sed -i "s/TIMELINE_ID/${timeline_id}/" ${SPEC_FILE}
 
-cat ${CONFIG_FILE}
-
-# TODO(tristan957): Remove these workarounds for backwards compatibility after
-# the next compute release. That includes these next few lines and the
-# --spec-path in the compute_ctl invocation.
-if compute_ctl --help | grep --quiet -- '--config'; then
-  SPEC_PATH="$CONFIG_FILE"
-else
-  jq '.spec' < "$CONFIG_FILE" > /tmp/spec.json
-  SPEC_PATH=/tmp/spec.json
-fi
+cat ${SPEC_FILE}
 
 echo "Start compute node"
 /usr/local/bin/compute_ctl --pgdata /var/db/postgres/compute \
      -C "postgresql://cloud_admin@localhost:55433/postgres"  \
      -b /usr/local/bin/postgres                              \
      --compute-id "compute-$RANDOM"                          \
-     --spec-path "$SPEC_PATH"
+     -S ${SPEC_FILE}

docker-compose/compute_wrapper/var/db/postgres/configs/config.json

@@ -1,148 +0,0 @@
-{
-    "spec": {
-        "format_version": 1.0,
-
-        "timestamp": "2022-10-12T18:00:00.000Z",
-        "operation_uuid": "0f657b36-4b0f-4a2d-9c2e-1dcd615e7d8c",
-
-        "cluster": {
-            "cluster_id": "docker_compose",
-            "name": "docker_compose_test",
-            "state": "restarted",
-            "roles": [
-                {
-                    "name": "cloud_admin",
-                    "encrypted_password": "b093c0d3b281ba6da1eacc608620abd8",
-                    "options": null
-                }
-            ],
-            "databases": [
-            ],
-            "settings": [
-                {
-                    "name": "fsync",
-                    "value": "off",
-                    "vartype": "bool"
-                },
-                {
-                    "name": "wal_level",
-                    "value": "logical",
-                    "vartype": "enum"
-                },
-                {
-                    "name": "wal_log_hints",
-                    "value": "on",
-                    "vartype": "bool"
-                },
-                {
-                    "name": "log_connections",
-                    "value": "on",
-                    "vartype": "bool"
-                },
-                {
-                    "name": "port",
-                    "value": "55433",
-                    "vartype": "integer"
-                },
-                {
-                    "name": "shared_buffers",
-                    "value": "1MB",
-                    "vartype": "string"
-                },
-                {
-                    "name": "max_connections",
-                    "value": "100",
-                    "vartype": "integer"
-                },
-                {
-                    "name": "listen_addresses",
-                    "value": "0.0.0.0",
-                    "vartype": "string"
-                },
-                {
-                    "name": "max_wal_senders",
-                    "value": "10",
-                    "vartype": "integer"
-                },
-                {
-                    "name": "max_replication_slots",
-                    "value": "10",
-                    "vartype": "integer"
-                },
-                {
-                    "name": "wal_sender_timeout",
-                    "value": "5s",
-                    "vartype": "string"
-                },
-                {
-                    "name": "wal_keep_size",
-                    "value": "0",
-                    "vartype": "integer"
-                },
-                {
-                    "name": "password_encryption",
-                    "value": "md5",
-                    "vartype": "enum"
-                },
-                {
-                    "name": "restart_after_crash",
-                    "value": "off",
-                    "vartype": "bool"
-                },
-                {
-                    "name": "synchronous_standby_names",
-                    "value": "walproposer",
-                    "vartype": "string"
-                },
-                {
-                    "name": "shared_preload_libraries",
-                    "value": "neon,pg_cron,timescaledb,pg_stat_statements",
-                    "vartype": "string"
-                },
-                {
-                    "name": "neon.safekeepers",
-                    "value": "safekeeper1:5454,safekeeper2:5454,safekeeper3:5454",
-                    "vartype": "string"
-                },
-                {
-                    "name": "neon.timeline_id",
-                    "value": "TIMELINE_ID",
-                    "vartype": "string"
-                },
-                {
-                    "name": "neon.tenant_id",
-                    "value": "TENANT_ID",
-                    "vartype": "string"
-                },
-                {
-                    "name": "neon.pageserver_connstring",
-                    "value": "host=pageserver port=6400",
-                    "vartype": "string"
-                },
-                {
-                    "name": "max_replication_write_lag",
-                    "value": "500MB",
-                    "vartype": "string"
-                },
-                {
-                    "name": "max_replication_flush_lag",
-                    "value": "10GB",
-                    "vartype": "string"
-                },
-                {
-                    "name": "cron.database",
-                    "value": "postgres",
-                    "vartype": "string"
-                }
-            ]
-        },
-
-        "delta_operations": [
-        ]
-    },
-    "compute_ctl_config": {
-        "jwks": {
-            "keys": []
-        }
-    }
-}

docker-compose/compute_wrapper/var/db/postgres/specs/spec.json

@@ -0,0 +1,141 @@
+{
+    "format_version": 1.0,
+
+    "timestamp": "2022-10-12T18:00:00.000Z",
+    "operation_uuid": "0f657b36-4b0f-4a2d-9c2e-1dcd615e7d8c",
+
+    "cluster": {
+        "cluster_id": "docker_compose",
+        "name": "docker_compose_test",
+        "state": "restarted",
+        "roles": [
+            {
+                "name": "cloud_admin",
+                "encrypted_password": "b093c0d3b281ba6da1eacc608620abd8",
+                "options": null
+            }
+        ],
+        "databases": [
+        ],
+        "settings": [
+            {
+                "name": "fsync",
+                "value": "off",
+                "vartype": "bool"
+            },
+            {
+                "name": "wal_level",
+                "value": "logical",
+                "vartype": "enum"
+            },
+            {
+                "name": "wal_log_hints",
+                "value": "on",
+                "vartype": "bool"
+            },
+            {
+                "name": "log_connections",
+                "value": "on",
+                "vartype": "bool"
+            },
+            {
+                "name": "port",
+                "value": "55433",
+                "vartype": "integer"
+            },
+            {
+                "name": "shared_buffers",
+                "value": "1MB",
+                "vartype": "string"
+            },
+            {
+                "name": "max_connections",
+                "value": "100",
+                "vartype": "integer"
+            },
+            {
+                "name": "listen_addresses",
+                "value": "0.0.0.0",
+                "vartype": "string"
+            },
+            {
+                "name": "max_wal_senders",
+                "value": "10",
+                "vartype": "integer"
+            },
+            {
+                "name": "max_replication_slots",
+                "value": "10",
+                "vartype": "integer"
+            },
+            {
+                "name": "wal_sender_timeout",
+                "value": "5s",
+                "vartype": "string"
+            },
+            {
+                "name": "wal_keep_size",
+                "value": "0",
+                "vartype": "integer"
+            },
+            {
+                "name": "password_encryption",
+                "value": "md5",
+                "vartype": "enum"
+            },
+            {
+                "name": "restart_after_crash",
+                "value": "off",
+                "vartype": "bool"
+            },
+            {
+                "name": "synchronous_standby_names",
+                "value": "walproposer",
+                "vartype": "string"
+            },
+            {
+                "name": "shared_preload_libraries",
+                "value": "neon,pg_cron,timescaledb,pg_stat_statements",
+                "vartype": "string"
+            },
+            {
+                "name": "neon.safekeepers",
+                "value": "safekeeper1:5454,safekeeper2:5454,safekeeper3:5454",
+                "vartype": "string"
+            },
+            {
+                "name": "neon.timeline_id",
+                "value": "TIMELINE_ID",
+                "vartype": "string"
+            },
+            {
+                "name": "neon.tenant_id",
+                "value": "TENANT_ID",
+                "vartype": "string"
+            },
+            {
+                "name": "neon.pageserver_connstring",
+                "value": "host=pageserver port=6400",
+                "vartype": "string"
+            },
+            {
+                "name": "max_replication_write_lag",
+                "value": "500MB",
+                "vartype": "string"
+            },
+            {
+                "name": "max_replication_flush_lag",
+                "value": "10GB",
+                "vartype": "string"
+            },
+            {
+                "name": "cron.database",
+                "value": "postgres",
+                "vartype": "string"
+            }
+        ]
+    },
+
+    "delta_operations": [
+    ]
+}

docker-compose/docker-compose.yml

@@ -159,7 +159,7 @@ services:
       #- RUST_BACKTRACE=1
     # Mount the test files directly, for faster editing cycle.
     volumes:
-      - ./compute_wrapper/var/db/postgres/configs/:/var/db/postgres/configs/
+      - ./compute_wrapper/var/db/postgres/specs/:/var/db/postgres/specs/
       - ./compute_wrapper/shell/:/shell/
     ports:
       - 55433:55433 # pg protocol handler

libs/compute_api/src/responses.rs

@@ -14,32 +14,6 @@ pub struct GenericAPIError {
     pub error: String,
 }
 
-/// All configuration parameters necessary for a compute. When
-/// [`ComputeConfig::spec`] is provided, it means that the compute is attached
-/// to a tenant. [`ComputeConfig::compute_ctl_config`] will always be provided
-/// and contains parameters necessary for operating `compute_ctl` independently
-/// of whether a tenant is attached to the compute or not.
-///
-/// This also happens to be the body of `compute_ctl`'s /configure request.
-#[derive(Debug, Deserialize, Serialize)]
-pub struct ComputeConfig {
-    /// The compute spec
-    pub spec: Option<ComputeSpec>,
-
-    /// The compute_ctl configuration
-    #[allow(dead_code)]
-    pub compute_ctl_config: ComputeCtlConfig,
-}
-
-impl From<ControlPlaneConfigResponse> for ComputeConfig {
-    fn from(value: ControlPlaneConfigResponse) -> Self {
-        Self {
-            spec: value.spec,
-            compute_ctl_config: value.compute_ctl_config,
-        }
-    }
-}
-
 #[derive(Debug, Clone, Serialize)]
 pub struct ExtensionInstallResponse {
     pub extension: PgIdent,
@@ -187,7 +161,7 @@ pub struct TlsConfig {
 
 /// Response of the `/computes/{compute_id}/spec` control-plane API.
 #[derive(Deserialize, Debug)]
-pub struct ControlPlaneConfigResponse {
+pub struct ControlPlaneSpecResponse {
     pub spec: Option<ComputeSpec>,
     pub status: ControlPlaneComputeStatus,
     pub compute_ctl_config: ComputeCtlConfig,

libs/compute_api/src/spec.rs

@@ -1,8 +1,8 @@
-//! The ComputeSpec contains all the information needed to start up
-//! the right version of PostgreSQL, and connect it to the storage nodes.
-//! It can be passed as part of the `config.json`, or the control plane can
-//! provide it by calling the compute_ctl's `/compute_ctl` endpoint, or
-//! compute_ctl can fetch it by calling the control plane's API.
+//! `ComputeSpec` represents the contents of the spec.json file.
+//!
+//! The spec.json file is used to pass information to 'compute_ctl'. It contains
+//! all the information needed to start up the right version of PostgreSQL,
+//! and connect it to the storage nodes.
 use std::collections::HashMap;
 
 use indexmap::IndexMap;
@@ -104,12 +104,6 @@ pub struct ComputeSpec {
     pub timeline_id: Option<TimelineId>,
     pub pageserver_connstring: Option<String>,
 
-    // More neon ids that we expose to the compute_ctl
-    // and to postgres as neon extension GUCs.
-    pub project_id: Option<String>,
-    pub branch_id: Option<String>,
-    pub endpoint_id: Option<String>,
-
     /// Safekeeper membership config generation. It is put in
     /// neon.safekeepers GUC and serves two purposes:
     /// 1) Non zero value forces walproposer to use membership configurations.
@@ -165,7 +159,13 @@ pub struct ComputeSpec {
     #[serde(default)] // Default false
     pub drop_subscriptions_before_start: bool,
 
-    /// Log level for compute audit logging
+    /// Log level for audit logging:
+    ///
+    /// Disabled - no audit logging. This is the default.
+    /// log - log masked statements to the postgres log using pgaudit extension
+    /// hipaa - log unmasked statements to the file using pgaudit and pgauditlogtofile extension
+    ///
+    /// Extensions should be present in shared_preload_libraries
     #[serde(default)]
     pub audit_log_level: ComputeAudit,
 
@@ -289,25 +289,14 @@ impl ComputeMode {
 }
 
 /// Log level for audit logging
+/// Disabled, log, hipaa
+/// Default is Disabled
 #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize, Serialize)]
 pub enum ComputeAudit {
     #[default]
     Disabled,
-    // Deprecated, use Base instead
     Log,
-    // (pgaudit.log = 'ddl', pgaudit.log_parameter='off')
-    // logged to the standard postgresql log stream
-    Base,
-    // Deprecated, use Full or Extended instead
     Hipaa,
-    // (pgaudit.log = 'all, -misc', pgaudit.log_parameter='off')
-    // logged to separate files collected by rsyslog
-    // into dedicated log storage with strict access
-    Extended,
-    // (pgaudit.log='all', pgaudit.log_parameter='on'),
-    // logged to separate files collected by rsyslog
-    // into dedicated log storage with strict access.
-    Full,
 }
 
 #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)]

libs/http-utils/Cargo.toml

@@ -30,7 +30,6 @@ tokio.workspace = true
 tracing.workspace = true
 url.workspace = true
 uuid.workspace = true
-x509-cert.workspace = true
 
 # to use tokio channels as streams, this is faster to compile than async_stream
 # why is it only here? no other crate should use it, streams are rarely needed.

libs/http-utils/src/server.rs

@@ -4,8 +4,6 @@ use futures::StreamExt;
 use futures::stream::FuturesUnordered;
 use hyper0::Body;
 use hyper0::server::conn::Http;
-use metrics::{IntCounterVec, register_int_counter_vec};
-use once_cell::sync::Lazy;
 use routerify::{RequestService, RequestServiceBuilder};
 use tokio::io::{AsyncRead, AsyncWrite};
 use tokio_rustls::TlsAcceptor;
@@ -28,24 +26,6 @@ pub struct Server {
     tls_acceptor: Option<TlsAcceptor>,
 }
 
-static CONNECTION_STARTED_COUNT: Lazy<IntCounterVec> = Lazy::new(|| {
-    register_int_counter_vec!(
-        "http_server_connection_started_total",
-        "Number of established http/https connections",
-        &["scheme"]
-    )
-    .expect("failed to define a metric")
-});
-
-static CONNECTION_ERROR_COUNT: Lazy<IntCounterVec> = Lazy::new(|| {
-    register_int_counter_vec!(
-        "http_server_connection_errors_total",
-        "Number of occured connection errors by type",
-        &["type"]
-    )
-    .expect("failed to define a metric")
-});
-
 impl Server {
     pub fn new(
         request_service: Arc<RequestServiceBuilder<Body, ApiError>>,
@@ -80,15 +60,6 @@ impl Server {
             false
         }
 
-        let tcp_error_cnt = CONNECTION_ERROR_COUNT.with_label_values(&["tcp"]);
-        let tls_error_cnt = CONNECTION_ERROR_COUNT.with_label_values(&["tls"]);
-        let http_error_cnt = CONNECTION_ERROR_COUNT.with_label_values(&["http"]);
-        let https_error_cnt = CONNECTION_ERROR_COUNT.with_label_values(&["https"]);
-        let panic_error_cnt = CONNECTION_ERROR_COUNT.with_label_values(&["panic"]);
-
-        let http_connection_cnt = CONNECTION_STARTED_COUNT.with_label_values(&["http"]);
-        let https_connection_cnt = CONNECTION_STARTED_COUNT.with_label_values(&["https"]);
-
         let mut connections = FuturesUnordered::new();
         loop {
             tokio::select! {
@@ -96,7 +67,6 @@ impl Server {
                     let (tcp_stream, remote_addr) = match stream {
                         Ok(stream) => stream,
                         Err(err) => {
-                            tcp_error_cnt.inc();
                             if !suppress_io_error(&err) {
                                 info!("Failed to accept TCP connection: {err:#}");
                             }
@@ -108,18 +78,11 @@ impl Server {
                     let tls_acceptor = self.tls_acceptor.clone();
                     let cancel = cancel.clone();
 
-                    let tls_error_cnt = tls_error_cnt.clone();
-                    let http_error_cnt = http_error_cnt.clone();
-                    let https_error_cnt = https_error_cnt.clone();
-                    let http_connection_cnt = http_connection_cnt.clone();
-                    let https_connection_cnt = https_connection_cnt.clone();
-
                     connections.push(tokio::spawn(
                         async move {
                             match tls_acceptor {
                                 Some(tls_acceptor) => {
                                     // Handle HTTPS connection.
-                                    https_connection_cnt.inc();
                                     let tls_stream = tokio::select! {
                                         tls_stream = tls_acceptor.accept(tcp_stream) => tls_stream,
                                         _ = cancel.cancelled() => return,
@@ -127,7 +90,6 @@ impl Server {
                                     let tls_stream = match tls_stream {
                                         Ok(tls_stream) => tls_stream,
                                         Err(err) => {
-                                            tls_error_cnt.inc();
                                             if !suppress_io_error(&err) {
                                                 info!(%remote_addr, "Failed to accept TLS connection: {err:#}");
                                             }
@@ -135,7 +97,6 @@ impl Server {
                                         }
                                     };
                                     if let Err(err) = Self::serve_connection(tls_stream, service, cancel).await {
-                                        https_error_cnt.inc();
                                         if !suppress_hyper_error(&err) {
                                             info!(%remote_addr, "Failed to serve HTTPS connection: {err:#}");
                                         }
@@ -143,9 +104,7 @@ impl Server {
                                 }
                                 None => {
                                     // Handle HTTP connection.
-                                    http_connection_cnt.inc();
                                     if let Err(err) = Self::serve_connection(tcp_stream, service, cancel).await {
-                                        http_error_cnt.inc();
                                         if !suppress_hyper_error(&err) {
                                             info!(%remote_addr, "Failed to serve HTTP connection: {err:#}");
                                         }
@@ -156,7 +115,6 @@ impl Server {
                  }
                 Some(conn) = connections.next() => {
                     if let Err(err) = conn {
-                        panic_error_cnt.inc();
                         error!("Connection panicked: {err:#}");
                     }
                 }
@@ -164,7 +122,6 @@ impl Server {
                     // Wait for graceful shutdown of all connections.
                     while let Some(conn) = connections.next().await {
                         if let Err(err) = conn {
-                            panic_error_cnt.inc();
                             error!("Connection panicked: {err:#}");
                         }
                     }

libs/http-utils/src/tls_certs.rs

@@ -3,14 +3,11 @@ use std::{sync::Arc, time::Duration};
 use anyhow::Context;
 use arc_swap::ArcSwap;
 use camino::Utf8Path;
-use metrics::{IntCounterVec, UIntGaugeVec, register_int_counter_vec, register_uint_gauge_vec};
-use once_cell::sync::Lazy;
 use rustls::{
-    pki_types::{CertificateDer, PrivateKeyDer, UnixTime},
+    pki_types::{CertificateDer, PrivateKeyDer},
     server::{ClientHello, ResolvesServerCert},
     sign::CertifiedKey,
 };
-use x509_cert::der::Reader;
 
 pub async fn load_cert_chain(filename: &Utf8Path) -> anyhow::Result<Vec<CertificateDer<'static>>> {
     let cert_data = tokio::fs::read(filename)
@@ -56,76 +53,6 @@ pub async fn load_certified_key(
     Ok(certified_key)
 }
 
-/// rustls's CertifiedKey with extra parsed fields used for metrics.
-struct ParsedCertifiedKey {
-    certified_key: CertifiedKey,
-    expiration_time: UnixTime,
-}
-
-/// Parse expiration time from an X509 certificate.
-fn parse_expiration_time(cert: &CertificateDer<'_>) -> anyhow::Result<UnixTime> {
-    let parsed_cert = x509_cert::der::SliceReader::new(cert)
-        .context("Failed to parse cerficiate")?
-        .decode::<x509_cert::Certificate>()
-        .context("Failed to parse cerficiate")?;
-
-    Ok(UnixTime::since_unix_epoch(
-        parsed_cert
-            .tbs_certificate
-            .validity
-            .not_after
-            .to_unix_duration(),
-    ))
-}
-
-async fn load_and_parse_certified_key(
-    key_filename: &Utf8Path,
-    cert_filename: &Utf8Path,
-) -> anyhow::Result<ParsedCertifiedKey> {
-    let certified_key = load_certified_key(key_filename, cert_filename).await?;
-    let expiration_time = parse_expiration_time(certified_key.end_entity_cert()?)?;
-    Ok(ParsedCertifiedKey {
-        certified_key,
-        expiration_time,
-    })
-}
-
-static CERT_EXPIRATION_TIME: Lazy<UIntGaugeVec> = Lazy::new(|| {
-    register_uint_gauge_vec!(
-        "tls_certs_expiration_time_seconds",
-        "Expiration time of the loaded certificate since unix epoch in seconds",
-        &["resolver_name"]
-    )
-    .expect("failed to define a metric")
-});
-
-static CERT_RELOAD_STARTED_COUNTER: Lazy<IntCounterVec> = Lazy::new(|| {
-    register_int_counter_vec!(
-        "tls_certs_reload_started_total",
-        "Number of certificate reload loop iterations started",
-        &["resolver_name"]
-    )
-    .expect("failed to define a metric")
-});
-
-static CERT_RELOAD_UPDATED_COUNTER: Lazy<IntCounterVec> = Lazy::new(|| {
-    register_int_counter_vec!(
-        "tls_certs_reload_updated_total",
-        "Number of times the certificate was updated to the new one",
-        &["resolver_name"]
-    )
-    .expect("failed to define a metric")
-});
-
-static CERT_RELOAD_FAILED_COUNTER: Lazy<IntCounterVec> = Lazy::new(|| {
-    register_int_counter_vec!(
-        "tls_certs_reload_failed_total",
-        "Number of times the certificate reload failed",
-        &["resolver_name"]
-    )
-    .expect("failed to define a metric")
-});
-
 /// Implementation of [`rustls::server::ResolvesServerCert`] which reloads certificates from
 /// the disk periodically.
 #[derive(Debug)]
@@ -136,28 +63,16 @@ pub struct ReloadingCertificateResolver {
 impl ReloadingCertificateResolver {
     /// Creates a new Resolver by loading certificate and private key from FS and
     /// creating tokio::task to reload them with provided reload_period.
-    /// resolver_name is used as metric's label.
     pub async fn new(
-        resolver_name: &str,
         key_filename: &Utf8Path,
         cert_filename: &Utf8Path,
         reload_period: Duration,
     ) -> anyhow::Result<Arc<Self>> {
-        // Create metrics for current resolver.
-        let cert_expiration_time = CERT_EXPIRATION_TIME.with_label_values(&[resolver_name]);
-        let cert_reload_started_counter =
-            CERT_RELOAD_STARTED_COUNTER.with_label_values(&[resolver_name]);
-        let cert_reload_updated_counter =
-            CERT_RELOAD_UPDATED_COUNTER.with_label_values(&[resolver_name]);
-        let cert_reload_failed_counter =
-            CERT_RELOAD_FAILED_COUNTER.with_label_values(&[resolver_name]);
-
-        let parsed_key = load_and_parse_certified_key(key_filename, cert_filename).await?;
-
         let this = Arc::new(Self {
-            certified_key: ArcSwap::from_pointee(parsed_key.certified_key),
+            certified_key: ArcSwap::from_pointee(
+                load_certified_key(key_filename, cert_filename).await?,
+            ),
         });
-        cert_expiration_time.set(parsed_key.expiration_time.as_secs());
 
         tokio::spawn({
             let weak_this = Arc::downgrade(&this);
@@ -173,22 +88,17 @@ impl ReloadingCertificateResolver {
                         Some(this) => this,
                         None => break, // Resolver has been destroyed, exit.
                     };
-                    cert_reload_started_counter.inc();
-
-                    match load_and_parse_certified_key(&key_filename, &cert_filename).await {
-                        Ok(parsed_key) => {
-                            if parsed_key.certified_key.cert == this.certified_key.load().cert {
+                    match load_certified_key(&key_filename, &cert_filename).await {
+                        Ok(new_certified_key) => {
+                            if new_certified_key.cert == this.certified_key.load().cert {
                                 tracing::debug!("Certificate has not changed since last reloading");
                             } else {
                                 tracing::info!("Certificate has been reloaded");
-                                this.certified_key.store(Arc::new(parsed_key.certified_key));
-                                cert_expiration_time.set(parsed_key.expiration_time.as_secs());
-                                cert_reload_updated_counter.inc();
+                                this.certified_key.store(Arc::new(new_certified_key));
                             }
                             last_reload_failed = false;
                         }
                         Err(err) => {
-                            cert_reload_failed_counter.inc();
                             // Note: Reloading certs may fail if it conflicts with the script updating
                             // the files at the same time. Warn only if the error is persistent.
                             if last_reload_failed {

libs/pageserver_api/src/config.rs

@@ -180,7 +180,8 @@ pub struct ConfigToml {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub generate_unarchival_heatmap: Option<bool>,
     pub tracing: Option<Tracing>,
-    pub enable_tls_page_service_api: bool,
+
+    pub dev_mode: bool,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
@@ -632,7 +633,7 @@ impl Default for ConfigToml {
             load_previous_heatmap: None,
             generate_unarchival_heatmap: None,
             tracing: None,
-            enable_tls_page_service_api: false,
+            dev_mode: false,
         }
     }
 }

pageserver/src/bin/pageserver.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
 
-use anyhow::{Context, anyhow};
+use anyhow::{Context, anyhow, bail};
 use camino::Utf8Path;
 use clap::{Arg, ArgAction, Command};
 use http_utils::tls_certs::ReloadingCertificateResolver;
@@ -99,6 +99,20 @@ fn main() -> anyhow::Result<()> {
 
     let (conf, ignored) = initialize_config(&identity_file_path, &cfg_file_path, &workdir)?;
 
+    if !conf.dev_mode {
+        if matches!(conf.http_auth_type, AuthType::Trust)
+            || matches!(conf.pg_auth_type, AuthType::Trust)
+        {
+            bail!(
+                "Pageserver refuses to start with HTTP or PostgreSQL API authentication disabled.\n\
+                  Run with --dev to allow running without authentication.\n\
+                  This is insecure and should only be used in development environments."
+            );
+        }
+    } else {
+        warn!("Starting in dev mode: this may be an insecure configuration.");
+    }
+
     // Initialize logging.
     //
     // It must be initialized before the custom panic hook is installed below.
@@ -452,24 +466,6 @@ fn start_pageserver(
     info!("Using auth for http API: {:#?}", conf.http_auth_type);
     info!("Using auth for pg connections: {:#?}", conf.pg_auth_type);
 
-    let tls_server_config = if conf.listen_https_addr.is_some() || conf.enable_tls_page_service_api
-    {
-        let resolver = BACKGROUND_RUNTIME.block_on(ReloadingCertificateResolver::new(
-            "main",
-            &conf.ssl_key_file,
-            &conf.ssl_cert_file,
-            conf.ssl_cert_reload_period,
-        ))?;
-
-        let server_config = rustls::ServerConfig::builder()
-            .with_no_client_auth()
-            .with_cert_resolver(resolver);
-
-        Some(Arc::new(server_config))
-    } else {
-        None
-    };
-
     match var("NEON_AUTH_TOKEN") {
         Ok(v) => {
             info!("Loaded JWT token for authentication with Safekeeper");
@@ -688,11 +684,17 @@ fn start_pageserver(
 
         let https_task = match https_listener {
             Some(https_listener) => {
-                let tls_server_config = tls_server_config
-                    .clone()
-                    .expect("tls_server_config is set earlier if https is enabled");
+                let resolver = MGMT_REQUEST_RUNTIME.block_on(ReloadingCertificateResolver::new(
+                    &conf.ssl_key_file,
+                    &conf.ssl_cert_file,
+                    conf.ssl_cert_reload_period,
+                ))?;
 
-                let tls_acceptor = tokio_rustls::TlsAcceptor::from(tls_server_config);
+                let server_config = rustls::ServerConfig::builder()
+                    .with_no_client_auth()
+                    .with_cert_resolver(resolver);
+
+                let tls_acceptor = tokio_rustls::TlsAcceptor::from(Arc::new(server_config));
 
                 let server =
                     http_utils::server::Server::new(service, https_listener, Some(tls_acceptor))?;
@@ -748,11 +750,6 @@ fn start_pageserver(
             tokio::net::TcpListener::from_std(pageserver_listener)
                 .context("create tokio listener")?
         },
-        if conf.enable_tls_page_service_api {
-            tls_server_config
-        } else {
-            None
-        },
     );
 
     // All started up! Now just sit and wait for shutdown signal.

pageserver/src/config.rs

@@ -220,10 +220,7 @@ pub struct PageServerConf {
 
     pub tracing: Option<pageserver_api::config::Tracing>,
 
-    /// Enable TLS in page service API.
-    /// Does not force TLS: the client negotiates TLS usage during the handshake.
-    /// Uses key and certificate from ssl_key_file/ssl_cert_file.
-    pub enable_tls_page_service_api: bool,
+    pub dev_mode: bool,
 }
 
 /// Token for authentication to safekeepers
@@ -396,7 +393,6 @@ impl PageServerConf {
             load_previous_heatmap,
             generate_unarchival_heatmap,
             tracing,
-            enable_tls_page_service_api,
         } = config_toml;
 
         let mut conf = PageServerConf {
@@ -447,7 +443,7 @@ impl PageServerConf {
             page_service_pipelining,
             get_vectored_concurrent_io,
             tracing,
-            enable_tls_page_service_api,
+            dev_mode,
 
             // ------------------------------------------------------------
             // fields that require additional validation or custom handling

pageserver/src/page_service.rs

@@ -105,7 +105,6 @@ pub fn spawn(
     pg_auth: Option<Arc<SwappableJwtAuth>>,
     perf_trace_dispatch: Option<Dispatch>,
     tcp_listener: tokio::net::TcpListener,
-    tls_config: Option<Arc<rustls::ServerConfig>>,
 ) -> Listener {
     let cancel = CancellationToken::new();
     let libpq_ctx = RequestContext::todo_child(
@@ -125,7 +124,6 @@ pub fn spawn(
             perf_trace_dispatch,
             tcp_listener,
             conf.pg_auth_type,
-            tls_config,
             conf.page_service_pipelining.clone(),
             libpq_ctx,
             cancel.clone(),
@@ -183,7 +181,6 @@ pub async fn libpq_listener_main(
     perf_trace_dispatch: Option<Dispatch>,
     listener: tokio::net::TcpListener,
     auth_type: AuthType,
-    tls_config: Option<Arc<rustls::ServerConfig>>,
     pipelining_config: PageServicePipeliningConfig,
     listener_ctx: RequestContext,
     listener_cancel: CancellationToken,
@@ -226,7 +223,6 @@ pub async fn libpq_listener_main(
                     local_auth,
                     socket,
                     auth_type,
-                    tls_config.clone(),
                     pipelining_config.clone(),
                     connection_ctx,
                     connections_cancel.child_token(),
@@ -268,7 +264,6 @@ async fn page_service_conn_main(
     auth: Option<Arc<SwappableJwtAuth>>,
     socket: tokio::net::TcpStream,
     auth_type: AuthType,
-    tls_config: Option<Arc<rustls::ServerConfig>>,
     pipelining_config: PageServicePipeliningConfig,
     connection_ctx: RequestContext,
     cancel: CancellationToken,
@@ -339,8 +334,7 @@ async fn page_service_conn_main(
         cancel.clone(),
         gate_guard,
     );
-    let pgbackend =
-        PostgresBackend::new_from_io(socket_fd, socket, peer_addr, auth_type, tls_config)?;
+    let pgbackend = PostgresBackend::new_from_io(socket_fd, socket, peer_addr, auth_type, None)?;
 
     match pgbackend.run(&mut conn_handler, &cancel).await {
         Ok(()) => {

pageserver/src/tenant.rs

@@ -11571,99 +11571,6 @@ mod tests {
         Ok(())
     }
 
-    #[cfg(feature = "testing")]
-    #[tokio::test]
-    async fn test_bottom_most_compation_redo_failure() -> anyhow::Result<()> {
-        let harness = TenantHarness::create("test_bottom_most_compation_redo_failure").await?;
-        let (tenant, ctx) = harness.load().await;
-
-        fn get_key(id: u32) -> Key {
-            // using aux key here b/c they are guaranteed to be inside `collect_keyspace`.
-            let mut key = Key::from_hex("620000000033333333444444445500000000").unwrap();
-            key.field6 = id;
-            key
-        }
-
-        let img_layer = (0..10)
-            .map(|id| (get_key(id), Bytes::from(format!("value {id}@0x10"))))
-            .collect_vec();
-
-        let delta1 = vec![
-            (
-                get_key(1),
-                Lsn(0x20),
-                Value::WalRecord(NeonWalRecord::wal_append("@0x20")),
-            ),
-            (
-                get_key(1),
-                Lsn(0x24),
-                Value::WalRecord(NeonWalRecord::wal_append("@0x24")),
-            ),
-            (
-                get_key(1),
-                Lsn(0x28),
-                // This record will fail to redo
-                Value::WalRecord(NeonWalRecord::wal_append_conditional("@0x28", "???")),
-            ),
-        ];
-
-        let tline = tenant
-            .create_test_timeline_with_layers(
-                TIMELINE_ID,
-                Lsn(0x10),
-                DEFAULT_PG_VERSION,
-                &ctx,
-                vec![], // in-memory layers
-                vec![DeltaLayerTestDesc::new_with_inferred_key_range(
-                    Lsn(0x20)..Lsn(0x30),
-                    delta1,
-                )], // delta layers
-                vec![(Lsn(0x10), img_layer)], // image layers
-                Lsn(0x50),
-            )
-            .await?;
-        {
-            tline
-                .applied_gc_cutoff_lsn
-                .lock_for_write()
-                .store_and_unlock(Lsn(0x30))
-                .wait()
-                .await;
-            // Update GC info
-            let mut guard = tline.gc_info.write().unwrap();
-            *guard = GcInfo {
-                retain_lsns: vec![],
-                cutoffs: GcCutoffs {
-                    time: Lsn(0x30),
-                    space: Lsn(0x30),
-                },
-                leases: Default::default(),
-                within_ancestor_pitr: false,
-            };
-        }
-
-        let cancel = CancellationToken::new();
-
-        // Compaction will fail, but should not fire any critical error.
-        // Gc-compaction currently cannot figure out what keys are not in the keyspace during the compaction
-        // process. It will always try to redo the logs it reads and if it doesn't work, fail the entire
-        // compaction job. Tracked in <https://github.com/neondatabase/neon/issues/10395>.
-        let res = tline
-            .compact_with_gc(
-                &cancel,
-                CompactOptions {
-                    compact_key_range: None,
-                    compact_lsn_range: None,
-                    ..Default::default()
-                },
-                &ctx,
-            )
-            .await;
-        assert!(res.is_err());
-
-        Ok(())
-    }
-
     #[cfg(feature = "testing")]
     #[tokio::test]
     async fn test_synthetic_size_calculation_with_invisible_branches() -> anyhow::Result<()> {

pageserver/src/tenant/storage_layer/image_layer.rs

@@ -1192,7 +1192,7 @@ mod test {
 
         // This key range contains several 0x8000 page stripes, only one of which belongs to shard zero
         let input_start = Key::from_hex("000000067f00000001000000ae0000000000").unwrap();
-        let input_end = Key::from_hex("000000067f00000001000000ae0000002000").unwrap();
+        let input_end = Key::from_hex("000000067f00000001000000ae0000020000").unwrap();
         let range = input_start..input_end;
 
         // Build an image layer to filter
@@ -1235,7 +1235,7 @@ mod test {
             let shard_identity = ShardIdentity::new(
                 ShardNumber(shard_number),
                 shard_count,
-                ShardStripeSize(0x800),
+                ShardStripeSize(0x8000),
             )
             .unwrap();
             let harness = TenantHarness::create_custom(
@@ -1287,12 +1287,12 @@ mod test {
 
             // This exact size and those below will need updating as/when the layer encoding changes, but
             // should be deterministic for a given version of the format, as we used no randomness generating the input.
-            assert_eq!(original_size, 122880);
+            assert_eq!(original_size, 1597440);
 
             match shard_number {
                 0 => {
                     // We should have written out just one stripe for our shard identity
-                    assert_eq!(wrote_keys, 0x800);
+                    assert_eq!(wrote_keys, 0x8000);
                     let replacement = replacement.unwrap();
 
                     // We should have dropped some of the data
@@ -1300,7 +1300,7 @@ mod test {
                     assert!(replacement.metadata().file_size > 0);
 
                     // Assert that we dropped ~3/4 of the data.
-                    assert_eq!(replacement.metadata().file_size, 49152);
+                    assert_eq!(replacement.metadata().file_size, 417792);
                 }
                 1 => {
                     // Shard 1 has no keys in our input range
@@ -1309,19 +1309,19 @@ mod test {
                 }
                 2 => {
                     // Shard 2 has one stripes in the input range
-                    assert_eq!(wrote_keys, 0x800);
+                    assert_eq!(wrote_keys, 0x8000);
                     let replacement = replacement.unwrap();
                     assert!(replacement.metadata().file_size < original_size);
                     assert!(replacement.metadata().file_size > 0);
-                    assert_eq!(replacement.metadata().file_size, 49152);
+                    assert_eq!(replacement.metadata().file_size, 417792);
                 }
                 3 => {
                     // Shard 3 has two stripes in the input range
-                    assert_eq!(wrote_keys, 0x1000);
+                    assert_eq!(wrote_keys, 0x10000);
                     let replacement = replacement.unwrap();
                     assert!(replacement.metadata().file_size < original_size);
                     assert!(replacement.metadata().file_size > 0);
-                    assert_eq!(replacement.metadata().file_size, 73728);
+                    assert_eq!(replacement.metadata().file_size, 811008);
                 }
                 _ => unreachable!(),
             }

pgxn/neon/libpagestore.c

@@ -65,9 +65,6 @@ static const struct config_enum_entry neon_compute_modes[] = {
 /* GUCs */
 char	   *neon_timeline;
 char	   *neon_tenant;
-char	   *neon_project_id;
-char	   *neon_branch_id;
-char	   *neon_endpoint_id;
 int32		max_cluster_size;
 char	   *page_server_connstring;
 char	   *neon_auth_token;
@@ -1355,31 +1352,6 @@ pg_init_libpagestore(void)
 							   0,	/* no flags required */
 							   check_neon_id, NULL, NULL);
 
-	DefineCustomStringVariable("neon.project_id",
-							   "Neon project_id the server is running on",
-							   NULL,
-							   &neon_project_id,
-							   "",
-							   PGC_POSTMASTER,
-							   0,	/* no flags required */
-							   NULL, NULL, NULL);
-	DefineCustomStringVariable("neon.branch_id",
-							   "Neon branch_id the server is running on",
-							   NULL,
-							   &neon_branch_id,
-							   "",
-							   PGC_POSTMASTER,
-							   0,	/* no flags required */
-							   NULL, NULL, NULL);
-	DefineCustomStringVariable("neon.endpoint_id",
-							   "Neon endpoint_id the server is running on",
-							   NULL,
-							   &neon_endpoint_id,
-							   "",
-							   PGC_POSTMASTER,
-							   0,	/* no flags required */
-							   NULL, NULL, NULL);
-
 	DefineCustomIntVariable("neon.stripe_size",
 							"sharding stripe size",
 							NULL,

pgxn/neon/walproposer.c

@@ -99,9 +99,6 @@ WalProposerCreate(WalProposerConfig *config, walproposer_api api)
 	wp->config = config;
 	wp->api = api;
 	wp->state = WPS_COLLECTING_TERMS;
-	wp->mconf.generation = INVALID_GENERATION;
-	wp->mconf.members.len = 0;
-	wp->mconf.new_members.len = 0;
 
 	wp_log(LOG, "neon.safekeepers=%s", wp->config->safekeepers_list);
 
@@ -173,8 +170,6 @@ WalProposerCreate(WalProposerConfig *config, walproposer_api api)
 
 	if (wp->config->proto_version != 2 && wp->config->proto_version != 3)
 		wp_log(FATAL, "unsupported safekeeper protocol version %d", wp->config->proto_version);
-	if (wp->safekeepers_generation > INVALID_GENERATION && wp->config->proto_version < 3)
-		wp_log(FATAL, "enabling generations requires protocol version 3");
 	wp_log(LOG, "using safekeeper protocol version %d", wp->config->proto_version);
 
 	/* Fill the greeting package */
@@ -219,7 +214,7 @@ WalProposerFree(WalProposer *wp)
 static bool
 WalProposerGenerationsEnabled(WalProposer *wp)
 {
-	return wp->safekeepers_generation != INVALID_GENERATION;
+	return wp->safekeepers_generation != 0;
 }
 
 /*
@@ -728,176 +723,13 @@ SendProposerGreeting(Safekeeper *sk)
 	BlockingWrite(sk, sk->outbuf.data, sk->outbuf.len, SS_HANDSHAKE_RECV);
 }
 
-/*
- * Assuming `sk` sent its node id, find such member(s) in wp->mconf and set ptr in
- * members_safekeepers & new_members_safekeepers to sk.
- */
-static void
-UpdateMemberSafekeeperPtr(WalProposer *wp, Safekeeper *sk)
-{
-	/* members_safekeepers etc are fixed size, sanity check mconf size */
-	if (wp->mconf.members.len > MAX_SAFEKEEPERS)
-		wp_log(FATAL, "too many members %d in mconf", wp->mconf.members.len);
-	if (wp->mconf.new_members.len > MAX_SAFEKEEPERS)
-		wp_log(FATAL, "too many new_members %d in mconf", wp->mconf.new_members.len);
-
-	/* node id is not known until greeting is received */
-	if (sk->state < SS_WAIT_VOTING)
-		return;
-
-	/* 0 is assumed to be invalid node id, should never happen */
-	if (sk->greetResponse.nodeId == 0)
-	{
-		wp_log(WARNING, "safekeeper %s:%s sent zero node id", sk->host, sk->port);
-		return;
-	}
-
-	for (uint32 i = 0; i < wp->mconf.members.len; i++)
-	{
-		SafekeeperId *sk_id = &wp->mconf.members.m[i];
-
-		if (wp->mconf.members.m[i].node_id == sk->greetResponse.nodeId)
-		{
-			/*
-			 * If mconf or list of safekeepers to connect to changed (the
-			 * latter always currently goes through restart though),
-			 * ResetMemberSafekeeperPtrs is expected to be called before
-			 * UpdateMemberSafekeeperPtr. So, other value suggests that we are
-			 * connected to the same sk under different host name, complain
-			 * about that.
-			 */
-			if (wp->members_safekeepers[i] != NULL && wp->members_safekeepers[i] != sk)
-			{
-				wp_log(WARNING, "safekeeper {id = %lu, ep = %s:%u } in members[%u] is already mapped to connection slot %lu",
-					   sk_id->node_id, sk_id->host, sk_id->port, i, wp->members_safekeepers[i] - wp->safekeeper);
-			}
-			wp_log(LOG, "safekeeper {id = %lu, ep = %s:%u } in members[%u] mapped to connection slot %lu",
-				   sk_id->node_id, sk_id->host, sk_id->port, i, sk - wp->safekeeper);
-			wp->members_safekeepers[i] = sk;
-		}
-	}
-	/* repeat for new_members */
-	for (uint32 i = 0; i < wp->mconf.new_members.len; i++)
-	{
-		SafekeeperId *sk_id = &wp->mconf.new_members.m[i];
-
-		if (wp->mconf.new_members.m[i].node_id == sk->greetResponse.nodeId)
-		{
-			if (wp->new_members_safekeepers[i] != NULL && wp->new_members_safekeepers[i] != sk)
-			{
-				wp_log(WARNING, "safekeeper {id = %lu, ep = %s:%u } in new_members[%u] is already mapped to connection slot %lu",
-					   sk_id->node_id, sk_id->host, sk_id->port, i, wp->new_members_safekeepers[i] - wp->safekeeper);
-			}
-			wp_log(LOG, "safekeeper {id = %lu, ep = %s:%u } in new_members[%u] mapped to connection slot %lu",
-				   sk_id->node_id, sk_id->host, sk_id->port, i, sk - wp->safekeeper);
-			wp->new_members_safekeepers[i] = sk;
-		}
-	}
-}
-
-/*
- * Reset wp->members_safekeepers & new_members_safekeepers and refill them.
- * Called after wp changes mconf.
- */
-static void
-ResetMemberSafekeeperPtrs(WalProposer *wp)
-{
-	memset(&wp->members_safekeepers, 0, sizeof(Safekeeper *) * MAX_SAFEKEEPERS);
-	memset(&wp->new_members_safekeepers, 0, sizeof(Safekeeper *) * MAX_SAFEKEEPERS);
-	for (int i = 0; i < wp->n_safekeepers; i++)
-	{
-		if (wp->safekeeper[i].state >= SS_WAIT_VOTING)
-			UpdateMemberSafekeeperPtr(wp, &wp->safekeeper[i]);
-	}
-}
-
-static uint32
-MsetQuorum(MemberSet *mset)
-{
-	Assert(mset->len > 0);
-	return mset->len / 2 + 1;
-}
-
-/* Does n forms quorum in mset? */
-static bool
-MsetHasQuorum(MemberSet *mset, uint32 n)
-{
-	return n >= MsetQuorum(mset);
-}
-
-/*
- * TermsCollected helper for a single member set `mset`.
- *
- * `msk` is the member -> safekeeper mapping for mset, i.e. members_safekeepers
- * or new_members_safekeepers.
- */
-static bool
-TermsCollectedMset(WalProposer *wp, MemberSet *mset, Safekeeper **msk, StringInfo s)
-{
-	uint32		n_greeted = 0;
-
-	for (uint32 i = 0; i < wp->mconf.members.len; i++)
-	{
-		Safekeeper *sk = msk[i];
-
-		if (sk != NULL && sk->state == SS_WAIT_VOTING)
-		{
-			if (n_greeted > 0)
-				appendStringInfoString(s, ", ");
-			appendStringInfo(s, "{id = %lu, ep = %s:%s}", sk->greetResponse.nodeId, sk->host, sk->port);
-			n_greeted++;
-		}
-	}
-	appendStringInfo(s, ", %u/%u total", n_greeted, mset->len);
-	return MsetHasQuorum(mset, n_greeted);
-}
-
 /*
  * Have we received greeting from enough (quorum) safekeepers to start voting?
  */
 static bool
 TermsCollected(WalProposer *wp)
 {
-	StringInfoData s;			/* str for logging */
-	bool		collected = false;
-
-	/* legacy: generations disabled */
-	if (!WalProposerGenerationsEnabled(wp) && wp->mconf.generation == INVALID_GENERATION)
-	{
-		collected = wp->n_connected >= wp->quorum;
-		if (collected)
-		{
-			wp->propTerm++;
-			wp_log(LOG, "walproposer connected to quorum (%d) safekeepers, propTerm=" INT64_FORMAT ", starting voting", wp->quorum, wp->propTerm);
-		}
-		return collected;
-	}
-
-	/*
-	 * With generations enabled, we start campaign only when 1) some mconf is
-	 * actually received 2) we have greetings from majority of members as well
-	 * as from majority of new_members if it exists.
-	 */
-	if (wp->mconf.generation == INVALID_GENERATION)
-		return false;
-
-	initStringInfo(&s);
-	appendStringInfoString(&s, "mset greeters: ");
-	if (!TermsCollectedMset(wp, &wp->mconf.members, wp->members_safekeepers, &s))
-		goto res;
-	if (wp->mconf.new_members.len > 0)
-	{
-		appendStringInfoString(&s, ", new_mset greeters: ");
-		if (!TermsCollectedMset(wp, &wp->mconf.new_members, wp->new_members_safekeepers, &s))
-			goto res;
-	}
-	wp->propTerm++;
-	wp_log(LOG, "walproposer connected to quorum of safekeepers: %s, propTerm=" INT64_FORMAT ", starting voting", s.data, wp->propTerm);
-	collected = true;
-
-res:
-	pfree(s.data);
-	return collected;
+	return wp->n_connected >= wp->quorum;
 }
 
 static void
@@ -921,41 +753,13 @@ RecvAcceptorGreeting(Safekeeper *sk)
 	pfree(mconf_toml);
 
 	/*
-	 * Adopt mconf of safekeepers if it is higher.
+	 * Adopt mconf of safekeepers if it is higher. TODO: mconf change should
+	 * restart wp if it started voting.
 	 */
 	if (sk->greetResponse.mconf.generation > wp->mconf.generation)
 	{
-		/* sanity check before adopting, should never happen */
-		if (sk->greetResponse.mconf.members.len == 0)
-		{
-			wp_log(FATAL, "mconf %u has zero members", sk->greetResponse.mconf.generation);
-		}
-
-		/*
-		 * If we at least started campaign, restart wp to get elected in the
-		 * new mconf. Note: in principle once wp is already elected
-		 * re-election is not required, but being conservative here is not
-		 * bad.
-		 *
-		 * TODO: put mconf to shmem to immediately pick it up on start,
-		 * otherwise if some safekeeper(s) misses latest mconf and gets
-		 * connected the first, it may cause redundant restarts here.
-		 *
-		 * More generally, it would be nice to restart walproposer (wiping
-		 * election state) without restarting the process. In particular, that
-		 * would allow sync-safekeepers not to die here if it intersected with
-		 * sk migration (as well as remove 1s delay).
-		 *
-		 * Note that assign_neon_safekeepers also currently restarts the
-		 * process, so during normal migration walproposer may restart twice.
-		 */
-		if (wp->state >= WPS_CAMPAIGN)
-		{
-			wp_log(FATAL, "restarting to adopt mconf generation %d", sk->greetResponse.mconf.generation);
-		}
 		MembershipConfigurationFree(&wp->mconf);
 		MembershipConfigurationCopy(&sk->greetResponse.mconf, &wp->mconf);
-		ResetMemberSafekeeperPtrs(wp);
 		/* full conf was just logged above */
 		wp_log(LOG, "changed mconf to generation %u", wp->mconf.generation);
 	}
@@ -963,9 +767,6 @@ RecvAcceptorGreeting(Safekeeper *sk)
 	/* Protocol is all good, move to voting. */
 	sk->state = SS_WAIT_VOTING;
 
-	/* In greeting safekeeper sent its id; update mappings accordingly. */
-	UpdateMemberSafekeeperPtr(wp, sk);
-
 	/*
 	 * Note: it would be better to track the counter on per safekeeper basis,
 	 * but at worst walproposer would restart with 'term rejected', so leave
@@ -977,9 +778,12 @@ RecvAcceptorGreeting(Safekeeper *sk)
 		/* We're still collecting terms from the majority. */
 		wp->propTerm = Max(sk->greetResponse.term, wp->propTerm);
 
-		/* Quorum is acquired, prepare the vote request. */
+		/* Quorum is acquried, prepare the vote request. */
 		if (TermsCollected(wp))
 		{
+			wp->propTerm++;
+			wp_log(LOG, "proposer connected to quorum (%d) safekeepers, propTerm=" INT64_FORMAT, wp->quorum, wp->propTerm);
+
 			wp->state = WPS_CAMPAIGN;
 			wp->voteRequest.pam.tag = 'v';
 			wp->voteRequest.generation = wp->mconf.generation;
@@ -1028,8 +832,8 @@ SendVoteRequest(Safekeeper *sk)
 					   &sk->outbuf, wp->config->proto_version);
 
 	/* We have quorum for voting, send our vote request */
-	wp_log(LOG, "requesting vote from sk {id = %lu, ep = %s:%s} for generation %u term " UINT64_FORMAT,
-		   sk->greetResponse.nodeId, sk->host, sk->port, wp->voteRequest.generation, wp->voteRequest.term);
+	wp_log(LOG, "requesting vote from %s:%s for generation %u term " UINT64_FORMAT, sk->host, sk->port,
+		   wp->voteRequest.generation, wp->voteRequest.term);
 	/* On failure, logging & resetting is handled */
 	BlockingWrite(sk, sk->outbuf.data, sk->outbuf.len, SS_WAIT_VERDICT);
 	/* If successful, wait for read-ready with SS_WAIT_VERDICT */
@@ -1047,8 +851,8 @@ RecvVoteResponse(Safekeeper *sk)
 		return;
 
 	wp_log(LOG,
-		   "got VoteResponse from sk {id = %lu, ep = %s:%s}, generation=%u, term=%lu, voteGiven=%u, last_log_term=" UINT64_FORMAT ", flushLsn=%X/%X, truncateLsn=%X/%X",
-		   sk->greetResponse.nodeId, sk->host, sk->port, sk->voteResponse.generation, sk->voteResponse.term,
+		   "got VoteResponse from acceptor %s:%s, generation=%u, term=%lu, voteGiven=%u, last_log_term=" UINT64_FORMAT ", flushLsn=%X/%X, truncateLsn=%X/%X",
+		   sk->host, sk->port, sk->voteResponse.generation, sk->voteResponse.term,
 		   sk->voteResponse.voteGiven,
 		   GetHighestTerm(&sk->voteResponse.termHistory),
 		   LSN_FORMAT_ARGS(sk->voteResponse.flushLsn),
@@ -1095,53 +899,6 @@ RecvVoteResponse(Safekeeper *sk)
 	}
 }
 
-/*
- * VotesCollected helper for a single member set `mset`.
- *
- * `msk` is the member -> safekeeper mapping for mset, i.e. members_safekeepers
- * or new_members_safekeepers.
- */
-static bool
-VotesCollectedMset(WalProposer *wp, MemberSet *mset, Safekeeper **msk, StringInfo s)
-{
-	uint32		n_votes = 0;
-
-	for (uint32 i = 0; i < wp->mconf.members.len; i++)
-	{
-		Safekeeper *sk = msk[i];
-
-		if (sk != NULL && sk->state == SS_WAIT_ELECTED)
-		{
-			Assert(sk->voteResponse.voteGiven);
-
-			/*
-			 * Find the highest vote. NULL check is for the legacy case where
-			 * safekeeper might be not initialized with LSN at all and return
-			 * 0 LSN in the vote response; we still want to set donor to
-			 * something in this case.
-			 */
-			if (GetLastLogTerm(sk) > wp->donorLastLogTerm ||
-				(GetLastLogTerm(sk) == wp->donorLastLogTerm &&
-				 sk->voteResponse.flushLsn > wp->propTermStartLsn) ||
-				wp->donor == NULL)
-			{
-				wp->donorLastLogTerm = GetLastLogTerm(sk);
-				wp->propTermStartLsn = sk->voteResponse.flushLsn;
-				wp->donor = sk;
-			}
-			wp->truncateLsn = Max(wp->safekeeper[i].voteResponse.truncateLsn, wp->truncateLsn);
-
-			if (n_votes > 0)
-				appendStringInfoString(s, ", ");
-			appendStringInfo(s, "{id = %lu, ep = %s:%s}", sk->greetResponse.nodeId, sk->host, sk->port);
-			n_votes++;
-		}
-	}
-	appendStringInfo(s, ", %u/%u total", n_votes, mset->len);
-	return MsetHasQuorum(mset, n_votes);
-}
-
-
 /*
  * Checks if enough votes has been collected to get elected and if that's the
  * case finds the highest vote, setting donor, donorLastLogTerm,
@@ -1150,8 +907,7 @@ VotesCollectedMset(WalProposer *wp, MemberSet *mset, Safekeeper **msk, StringInf
 static bool
 VotesCollected(WalProposer *wp)
 {
-	StringInfoData s;			/* str for logging */
-	bool		collected = false;
+	int			n_ready = 0;
 
 	/* assumed to be called only when not elected yet */
 	Assert(wp->state == WPS_CAMPAIGN);
@@ -1160,62 +916,25 @@ VotesCollected(WalProposer *wp)
 	wp->donorLastLogTerm = 0;
 	wp->truncateLsn = InvalidXLogRecPtr;
 
-	/* legacy: generations disabled */
-	if (!WalProposerGenerationsEnabled(wp) && wp->mconf.generation == INVALID_GENERATION)
+	for (int i = 0; i < wp->n_safekeepers; i++)
 	{
-		int			n_ready = 0;
-
-		for (int i = 0; i < wp->n_safekeepers; i++)
+		if (wp->safekeeper[i].state == SS_WAIT_ELECTED)
 		{
-			if (wp->safekeeper[i].state == SS_WAIT_ELECTED)
+			n_ready++;
+
+			if (GetLastLogTerm(&wp->safekeeper[i]) > wp->donorLastLogTerm ||
+				(GetLastLogTerm(&wp->safekeeper[i]) == wp->donorLastLogTerm &&
+				 wp->safekeeper[i].voteResponse.flushLsn > wp->propTermStartLsn))
 			{
-				n_ready++;
-
-				if (GetLastLogTerm(&wp->safekeeper[i]) > wp->donorLastLogTerm ||
-					(GetLastLogTerm(&wp->safekeeper[i]) == wp->donorLastLogTerm &&
-					 wp->safekeeper[i].voteResponse.flushLsn > wp->propTermStartLsn) ||
-					wp->donor == NULL)
-				{
-					wp->donorLastLogTerm = GetLastLogTerm(&wp->safekeeper[i]);
-					wp->propTermStartLsn = wp->safekeeper[i].voteResponse.flushLsn;
-					wp->donor = &wp->safekeeper[i];
-				}
-				wp->truncateLsn = Max(wp->safekeeper[i].voteResponse.truncateLsn, wp->truncateLsn);
+				wp->donorLastLogTerm = GetLastLogTerm(&wp->safekeeper[i]);
+				wp->propTermStartLsn = wp->safekeeper[i].voteResponse.flushLsn;
+				wp->donor = i;
 			}
+			wp->truncateLsn = Max(wp->safekeeper[i].voteResponse.truncateLsn, wp->truncateLsn);
 		}
-		collected = n_ready >= wp->quorum;
-		if (collected)
-		{
-			wp_log(LOG, "walproposer elected with %d/%d votes", n_ready, wp->n_safekeepers);
-		}
-		return collected;
 	}
 
-	/*
-	 * if generations are enabled we're expected to get to voting only when
-	 * mconf is established.
-	 */
-	Assert(wp->mconf.generation != INVALID_GENERATION);
-
-	/*
-	 * We must get votes from both msets if both are present.
-	 */
-	initStringInfo(&s);
-	appendStringInfoString(&s, "mset voters: ");
-	if (!VotesCollectedMset(wp, &wp->mconf.members, wp->members_safekeepers, &s))
-		goto res;
-	if (wp->mconf.new_members.len > 0)
-	{
-		appendStringInfoString(&s, ", new_mset voters: ");
-		if (!VotesCollectedMset(wp, &wp->mconf.new_members, wp->new_members_safekeepers, &s))
-			goto res;
-	}
-	wp_log(LOG, "walproposer elected, %s", s.data);
-	collected = true;
-
-res:
-	pfree(s.data);
-	return collected;
+	return n_ready >= wp->quorum;
 }
 
 /*
@@ -1236,7 +955,7 @@ HandleElectedProposer(WalProposer *wp)
 	 * that only for logical replication (and switching logical walsenders to
 	 * neon_walreader is a todo.)
 	 */
-	if (!wp->api.recovery_download(wp, wp->donor))
+	if (!wp->api.recovery_download(wp, &wp->safekeeper[wp->donor]))
 	{
 		wp_log(FATAL, "failed to download WAL for logical replicaiton");
 	}
@@ -1359,7 +1078,7 @@ ProcessPropStartPos(WalProposer *wp)
 	/*
 	 * Proposer's term history is the donor's + its own entry.
 	 */
-	dth = &wp->donor->voteResponse.termHistory;
+	dth = &wp->safekeeper[wp->donor].voteResponse.termHistory;
 	wp->propTermHistory.n_entries = dth->n_entries + 1;
 	wp->propTermHistory.entries = palloc(sizeof(TermSwitchEntry) * wp->propTermHistory.n_entries);
 	if (dth->n_entries > 0)
@@ -1367,10 +1086,11 @@ ProcessPropStartPos(WalProposer *wp)
 	wp->propTermHistory.entries[wp->propTermHistory.n_entries - 1].term = wp->propTerm;
 	wp->propTermHistory.entries[wp->propTermHistory.n_entries - 1].lsn = wp->propTermStartLsn;
 
-	wp_log(LOG, "walproposer elected in term " UINT64_FORMAT ", epochStartLsn %X/%X, donor %s:%s, truncate_lsn %X/%X",
+	wp_log(LOG, "got votes from majority (%d) of nodes, term " UINT64_FORMAT ", epochStartLsn %X/%X, donor %s:%s, truncate_lsn %X/%X",
+		   wp->quorum,
 		   wp->propTerm,
 		   LSN_FORMAT_ARGS(wp->propTermStartLsn),
-		   wp->donor->host, wp->donor->port,
+		   wp->safekeeper[wp->donor].host, wp->safekeeper[wp->donor].port,
 		   LSN_FORMAT_ARGS(wp->truncateLsn));
 
 	/*
@@ -1788,14 +1508,6 @@ RecvAppendResponses(Safekeeper *sk)
 
 		readAnything = true;
 
-		/* should never happen: sk is expected to send ERROR instead */
-		if (sk->appendResponse.generation != wp->mconf.generation)
-		{
-			wp_log(FATAL, "safekeeper {id = %lu, ep = %s:%s} sent response with generation %u, expected %u",
-				   sk->greetResponse.nodeId, sk->host, sk->port,
-				   sk->appendResponse.generation, wp->mconf.generation);
-		}
-
 		if (sk->appendResponse.term > wp->propTerm)
 		{
 			/*
@@ -1912,101 +1624,30 @@ CalculateMinFlushLsn(WalProposer *wp)
 }
 
 /*
- * GetAcknowledgedByQuorumWALPosition for a single member set `mset`.
- *
- * `msk` is the member -> safekeeper mapping for mset, i.e. members_safekeepers
- * or new_members_safekeepers.
- */
-static XLogRecPtr
-GetCommittedMset(WalProposer *wp, MemberSet *mset, Safekeeper **msk)
-{
-	XLogRecPtr	responses[MAX_SAFEKEEPERS];
-
-	/*
-	 * Ascending sort acknowledged LSNs.
-	 */
-	Assert(mset->len <= MAX_SAFEKEEPERS);
-	for (uint32 i = 0; i < mset->len; i++)
-	{
-		Safekeeper *sk = msk[i];
-
-		/*
-		 * Like in Raft, we aren't allowed to commit entries from previous
-		 * terms, so ignore reported LSN until it gets to propTermStartLsn.
-		 *
-		 * Note: we ignore sk state, which is ok: before first ack flushLsn is
-		 * 0, and later we just preserve value across reconnections. It would
-		 * be ok to check for SS_ACTIVE as well.
-		 */
-		if (sk != NULL && sk->appendResponse.flushLsn >= wp->propTermStartLsn)
-		{
-			responses[i] = sk->appendResponse.flushLsn;
-		}
-		else
-		{
-			responses[i] = 0;
-		}
-	}
-	qsort(responses, mset->len, sizeof(XLogRecPtr), CompareLsn);
-
-	/*
-	 * And get value committed by the quorum. A way to view this: to get the
-	 * highest value committed on the quorum, in the ordered array we skip n -
-	 * n_quorum elements to get to the first (lowest) value present on all sks
-	 * of the highest quorum.
-	 */
-	return responses[mset->len - MsetQuorum(mset)];
-}
-
-/*
- * Calculate WAL position acknowledged by quorum, i.e. which may be regarded
- * committed.
- *
- * Zero may be returned when there is no quorum of nodes recovered to term start
- * lsn which sent feedback yet.
+ * Calculate WAL position acknowledged by quorum
  */
 static XLogRecPtr
 GetAcknowledgedByQuorumWALPosition(WalProposer *wp)
 {
-	XLogRecPtr	committed;
+	XLogRecPtr	responses[MAX_SAFEKEEPERS];
 
-	/* legacy: generations disabled */
-	if (!WalProposerGenerationsEnabled(wp) && wp->mconf.generation == INVALID_GENERATION)
+	/*
+	 * Sort acknowledged LSNs
+	 */
+	for (int i = 0; i < wp->n_safekeepers; i++)
 	{
-		XLogRecPtr	responses[MAX_SAFEKEEPERS];
-
 		/*
-		 * Sort acknowledged LSNs
+		 * Like in Raft, we aren't allowed to commit entries from previous
+		 * terms, so ignore reported LSN until it gets to epochStartLsn.
 		 */
-		for (int i = 0; i < wp->n_safekeepers; i++)
-		{
-			/*
-			 * Like in Raft, we aren't allowed to commit entries from previous
-			 * terms, so ignore reported LSN until it gets to
-			 * propTermStartLsn.
-			 *
-			 * Note: we ignore sk state, which is ok: before first ack
-			 * flushLsn is 0, and later we just preserve value across
-			 * reconnections. It would be ok to check for SS_ACTIVE as well.
-			 */
-			responses[i] = wp->safekeeper[i].appendResponse.flushLsn >= wp->propTermStartLsn ? wp->safekeeper[i].appendResponse.flushLsn : 0;
-		}
-		qsort(responses, wp->n_safekeepers, sizeof(XLogRecPtr), CompareLsn);
-
-		/*
-		 * Get the smallest LSN committed by quorum
-		 */
-		return responses[wp->n_safekeepers - wp->quorum];
+		responses[i] = wp->safekeeper[i].appendResponse.flushLsn >= wp->propTermStartLsn ? wp->safekeeper[i].appendResponse.flushLsn : 0;
 	}
+	qsort(responses, wp->n_safekeepers, sizeof(XLogRecPtr), CompareLsn);
 
-	committed = GetCommittedMset(wp, &wp->mconf.members, wp->members_safekeepers);
-	if (wp->mconf.new_members.len > 0)
-	{
-		XLogRecPtr	new_mset_committed = GetCommittedMset(wp, &wp->mconf.new_members, wp->new_members_safekeepers);
-
-		committed = Min(committed, new_mset_committed);
-	}
-	return committed;
+	/*
+	 * Get the smallest LSN committed by quorum
+	 */
+	return responses[wp->n_safekeepers - wp->quorum];
 }
 
 /*
@@ -2021,7 +1662,7 @@ UpdateDonorShmem(WalProposer *wp)
 	int			i;
 	XLogRecPtr	donor_lsn = InvalidXLogRecPtr;
 
-	if (wp->state < WPS_ELECTED)
+	if (wp->n_votes < wp->quorum)
 	{
 		wp_log(WARNING, "UpdateDonorShmem called before elections are won");
 		return;
@@ -2032,9 +1673,9 @@ UpdateDonorShmem(WalProposer *wp)
 	 * about its position immediately after election before any feedbacks are
 	 * sent.
 	 */
-	if (wp->donor->state >= SS_WAIT_ELECTED)
+	if (wp->safekeeper[wp->donor].state >= SS_WAIT_ELECTED)
 	{
-		donor = wp->donor;
+		donor = &wp->safekeeper[wp->donor];
 		donor_lsn = wp->propTermStartLsn;
 	}
 
@@ -2105,13 +1746,13 @@ HandleSafekeeperResponse(WalProposer *wp, Safekeeper *fromsk)
 	}
 
 	/*
-	 * Generally sync is done when majority reached propTermStartLsn so we
-	 * committed it and made the majority aware of it, ensuring they are ready
-	 * to give all WAL to pageserver. It would mean whichever majority is
-	 * alive, there will be at least one safekeeper who is able to stream WAL
-	 * to pageserver to make basebackup possible. However, since at the moment
-	 * we don't have any good mechanism of defining the healthy and most
-	 * advanced safekeeper who should push the wal into pageserver and
+	 * Generally sync is done when majority switched the epoch so we committed
+	 * epochStartLsn and made the majority aware of it, ensuring they are
+	 * ready to give all WAL to pageserver. It would mean whichever majority
+	 * is alive, there will be at least one safekeeper who is able to stream
+	 * WAL to pageserver to make basebackup possible. However, since at the
+	 * moment we don't have any good mechanism of defining the healthy and
+	 * most advanced safekeeper who should push the wal into pageserver and
 	 * basically the random one gets connected, to prevent hanging basebackup
 	 * (due to pageserver connecting to not-synced-safekeeper) we currently
 	 * wait for all seemingly alive safekeepers to get synced.
@@ -2133,7 +1774,7 @@ HandleSafekeeperResponse(WalProposer *wp, Safekeeper *fromsk)
 				n_synced++;
 		}
 
-		if (newCommitLsn >= wp->propTermStartLsn)
+		if (n_synced >= wp->quorum)
 		{
 			/* A quorum of safekeepers has been synced! */
 

pgxn/neon/walproposer.h

@@ -145,7 +145,6 @@ typedef uint64 NNodeId;
  * This and following structs pair ones in membership.rs.
  */
 typedef uint32 Generation;
-#define INVALID_GENERATION 0
 
 typedef struct SafekeeperId
 {
@@ -772,17 +771,7 @@ typedef struct WalProposer
 	/* Current walproposer membership configuration */
 	MembershipConfiguration mconf;
 
-	/*
-	 * Parallels mconf.members with pointers to the member's slot in
-	 * safekeepers array of connections, or NULL if such member is not
-	 * connected. Helps to avoid looking slot per id through all
-	 * .safekeepers[] when doing quorum checks.
-	 */
-	Safekeeper *members_safekeepers[MAX_SAFEKEEPERS];
-	/* As above, but for new_members. */
-	Safekeeper *new_members_safekeepers[MAX_SAFEKEEPERS];
-
-	/* (n_safekeepers / 2) + 1. Used for static pre-generations quorum checks. */
+	/* (n_safekeepers / 2) + 1 */
 	int			quorum;
 
 	/*
@@ -840,7 +829,7 @@ typedef struct WalProposer
 	term_t		donorLastLogTerm;
 
 	/* Most advanced acceptor */
-	Safekeeper *donor;
+	int			donor;
 
 	/* timeline globally starts at this LSN */
 	XLogRecPtr	timelineStartLsn;

safekeeper/src/bin/safekeeper.rs

@@ -226,6 +226,9 @@ struct Args {
     /// Path to the JWT auth token used to authenticate with other safekeepers.
     #[arg(long)]
     auth_token_path: Option<Utf8PathBuf>,
+
+    #[arg(long, help = "Run in development mode (disables security checks)")]
+    dev: bool,
 }
 
 // Like PathBufValueParser, but allows empty string.
@@ -343,6 +346,21 @@ async fn main() -> anyhow::Result<()> {
         }
     };
 
+    if !args.dev {
+        let http_auth_enabled = args.http_auth_public_key_path.is_some();
+        let pg_auth_enabled = args.pg_auth_public_key_path.is_some();
+        let pg_tenant_only_auth_enabled = args.pg_tenant_only_auth_public_key_path.is_some();
+        if !http_auth_enabled || !pg_auth_enabled || !pg_tenant_only_auth_enabled {
+            bail!(
+                "Safekeeper refuses to start with HTTP, PostgreSQL, or tenant-only PostgreSQL API authentication disabled.\n\
+                  Run with --dev to allow running without authentication.\n\
+                  This is insecure and should only be used in development environments."
+            );
+        }
+    } else {
+        warn!("Starting in dev mode: this may be an insecure configuration.");
+    }
+
     // Load JWT auth token to connect to other safekeepers for pull_timeline.
     // First check if the env var is present, then check the arg with the path.
     // We want to deprecate and remove the env var method in the future.

safekeeper/src/http/mod.rs

@@ -31,7 +31,6 @@ pub async fn task_main_https(
     global_timelines: Arc<GlobalTimelines>,
 ) -> anyhow::Result<()> {
     let cert_resolver = ReloadingCertificateResolver::new(
-        "main",
         &conf.ssl_key_file,
         &conf.ssl_cert_file,
         conf.ssl_cert_reload_period,

storage_controller/src/http.rs

@@ -1235,18 +1235,8 @@ async fn handle_step_down(req: Request<Body>) -> Result<Response<Body>, ApiError
         ForwardOutcome::NotForwarded(req) => req,
     };
 
-    // Spawn a background task: once we start stepping down, we must finish: if the client drops
-    // their request we should avoid stopping in some part-stepped-down state.
-    let handle = tokio::spawn(async move {
-        let state = get_state(&req);
-        state.service.step_down().await
-    });
-
-    let result = handle
-        .await
-        .map_err(|e| ApiError::InternalServerError(e.into()))?;
-
-    json_response(StatusCode::OK, result)
+    let state = get_state(&req);
+    json_response(StatusCode::OK, state.service.step_down().await)
 }
 
 async fn handle_tenant_drop(req: Request<Body>) -> Result<Response<Body>, ApiError> {

storage_controller/src/main.rs

@@ -472,7 +472,6 @@ async fn async_main() -> anyhow::Result<()> {
             let https_listener = tcp_listener::bind(https_addr)?;
 
             let resolver = ReloadingCertificateResolver::new(
-                "main",
                 &args.ssl_key_file,
                 &args.ssl_cert_file,
                 *args.ssl_cert_reload_period,

storage_controller/src/service.rs

@@ -61,7 +61,7 @@ use utils::completion::Barrier;
 use utils::generation::Generation;
 use utils::id::{NodeId, TenantId, TimelineId};
 use utils::lsn::Lsn;
-use utils::sync::gate::{Gate, GateGuard};
+use utils::sync::gate::Gate;
 use utils::{failpoint_support, pausable_failpoint};
 
 use crate::background_node_operations::{
@@ -594,8 +594,6 @@ struct TenantShardSplitAbort {
     new_stripe_size: Option<ShardStripeSize>,
     /// Until this abort op is complete, no other operations may be done on the tenant
     _tenant_lock: TracingExclusiveGuard<TenantOperations>,
-    /// The reconciler gate for the duration of the split operation, and any included abort.
-    _gate: GateGuard,
 }
 
 #[derive(thiserror::Error, Debug)]
@@ -1462,7 +1460,7 @@ impl Service {
             // Retry until shutdown: we must keep this request object alive until it is properly
             // processed, as it holds a lock guard that prevents other operations trying to do things
             // to the tenant while it is in a weird part-split state.
-            while !self.reconcilers_cancel.is_cancelled() {
+            while !self.cancel.is_cancelled() {
                 match self.abort_tenant_shard_split(&op).await {
                     Ok(_) => break,
                     Err(e) => {
@@ -1475,12 +1473,9 @@ impl Service {
                         // when we retry, so that the abort op will succeed.  If the abort op is failing
                         // for some other reason, we will keep retrying forever, or until a human notices
                         // and does something about it (either fixing a pageserver or restarting the controller).
-                        tokio::time::timeout(
-                            Duration::from_secs(5),
-                            self.reconcilers_cancel.cancelled(),
-                        )
-                        .await
-                        .ok();
+                        tokio::time::timeout(Duration::from_secs(5), self.cancel.cancelled())
+                            .await
+                            .ok();
                     }
                 }
             }
@@ -4915,7 +4910,7 @@ impl Service {
                     1,
                     10,
                     Duration::from_secs(5),
-                    &self.reconcilers_cancel,
+                    &self.cancel,
                 )
                 .await
             {
@@ -5166,11 +5161,6 @@ impl Service {
         )
         .await;
 
-        let _gate = self
-            .reconcilers_gate
-            .enter()
-            .map_err(|_| ApiError::ShuttingDown)?;
-
         let new_shard_count = ShardCount::new(split_req.new_shard_count);
         let new_stripe_size = split_req.new_stripe_size;
 
@@ -5198,7 +5188,6 @@ impl Service {
                         new_shard_count,
                         new_stripe_size,
                         _tenant_lock,
-                        _gate,
                     })
                     // Ignore error sending: that just means we're shutting down: aborts are ephemeral so it's fine to drop it.
                     .ok();
@@ -5538,10 +5527,7 @@ impl Service {
                 "failpoint".to_string()
             )));
 
-            failpoint_support::sleep_millis_async!(
-                "shard-split-post-remote-sleep",
-                &self.reconcilers_cancel
-            );
+            failpoint_support::sleep_millis_async!("shard-split-post-remote-sleep", &self.cancel);
 
             tracing::info!(
                 "Split {} into {}",
@@ -5599,7 +5585,7 @@ impl Service {
                         stripe_size,
                         preferred_az: preferred_az_id.as_ref().map(Cow::Borrowed),
                     },
-                    &self.reconcilers_cancel,
+                    &self.cancel,
                 )
                 .await
             {
@@ -8684,24 +8670,9 @@ impl Service {
         failpoint_support::sleep_millis_async!("sleep-on-step-down-handling");
 
         self.inner.write().unwrap().step_down();
-
-        // Wait for reconciliations to stop, or terminate this process if they
-        // fail to stop in time (this indicates a bug in shutdown)
-        tokio::select! {
-            _ = self.stop_reconciliations(StopReconciliationsReason::SteppingDown) => {
-                tracing::info!("Reconciliations stopped, proceeding with step down");
-            }
-            _ = async {
-                failpoint_support::sleep_millis_async!("step-down-delay-timeout");
-                tokio::time::sleep(Duration::from_secs(10)).await
-            } => {
-                tracing::warn!("Step down timed out while waiting for reconciliation gate, terminating process");
-
-                // The caller may proceed to act as leader when it sees this request fail: reduce the chance
-                // of a split-brain situation by terminating this controller instead of leaving it up in a partially-shut-down state.
-                std::process::exit(1);
-            }
-        }
+        // TODO: would it make sense to have a time-out for this?
+        self.stop_reconciliations(StopReconciliationsReason::SteppingDown)
+            .await;
 
         let mut global_observed = GlobalObservedState::default();
         let locked = self.inner.read().unwrap();

test_runner/fixtures/neon_fixtures.py

@@ -14,7 +14,6 @@ import threading
 import time
 import uuid
 from collections import defaultdict
-from collections.abc import Mapping
 from contextlib import closing, contextmanager
 from dataclasses import dataclass
 from datetime import datetime
@@ -80,12 +79,7 @@ from fixtures.remote_storage import (
     default_remote_storage,
     remote_storage_to_toml_dict,
 )
-from fixtures.safekeeper.http import (
-    MembershipConfiguration,
-    SafekeeperHttpClient,
-    SafekeeperId,
-    TimelineCreateRequest,
-)
+from fixtures.safekeeper.http import SafekeeperHttpClient
 from fixtures.safekeeper.utils import wait_walreceivers_absent
 from fixtures.utils import (
     ATTACHMENT_NAME_REGEX,
@@ -4298,32 +4292,31 @@ class Endpoint(PgProtocol, LogUtils):
     def respec_deep(self, **kwargs: Any) -> None:
         """
         Update the endpoint.json file taking into account nested keys.
+        It does one level deep update. Should enough for most cases.
         Distinct method from respec() to do not break existing functionality.
-        NOTE: This method also updates the config.json file, not endpoint.json.
-        We need it because neon_local also writes to config.json, so intended
+        NOTE: This method also updates the spec.json file, not endpoint.json.
+        We need it because neon_local also writes to spec.json, so intended
         use-case is i) start endpoint with some config, ii) respec_deep(),
         iii) call reconfigure() to apply the changes.
         """
-
-        def update(curr, patch):
-            for k, v in patch.items():
-                if isinstance(v, Mapping):
-                    curr[k] = update(curr.get(k, {}), v)
-                else:
-                    curr[k] = v
-            return curr
-
-        config_path = os.path.join(self.endpoint_path(), "config.json")
+        config_path = os.path.join(self.endpoint_path(), "spec.json")
         with open(config_path) as f:
-            config: dict[str, Any] = json.load(f)
+            data_dict: dict[str, Any] = json.load(f)
 
-        log.debug("Current compute config: %s", json.dumps(config, indent=4))
+        log.debug("Current compute spec: %s", json.dumps(data_dict, indent=4))
 
-        update(config, kwargs)
+        for key, value in kwargs.items():
+            if isinstance(value, dict):
+                if key not in data_dict:
+                    data_dict[key] = value
+                else:
+                    data_dict[key] = {**data_dict[key], **value}
+            else:
+                data_dict[key] = value
 
         with open(config_path, "w") as file:
-            log.debug("Updating compute config to: %s", json.dumps(config, indent=4))
-            json.dump(config, file, indent=4)
+            log.debug("Updating compute spec to: %s", json.dumps(data_dict, indent=4))
+            json.dump(data_dict, file, indent=4)
 
     def wait_for_migrations(self, wait_for: int = NUM_COMPUTE_MIGRATIONS) -> None:
         """
@@ -4340,7 +4333,7 @@ class Endpoint(PgProtocol, LogUtils):
             wait_until(check_migrations_done)
 
     # Mock the extension part of spec passed from control plane for local testing
-    # endpooint.rs adds content of this file as a part of the config.json
+    # endpooint.rs adds content of this file as a part of the spec.json
     def create_remote_extension_spec(self, spec: dict[str, Any]):
         """Create a remote extension spec file for the endpoint."""
         remote_extensions_spec_path = os.path.join(
@@ -4846,50 +4839,6 @@ class Safekeeper(LogUtils):
 
         wait_until(paused)
 
-    @staticmethod
-    def sks_to_safekeeper_ids(sks: list[Safekeeper]) -> list[SafekeeperId]:
-        return [SafekeeperId(sk.id, "localhost", sk.port.pg_tenant_only) for sk in sks]
-
-    @staticmethod
-    def mconf_sks(env: NeonEnv, mconf: MembershipConfiguration) -> list[Safekeeper]:
-        """
-        List of Safekeepers which are members in `mconf`.
-        """
-        members_ids = [m.id for m in mconf.members]
-        new_members_ids = [m.id for m in mconf.new_members] if mconf.new_members is not None else []
-        return [sk for sk in env.safekeepers if sk.id in members_ids or sk.id in new_members_ids]
-
-    @staticmethod
-    def create_timeline(
-        tenant_id: TenantId,
-        timeline_id: TimelineId,
-        ps: NeonPageserver,
-        mconf: MembershipConfiguration,
-        members_sks: list[Safekeeper],
-    ):
-        """
-        Manually create timeline on safekeepers with given (presumably inital)
-        mconf: figure out LSN from pageserver, bake request and execute it on
-        given safekeepers.
-
-        Normally done by storcon, but some tests want to do it manually so far.
-        """
-        ps_http_cli = ps.http_client()
-        # figure out initial LSN.
-        ps_timeline_detail = ps_http_cli.timeline_detail(tenant_id, timeline_id)
-        init_lsn = ps_timeline_detail["last_record_lsn"]
-        log.info(f"initial LSN: {init_lsn}")
-        # sk timeline creation request expects minor version
-        pg_version = ps_timeline_detail["pg_version"] * 10000
-        # create inital mconf
-        create_r = TimelineCreateRequest(
-            tenant_id, timeline_id, mconf, pg_version, Lsn(init_lsn), commit_lsn=None
-        )
-        log.info(f"sending timeline create: {create_r.to_json()}")
-
-        for sk in members_sks:
-            sk.http_client().timeline_create(create_r)
-
 
 class NeonBroker(LogUtils):
     """An object managing storage_broker instance"""

test_runner/fixtures/pageserver/allowed_errors.py

@@ -126,6 +126,8 @@ DEFAULT_STORAGE_CONTROLLER_ALLOWED_ERRORS = [
     ".*startup_reconcile: Could not scan node.*",
     # Tests run in dev mode
     ".*Starting in dev mode.*",
+    ".*Starting in dev mode - authentication security checks are disabled.*",
+    ".*Starting in dev mode: this may be an insecure configuration.*",
     # Tests that stop endpoints & use the storage controller's neon_local notification
     # mechanism might fail (neon_local's stopping and endpoint isn't atomic wrt the storage
     # controller's attempts to notify the endpoint).

test_runner/fixtures/safekeeper/http.py

@@ -25,7 +25,7 @@ class Walreceiver:
 
 @dataclass
 class SafekeeperTimelineStatus:
-    mconf: MembershipConfiguration | None
+    mconf: Configuration | None
     term: int
     last_log_term: int
     pg_version: int  # Not exactly a PgVersion, safekeeper returns version as int, for example 150002 for 15.2
@@ -78,17 +78,17 @@ class SafekeeperId:
 
 
 @dataclass
-class MembershipConfiguration:
+class Configuration:
     generation: int
     members: list[SafekeeperId]
     new_members: list[SafekeeperId] | None
 
     @classmethod
-    def from_json(cls, d: dict[str, Any]) -> MembershipConfiguration:
+    def from_json(cls, d: dict[str, Any]) -> Configuration:
         generation = d["generation"]
         members = d["members"]
         new_members = d.get("new_members")
-        return MembershipConfiguration(generation, members, new_members)
+        return Configuration(generation, members, new_members)
 
     def to_json(self) -> str:
         return json.dumps(self, cls=EnhancedJSONEncoder)
@@ -98,7 +98,7 @@ class MembershipConfiguration:
 class TimelineCreateRequest:
     tenant_id: TenantId
     timeline_id: TimelineId
-    mconf: MembershipConfiguration
+    mconf: Configuration
     # not exactly PgVersion, for example 150002 for 15.2
     pg_version: int
     start_lsn: Lsn
@@ -110,13 +110,13 @@ class TimelineCreateRequest:
 
 @dataclass
 class TimelineMembershipSwitchResponse:
-    previous_conf: MembershipConfiguration
-    current_conf: MembershipConfiguration
+    previous_conf: Configuration
+    current_conf: Configuration
 
     @classmethod
     def from_json(cls, d: dict[str, Any]) -> TimelineMembershipSwitchResponse:
-        previous_conf = MembershipConfiguration.from_json(d["previous_conf"])
-        current_conf = MembershipConfiguration.from_json(d["current_conf"])
+        previous_conf = Configuration.from_json(d["previous_conf"])
+        current_conf = Configuration.from_json(d["current_conf"])
         return TimelineMembershipSwitchResponse(previous_conf, current_conf)
 
 
@@ -194,7 +194,7 @@ class SafekeeperHttpClient(requests.Session, MetricsGetter):
         resj = res.json()
         walreceivers = [Walreceiver(wr["conn_id"], wr["status"]) for wr in resj["walreceivers"]]
         # It is always normally not None, it is allowed only to make forward compat tests happy.
-        mconf = MembershipConfiguration.from_json(resj["mconf"]) if "mconf" in resj else None
+        mconf = Configuration.from_json(resj["mconf"]) if "mconf" in resj else None
         return SafekeeperTimelineStatus(
             mconf=mconf,
             term=resj["acceptor_state"]["term"],
@@ -223,9 +223,7 @@ class SafekeeperHttpClient(requests.Session, MetricsGetter):
         return self.timeline_status(tenant_id, timeline_id).commit_lsn
 
     # Get timeline membership configuration.
-    def get_membership(
-        self, tenant_id: TenantId, timeline_id: TimelineId
-    ) -> MembershipConfiguration:
+    def get_membership(self, tenant_id: TenantId, timeline_id: TimelineId) -> Configuration:
         # make mypy happy
         return self.timeline_status(tenant_id, timeline_id).mconf  # type: ignore
 
@@ -277,7 +275,7 @@ class SafekeeperHttpClient(requests.Session, MetricsGetter):
         return res_json
 
     def timeline_exclude(
-        self, tenant_id: TenantId, timeline_id: TimelineId, to: MembershipConfiguration
+        self, tenant_id: TenantId, timeline_id: TimelineId, to: Configuration
     ) -> dict[str, Any]:
         res = self.put(
             f"http://localhost:{self.port}/v1/tenant/{tenant_id}/timeline/{timeline_id}/exclude",
@@ -289,7 +287,7 @@ class SafekeeperHttpClient(requests.Session, MetricsGetter):
         return res_json
 
     def membership_switch(
-        self, tenant_id: TenantId, timeline_id: TimelineId, to: MembershipConfiguration
+        self, tenant_id: TenantId, timeline_id: TimelineId, to: Configuration
     ) -> TimelineMembershipSwitchResponse:
         res = self.put(
             f"http://localhost:{self.port}/v1/tenant/{tenant_id}/timeline/{timeline_id}/membership",

test_runner/regress/test_compute_catalog.py

@@ -90,12 +90,10 @@ def test_compute_catalog(neon_simple_env: NeonEnv):
     # and reconfigure the endpoint to create some test databases.
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "roles": TEST_ROLE_NAMES,
-                    "databases": TEST_DB_NAMES,
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "roles": TEST_ROLE_NAMES,
+                "databases": TEST_DB_NAMES,
             },
         }
     )
@@ -157,12 +155,10 @@ def test_compute_create_drop_dbs_and_roles(neon_simple_env: NeonEnv):
     # and reconfigure the endpoint to apply the changes.
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "roles": TEST_ROLE_NAMES,
-                    "databases": TEST_DB_NAMES,
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "roles": TEST_ROLE_NAMES,
+                "databases": TEST_DB_NAMES,
             },
         }
     )
@@ -200,14 +196,12 @@ def test_compute_create_drop_dbs_and_roles(neon_simple_env: NeonEnv):
 
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "roles": [],
-                    "databases": [],
-                },
-                "delta_operations": delta_operations,
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "roles": [],
+                "databases": [],
             },
+            "delta_operations": delta_operations,
         }
     )
     endpoint.reconfigure()
@@ -256,11 +250,9 @@ def test_dropdb_with_subscription(neon_simple_env: NeonEnv):
     # and reconfigure the endpoint to apply the changes.
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "databases": TEST_DB_NAMES,
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "databases": TEST_DB_NAMES,
             },
         }
     )
@@ -314,19 +306,17 @@ def test_dropdb_with_subscription(neon_simple_env: NeonEnv):
     # and reconfigure the endpoint to apply the changes.
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "databases": TEST_DB_NAMES_NEW,
-                },
-                "delta_operations": [
-                    {"action": "delete_db", "name": SUB_DB_NAME},
-                    # also test the case when we try to delete a non-existent database
-                    # shouldn't happen in normal operation,
-                    # but can occur when failed operations are retried
-                    {"action": "delete_db", "name": "nonexistent_db"},
-                ],
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "databases": TEST_DB_NAMES_NEW,
             },
+            "delta_operations": [
+                {"action": "delete_db", "name": SUB_DB_NAME},
+                # also test the case when we try to delete a non-existent database
+                # shouldn't happen in normal operation,
+                # but can occur when failed operations are retried
+                {"action": "delete_db", "name": "nonexistent_db"},
+            ],
         }
     )
 
@@ -364,27 +354,25 @@ def test_drop_role_with_table_privileges_from_neon_superuser(neon_simple_env: Ne
 
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "roles": [
-                        {
-                            # We need to create role via compute_ctl, because in this case it will receive
-                            # additional grants equivalent to our real environment, so we can repro some
-                            # issues.
-                            "name": "neon",
-                            # Some autocomplete-suggested hash, no specific meaning.
-                            "encrypted_password": "SCRAM-SHA-256$4096:hBT22QjqpydQWqEulorfXA==$miBogcoj68JWYdsNB5PW1X6PjSLBEcNuctuhtGkb4PY=:hxk2gxkwxGo6P7GCtfpMlhA9zwHvPMsCz+NQf2HfvWk=",
-                            "options": [],
-                        },
-                    ],
-                    "databases": [
-                        {
-                            "name": TEST_DB_NAME,
-                            "owner": "neon",
-                        },
-                    ],
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "roles": [
+                    {
+                        # We need to create role via compute_ctl, because in this case it will receive
+                        # additional grants equivalent to our real environment, so we can repro some
+                        # issues.
+                        "name": "neon",
+                        # Some autocomplete-suggested hash, no specific meaning.
+                        "encrypted_password": "SCRAM-SHA-256$4096:hBT22QjqpydQWqEulorfXA==$miBogcoj68JWYdsNB5PW1X6PjSLBEcNuctuhtGkb4PY=:hxk2gxkwxGo6P7GCtfpMlhA9zwHvPMsCz+NQf2HfvWk=",
+                        "options": [],
+                    },
+                ],
+                "databases": [
+                    {
+                        "name": TEST_DB_NAME,
+                        "owner": "neon",
+                    },
+                ],
             },
         }
     )
@@ -427,15 +415,13 @@ def test_drop_role_with_table_privileges_from_neon_superuser(neon_simple_env: Ne
     # Drop role via compute_ctl
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "delta_operations": [
-                    {
-                        "action": "delete_role",
-                        "name": TEST_GRANTEE,
-                    },
-                ],
-            },
+            "skip_pg_catalog_updates": False,
+            "delta_operations": [
+                {
+                    "action": "delete_role",
+                    "name": TEST_GRANTEE,
+                },
+            ],
         }
     )
     endpoint.reconfigure()
@@ -458,15 +444,13 @@ def test_drop_role_with_table_privileges_from_neon_superuser(neon_simple_env: Ne
 
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "delta_operations": [
-                    {
-                        "action": "delete_role",
-                        "name": "readonly2",
-                    },
-                ],
-            },
+            "skip_pg_catalog_updates": False,
+            "delta_operations": [
+                {
+                    "action": "delete_role",
+                    "name": "readonly2",
+                },
+            ],
         }
     )
     endpoint.reconfigure()
@@ -491,27 +475,25 @@ def test_drop_role_with_table_privileges_from_non_neon_superuser(neon_simple_env
     endpoint = env.endpoints.create_start("main")
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "roles": [
-                        {
-                            # We need to create role via compute_ctl, because in this case it will receive
-                            # additional grants equivalent to our real environment, so we can repro some
-                            # issues.
-                            "name": TEST_GRANTOR,
-                            # Some autocomplete-suggested hash, no specific meaning.
-                            "encrypted_password": "SCRAM-SHA-256$4096:hBT22QjqpydQWqEulorfXA==$miBogcoj68JWYdsNB5PW1X6PjSLBEcNuctuhtGkb4PY=:hxk2gxkwxGo6P7GCtfpMlhA9zwHvPMsCz+NQf2HfvWk=",
-                            "options": [],
-                        },
-                    ],
-                    "databases": [
-                        {
-                            "name": TEST_DB_NAME,
-                            "owner": TEST_GRANTOR,
-                        },
-                    ],
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "roles": [
+                    {
+                        # We need to create role via compute_ctl, because in this case it will receive
+                        # additional grants equivalent to our real environment, so we can repro some
+                        # issues.
+                        "name": TEST_GRANTOR,
+                        # Some autocomplete-suggested hash, no specific meaning.
+                        "encrypted_password": "SCRAM-SHA-256$4096:hBT22QjqpydQWqEulorfXA==$miBogcoj68JWYdsNB5PW1X6PjSLBEcNuctuhtGkb4PY=:hxk2gxkwxGo6P7GCtfpMlhA9zwHvPMsCz+NQf2HfvWk=",
+                        "options": [],
+                    },
+                ],
+                "databases": [
+                    {
+                        "name": TEST_DB_NAME,
+                        "owner": TEST_GRANTOR,
+                    },
+                ],
             },
         }
     )
@@ -525,15 +507,13 @@ def test_drop_role_with_table_privileges_from_non_neon_superuser(neon_simple_env
 
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "delta_operations": [
-                    {
-                        "action": "delete_role",
-                        "name": TEST_GRANTEE,
-                    },
-                ],
-            },
+            "skip_pg_catalog_updates": False,
+            "delta_operations": [
+                {
+                    "action": "delete_role",
+                    "name": TEST_GRANTEE,
+                },
+            ],
         }
     )
     endpoint.reconfigure()

test_runner/regress/test_compute_reconfigure.py

@@ -31,17 +31,15 @@ def test_compute_reconfigure(neon_simple_env: NeonEnv):
 
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": True,
-                "cluster": {
-                    "settings": [
-                        {
-                            "name": "log_line_prefix",
-                            "vartype": "string",
-                            "value": TEST_LOG_LINE_PREFIX,
-                        }
-                    ]
-                },
+            "skip_pg_catalog_updates": True,
+            "cluster": {
+                "settings": [
+                    {
+                        "name": "log_line_prefix",
+                        "vartype": "string",
+                        "value": TEST_LOG_LINE_PREFIX,
+                    }
+                ]
             },
         }
     )

test_runner/regress/test_pageserver_getpage_throttle.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import copy
 import json
 import uuid
 from typing import TYPE_CHECKING
@@ -43,6 +44,7 @@ def test_pageserver_getpage_throttle(neon_env_builder: NeonEnvBuilder, pg_bin: P
                 "refill_interval": "100ms",
                 "refill_amount": int(rate_limit_rps / 10),
                 "max": int(rate_limit_rps / 10),
+                "fair": True,
             },
         },
     )
@@ -137,3 +139,63 @@ def test_pageserver_getpage_throttle(neon_env_builder: NeonEnvBuilder, pg_bin: P
     assert pytest.approx(actual_throttled_secs + actual_smgr_query_seconds, 0.1) == duration_secs, (
         "most of the time in this test is spent throttled because the rate-limit's contribution to latency dominates"
     )
+
+
+throttle_config_with_field_fair_set = {
+    "task_kinds": ["PageRequestHandler"],
+    "fair": True,
+    "initial": 27,
+    "refill_interval": "43s",
+    "refill_amount": 23,
+    "max": 42,
+}
+
+
+def assert_throttle_config_with_field_fair_set(conf):
+    """
+    Field `fair` is ignored, so, responses don't contain it
+    """
+    without_fair = copy.deepcopy(throttle_config_with_field_fair_set)
+    without_fair.pop("fair")
+
+    assert conf == without_fair
+
+
+def test_throttle_fair_config_is_settable_but_ignored_in_mgmt_api(neon_env_builder: NeonEnvBuilder):
+    """
+    To be removed after https://github.com/neondatabase/neon/pull/8539 is rolled out.
+    """
+    env = neon_env_builder.init_start()
+    vps_http = env.storage_controller.pageserver_api()
+    # with_fair config should still be settable
+    vps_http.set_tenant_config(
+        env.initial_tenant,
+        {"timeline_get_throttle": throttle_config_with_field_fair_set},
+    )
+    conf = vps_http.tenant_config(env.initial_tenant)
+    assert_throttle_config_with_field_fair_set(conf.effective_config["timeline_get_throttle"])
+    assert_throttle_config_with_field_fair_set(
+        conf.tenant_specific_overrides["timeline_get_throttle"]
+    )
+
+
+def test_throttle_fair_config_is_settable_but_ignored_in_config_toml(
+    neon_env_builder: NeonEnvBuilder,
+):
+    """
+    To be removed after https://github.com/neondatabase/neon/pull/8539 is rolled out.
+    """
+
+    def set_tenant_config(ps_cfg):
+        tenant_config = ps_cfg.setdefault("tenant_config", {})
+        tenant_config["timeline_get_throttle"] = throttle_config_with_field_fair_set
+
+    neon_env_builder.pageserver_config_override = set_tenant_config
+    env = neon_env_builder.init_start()
+    ps_http = env.pageserver.http_client()
+    conf = ps_http.tenant_config(env.initial_tenant)
+    assert_throttle_config_with_field_fair_set(conf.effective_config["timeline_get_throttle"])
+
+    env.pageserver.allowed_errors.append(
+        r'.*ignoring unknown configuration item path="tenant_config\.timeline_get_throttle\.fair"*'
+    )

test_runner/regress/test_pageserver_secondary.py

@@ -242,13 +242,7 @@ def test_location_conf_churn(neon_env_builder: NeonEnvBuilder, make_httpserver,
             pageserver.tenant_location_configure(tenant_id, location_conf)
             last_state[pageserver.id] = (mode, generation)
 
-            # It's only valid to connect to the last generation. Newer generations may yank layer
-            # files used in older generations.
-            last_generation = max(
-                [s[1] for s in last_state.values() if s[1] is not None], default=None
-            )
-
-            if mode.startswith("Attached") and generation == last_generation:
+            if mode.startswith("Attached"):
                 # This is a basic test: we are validating that he endpoint works properly _between_
                 # configuration changes.  A stronger test would be to validate that clients see
                 # no errors while we are making the changes.

test_runner/regress/test_ssl.py

@@ -1,6 +1,5 @@
 import os
 import ssl
-from datetime import datetime, timedelta
 
 import pytest
 import requests
@@ -152,63 +151,3 @@ def test_certificate_rotation(neon_env_builder: NeonEnvBuilder):
     requests.get(addr, verify=str(env.ssl_ca_file)).raise_for_status()
     cur_cert = ssl.get_server_certificate(("localhost", port))
     assert cur_cert == sk_cert
-
-
-def test_server_and_cert_metrics(neon_env_builder: NeonEnvBuilder):
-    """
-    Test metrics exported from http/https server and tls cert reloader.
-    """
-    neon_env_builder.use_https_pageserver_api = True
-    neon_env_builder.pageserver_config_override = "ssl_cert_reload_period='100 ms'"
-    env = neon_env_builder.init_start()
-
-    env.pageserver.allowed_errors.append(".*Error reloading certificate.*")
-
-    ps_client = env.pageserver.http_client()
-
-    # 1. Test connection started metric.
-    filter_https = {"scheme": "https"}
-    old_https_conn_count = (
-        ps_client.get_metric_value("http_server_connection_started_total", filter_https) or 0
-    )
-
-    addr = f"https://localhost:{env.pageserver.service_port.https}/v1/status"
-    requests.get(addr, verify=str(env.ssl_ca_file)).raise_for_status()
-
-    new_https_conn_count = (
-        ps_client.get_metric_value("http_server_connection_started_total", filter_https) or 0
-    )
-    # The counter should increase after the request,
-    # but it may increase by more than one because of storcon requests.
-    assert new_https_conn_count > old_https_conn_count
-
-    # 2. Test tls connection error.
-    # Request without specified CA cert file should fail.
-    with pytest.raises(requests.exceptions.SSLError):
-        requests.get(addr)
-
-    tls_error_cnt = (
-        ps_client.get_metric_value("http_server_connection_errors_total", {"type": "tls"}) or 0
-    )
-    assert tls_error_cnt == 1
-
-    # 3. Test expiration time metric.
-    expiration_time = datetime.fromtimestamp(
-        ps_client.get_metric_value("tls_certs_expiration_time_seconds") or 0
-    )
-    now = datetime.now()
-    # neon_local generates certs valid for 100 years.
-    # Compare with +-1 year to not care about leap years.
-    assert now + timedelta(days=365 * 99) < expiration_time < now + timedelta(days=365 * 101)
-
-    # 4. Test cert reload failed metric.
-    reload_error_cnt = ps_client.get_metric_value("tls_certs_reload_failed_total")
-    assert reload_error_cnt == 0
-
-    os.remove(env.pageserver.workdir / "server.crt")
-
-    def reload_failed():
-        reload_error_cnt = ps_client.get_metric_value("tls_certs_reload_failed_total") or 0
-        assert reload_error_cnt > 0
-
-    wait_until(reload_failed)

test_runner/regress/test_storage_controller.py

@@ -2892,12 +2892,10 @@ def test_storage_controller_leadership_transfer(
         )
 
 
-@pytest.mark.parametrize("step_down_times_out", [False, True])
 def test_storage_controller_leadership_transfer_during_split(
     neon_env_builder: NeonEnvBuilder,
     storage_controller_proxy: StorageControllerProxy,
     port_distributor: PortDistributor,
-    step_down_times_out: bool,
 ):
     """
     Exercise a race between shard splitting and graceful leadership transfer.  This is
@@ -2938,18 +2936,6 @@ def test_storage_controller_leadership_transfer_during_split(
         )
     env.storage_controller.reconcile_until_idle()
 
-    # We are testing scenarios where the step down API does not complete: either because it is stuck
-    # doing a shard split, or because it totally times out on some other failpoint.
-    env.storage_controller.allowed_errors.extend(
-        [
-            ".*step_down.*request was dropped before completing.*",
-            ".*step_down.*operation timed out.*",
-            ".*Send step down request failed, will retry.*",
-            ".*Send step down request still failed after.*retries.*",
-            ".*Leader .+ did not respond to step-down request.*",
-        ]
-    )
-
     with concurrent.futures.ThreadPoolExecutor(max_workers=2) as executor:
         # Start a shard split
         env.storage_controller.allowed_errors.extend(
@@ -2957,14 +2943,6 @@ def test_storage_controller_leadership_transfer_during_split(
         )
         pause_failpoint = "shard-split-pre-complete"
         env.storage_controller.configure_failpoints((pause_failpoint, "pause"))
-
-        if not step_down_times_out:
-            # Prevent the timeout self-terminate code from executing: we will block step down on the
-            # shard split itself
-            env.storage_controller.configure_failpoints(
-                ("step-down-delay-timeout", "return(3600000)")
-            )
-
         split_fut = executor.submit(
             env.storage_controller.tenant_shard_split, list(tenants)[0], shard_count * 2
         )
@@ -2983,20 +2961,12 @@ def test_storage_controller_leadership_transfer_during_split(
             timeout_in_seconds=30, instance_id=2, base_port=storage_controller_2_port
         )
 
-        if step_down_times_out:
-            # Step down will time out, original controller will terminate itself
-            env.storage_controller.allowed_errors.extend([".*terminating process.*"])
-        else:
-            # Step down does not time out: original controller hits its shard split completion
-            # code path and realises that it must not purge the parent shards from the database.
-            env.storage_controller.allowed_errors.extend([".*Enqueuing background abort.*"])
-
         def passed_split_abort():
             try:
                 log.info("Checking log for pattern...")
-                # This log is indicative of entering startup_reconcile, which happens
-                # after the point we would abort shard splits
-                assert env.storage_controller.log_contains(".*Populating tenant shards.*")
+                assert env.storage_controller.log_contains(
+                    ".*Using observed state received from leader.*"
+                )
             except Exception:
                 log.exception("Failed to find pattern in log")
                 raise
@@ -3005,42 +2975,34 @@ def test_storage_controller_leadership_transfer_during_split(
         wait_until(passed_split_abort, interval=0.1, status_interval=1.0)
         assert env.storage_controller.log_contains(".*Aborting shard split.*")
 
-        if step_down_times_out:
-            # We will let the old controller hit a timeout path where it terminates itself, rather than
-            # completing step_down and trying to complete a shard split
-            def old_controller_terminated():
-                assert env.storage_controller.log_contains(".*terminating process.*")
+        # Proxy is still talking to original controller here: disable its pause failpoint so
+        # that its shard split can run to completion.
+        log.info("Disabling failpoint")
+        # Bypass the proxy: the python test HTTPServer is single threaded and still blocked
+        # on handling the shard split request.
+        env.storage_controller.request(
+            "PUT",
+            f"http://127.0.0.1:{storage_controller_1_port}/debug/v1/failpoints",
+            json=[{"name": "shard-split-pre-complete", "actions": "off"}],
+            headers=env.storage_controller.headers(TokenScope.ADMIN),
+        )
 
-            wait_until(old_controller_terminated)
-        else:
-            # Proxy is still talking to original controller here: disable its pause failpoint so
-            # that its shard split can run to completion.
-            log.info("Disabling failpoint")
-            # Bypass the proxy: the python test HTTPServer is single threaded and still blocked
-            # on handling the shard split request.
-            env.storage_controller.request(
-                "PUT",
-                f"http://127.0.0.1:{storage_controller_1_port}/debug/v1/failpoints",
-                json=[{"name": "shard-split-pre-complete", "actions": "off"}],
-                headers=env.storage_controller.headers(TokenScope.ADMIN),
+        def previous_stepped_down():
+            assert (
+                env.storage_controller.get_leadership_status()
+                == StorageControllerLeadershipStatus.STEPPED_DOWN
             )
 
-            def previous_stepped_down():
-                assert (
-                    env.storage_controller.get_leadership_status()
-                    == StorageControllerLeadershipStatus.STEPPED_DOWN
-                )
+        log.info("Awaiting step down")
+        wait_until(previous_stepped_down)
 
-            log.info("Awaiting step down")
-            wait_until(previous_stepped_down)
-
-            # Let the shard split complete: this may happen _after_ the replacement has come up
-            # and tried to clean up the databases
-            log.info("Unblocking & awaiting shard split")
-            with pytest.raises(Exception, match="Unexpected child shard count"):
-                # This split fails when it tries to persist results, because it encounters
-                # changes already made by the new controller's abort-on-startup
-                split_fut.result()
+        # Let the shard split complete: this may happen _after_ the replacement has come up
+        # and tried to clean up the databases
+        log.info("Unblocking & awaiting shard split")
+        with pytest.raises(Exception, match="Unexpected child shard count"):
+            # This split fails when it tries to persist results, because it encounters
+            # changes already made by the new controller's abort-on-startup
+            split_fut.result()
 
         log.info("Routing to new leader")
         storage_controller_proxy.route_to(f"http://127.0.0.1:{storage_controller_2_port}")
@@ -3058,14 +3020,13 @@ def test_storage_controller_leadership_transfer_during_split(
     env.storage_controller.wait_until_ready()
     env.storage_controller.consistency_check()
 
-    if not step_down_times_out:
-        # Check that the stepped down instance forwards requests
-        # to the new leader while it's still running.
-        storage_controller_proxy.route_to(f"http://127.0.0.1:{storage_controller_1_port}")
-        env.storage_controller.tenant_shard_dump()
-        env.storage_controller.node_configure(env.pageservers[0].id, {"scheduling": "Pause"})
-        status = env.storage_controller.node_status(env.pageservers[0].id)
-        assert status["scheduling"] == "Pause"
+    # Check that the stepped down instance forwards requests
+    # to the new leader while it's still running.
+    storage_controller_proxy.route_to(f"http://127.0.0.1:{storage_controller_1_port}")
+    env.storage_controller.tenant_shard_dump()
+    env.storage_controller.node_configure(env.pageservers[0].id, {"scheduling": "Pause"})
+    status = env.storage_controller.node_status(env.pageservers[0].id)
+    assert status["scheduling"] == "Pause"
 
 
 def test_storage_controller_ps_restarted_during_drain(neon_env_builder: NeonEnvBuilder):

test_runner/regress/test_subscriber_branching.py

@@ -251,7 +251,7 @@ def test_multiple_subscription_branching(neon_simple_env: NeonEnv):
     NUMBER_OF_DBS = 5
 
     # Create and start endpoint so that neon_local put all the generated
-    # stuff into the config.json file.
+    # stuff into the spec.json file.
     endpoint = env.endpoints.create_start(
         "main",
         config_lines=[
@@ -280,15 +280,13 @@ def test_multiple_subscription_branching(neon_simple_env: NeonEnv):
             }
         )
 
-    # Update the config.json file to create the databases
+    # Update the spec.json file to create the databases
     # and reconfigure the endpoint to apply the changes.
     endpoint.respec_deep(
         **{
-            "spec": {
-                "skip_pg_catalog_updates": False,
-                "cluster": {
-                    "databases": TEST_DB_NAMES,
-                },
+            "skip_pg_catalog_updates": False,
+            "cluster": {
+                "databases": TEST_DB_NAMES,
             },
         }
     )

test_runner/regress/test_wal_acceptor.py

@@ -45,7 +45,7 @@ from fixtures.remote_storage import (
     s3_storage,
 )
 from fixtures.safekeeper.http import (
-    MembershipConfiguration,
+    Configuration,
     SafekeeperHttpClient,
     SafekeeperId,
     TimelineCreateRequest,
@@ -589,7 +589,7 @@ def test_s3_wal_replay(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re
     for sk in env.safekeepers:
         sk.start()
         cli = sk.http_client()
-        mconf = MembershipConfiguration(generation=0, members=[], new_members=None)
+        mconf = Configuration(generation=0, members=[], new_members=None)
         # set start_lsn to the beginning of the first segment to allow reading
         # WAL from there (could you intidb LSN as well).
         r = TimelineCreateRequest(
@@ -1948,7 +1948,7 @@ def test_membership_api(neon_env_builder: NeonEnvBuilder):
     sk_id_2 = SafekeeperId(11, "localhost", 5434)  # just a mock
 
     # Request to switch before timeline creation should fail.
-    init_conf = MembershipConfiguration(generation=1, members=[sk_id_1], new_members=None)
+    init_conf = Configuration(generation=1, members=[sk_id_1], new_members=None)
     with pytest.raises(requests.exceptions.HTTPError):
         http_cli.membership_switch(tenant_id, timeline_id, init_conf)
 
@@ -1960,7 +1960,7 @@ def test_membership_api(neon_env_builder: NeonEnvBuilder):
     http_cli.timeline_create(create_r)
 
     # Switch into some conf.
-    joint_conf = MembershipConfiguration(generation=4, members=[sk_id_1], new_members=[sk_id_2])
+    joint_conf = Configuration(generation=4, members=[sk_id_1], new_members=[sk_id_2])
     resp = http_cli.membership_switch(tenant_id, timeline_id, joint_conf)
     log.info(f"joint switch resp: {resp}")
     assert resp.previous_conf.generation == 1
@@ -1973,26 +1973,24 @@ def test_membership_api(neon_env_builder: NeonEnvBuilder):
     assert after_restart.generation == 4
 
     # Switch into non joint conf of which sk is not a member, must fail.
-    non_joint_not_member = MembershipConfiguration(
-        generation=5, members=[sk_id_2], new_members=None
-    )
+    non_joint_not_member = Configuration(generation=5, members=[sk_id_2], new_members=None)
     with pytest.raises(requests.exceptions.HTTPError):
         resp = http_cli.membership_switch(tenant_id, timeline_id, non_joint_not_member)
 
     # Switch into good non joint conf.
-    non_joint = MembershipConfiguration(generation=6, members=[sk_id_1], new_members=None)
+    non_joint = Configuration(generation=6, members=[sk_id_1], new_members=None)
     resp = http_cli.membership_switch(tenant_id, timeline_id, non_joint)
     log.info(f"non joint switch resp: {resp}")
     assert resp.previous_conf.generation == 4
     assert resp.current_conf.generation == 6
 
     # Switch request to lower conf should be rejected.
-    lower_conf = MembershipConfiguration(generation=3, members=[sk_id_1], new_members=None)
+    lower_conf = Configuration(generation=3, members=[sk_id_1], new_members=None)
     with pytest.raises(requests.exceptions.HTTPError):
         http_cli.membership_switch(tenant_id, timeline_id, lower_conf)
 
     # Now, exclude sk from the membership, timeline should be deleted.
-    excluded_conf = MembershipConfiguration(generation=7, members=[sk_id_2], new_members=None)
+    excluded_conf = Configuration(generation=7, members=[sk_id_2], new_members=None)
     http_cli.timeline_exclude(tenant_id, timeline_id, excluded_conf)
     with pytest.raises(requests.exceptions.HTTPError):
         http_cli.timeline_status(tenant_id, timeline_id)
@@ -2012,6 +2010,11 @@ def test_explicit_timeline_creation(neon_env_builder: NeonEnvBuilder):
     tenant_id = env.initial_tenant
     timeline_id = env.initial_timeline
 
+    ps = env.pageservers[0]
+    ps_http_cli = ps.http_client()
+
+    http_clis = [sk.http_client() for sk in env.safekeepers]
+
     config_lines = [
         "neon.safekeeper_proto_version = 3",
     ]
@@ -2020,11 +2023,22 @@ def test_explicit_timeline_creation(neon_env_builder: NeonEnvBuilder):
     # expected to fail because timeline is not created on safekeepers
     with pytest.raises(Exception, match=r".*timed out.*"):
         ep.start(safekeeper_generation=1, safekeepers=[1, 2, 3], timeout="2s")
+    # figure out initial LSN.
+    ps_timeline_detail = ps_http_cli.timeline_detail(tenant_id, timeline_id)
+    init_lsn = ps_timeline_detail["last_record_lsn"]
+    log.info(f"initial LSN: {init_lsn}")
+    # sk timeline creation request expects minor version
+    pg_version = ps_timeline_detail["pg_version"] * 10000
     # create inital mconf
-    mconf = MembershipConfiguration(
-        generation=1, members=Safekeeper.sks_to_safekeeper_ids(env.safekeepers), new_members=None
+    sk_ids = [SafekeeperId(sk.id, "localhost", sk.port.pg_tenant_only) for sk in env.safekeepers]
+    mconf = Configuration(generation=1, members=sk_ids, new_members=None)
+    create_r = TimelineCreateRequest(
+        tenant_id, timeline_id, mconf, pg_version, Lsn(init_lsn), commit_lsn=None
     )
-    Safekeeper.create_timeline(tenant_id, timeline_id, env.pageservers[0], mconf, env.safekeepers)
+    log.info(f"sending timeline create: {create_r.to_json()}")
+
+    for sk_http_cli in http_clis:
+        sk_http_cli.timeline_create(create_r)
     # Once timeline created endpoint should start.
     ep.start(safekeeper_generation=1, safekeepers=[1, 2, 3])
     ep.safe_psql("CREATE TABLE IF NOT EXISTS t(key int, value text)")

test_runner/regress/test_wal_acceptor_async.py

@@ -18,7 +18,6 @@ from fixtures.neon_fixtures import (
     Safekeeper,
 )
 from fixtures.remote_storage import RemoteStorageKind
-from fixtures.safekeeper.http import MembershipConfiguration
 from fixtures.utils import skip_in_debug_build
 
 if TYPE_CHECKING:
@@ -453,24 +452,20 @@ def test_concurrent_computes(neon_env_builder: NeonEnvBuilder):
     asyncio.run(run_concurrent_computes(env))
 
 
-async def assert_query_hangs(endpoint: Endpoint, query: str):
-    """
-    Start on endpoint query which is expected to hang and check that it does.
-    """
-    conn = await endpoint.connect_async()
-    bg_query = asyncio.create_task(conn.execute(query))
-    await asyncio.sleep(2)
-    assert not bg_query.done()
-    return bg_query
-
-
 # Stop safekeeper and check that query cannot be executed while safekeeper is down.
 # Query will insert a single row into a table.
-async def check_unavailability(sk: Safekeeper, ep: Endpoint, key: int, start_delay_sec: int = 2):
+async def check_unavailability(
+    sk: Safekeeper, conn: asyncpg.Connection, key: int, start_delay_sec: int = 2
+):
     # shutdown one of two acceptors, that is, majority
     sk.stop()
 
-    bg_query = await assert_query_hangs(ep, f"INSERT INTO t values ({key}, 'payload')")
+    bg_query = asyncio.create_task(conn.execute(f"INSERT INTO t values ({key}, 'payload')"))
+
+    await asyncio.sleep(start_delay_sec)
+    # ensure that the query has not been executed yet
+    assert not bg_query.done()
+
     # start safekeeper and await the query
     sk.start()
     await bg_query
@@ -485,10 +480,10 @@ async def run_unavailability(env: NeonEnv, endpoint: Endpoint):
     await conn.execute("INSERT INTO t values (1, 'payload')")
 
     # stop safekeeper and check that query cannot be executed while safekeeper is down
-    await check_unavailability(env.safekeepers[0], endpoint, 2)
+    await check_unavailability(env.safekeepers[0], conn, 2)
 
     # for the world's balance, do the same with second safekeeper
-    await check_unavailability(env.safekeepers[1], endpoint, 3)
+    await check_unavailability(env.safekeepers[1], conn, 3)
 
     # check that we can execute queries after restart
     await conn.execute("INSERT INTO t values (4, 'payload')")
@@ -519,7 +514,15 @@ async def run_recovery_uncommitted(env: NeonEnv):
     # insert with only one safekeeper up to create tail of flushed but not committed WAL
     sk1.stop()
     sk2.stop()
-    await assert_query_hangs(ep, "insert into t select generate_series(1, 2000), 'payload'")
+    conn = await ep.connect_async()
+    # query should hang, so execute in separate task
+    bg_query = asyncio.create_task(
+        conn.execute("insert into t select generate_series(1, 2000), 'payload'")
+    )
+    sleep_sec = 2
+    await asyncio.sleep(sleep_sec)
+    # it must still be not finished
+    assert not bg_query.done()
     # note: destoy will kill compute_ctl, preventing it waiting for hanging sync-safekeepers.
     ep.stop_and_destroy()
 
@@ -556,7 +559,15 @@ async def run_wal_truncation(env: NeonEnv, safekeeper_proto_version: int):
     # insert with only one sk3 up to create tail of flushed but not committed WAL on it
     sk1.stop()
     sk2.stop()
-    await assert_query_hangs(ep, "insert into t select generate_series(1, 180000), 'Papaya'")
+    conn = await ep.connect_async()
+    # query should hang, so execute in separate task
+    bg_query = asyncio.create_task(
+        conn.execute("insert into t select generate_series(1, 180000), 'Papaya'")
+    )
+    sleep_sec = 2
+    await asyncio.sleep(sleep_sec)
+    # it must still be not finished
+    assert not bg_query.done()
     # note: destoy will kill compute_ctl, preventing it waiting for hanging sync-safekeepers.
     ep.stop_and_destroy()
 
@@ -596,132 +607,6 @@ def test_wal_truncation(neon_env_builder: NeonEnvBuilder, safekeeper_proto_versi
     asyncio.run(run_wal_truncation(env, safekeeper_proto_version))
 
 
-async def quorum_sanity_single(
-    env: NeonEnv,
-    compute_sks_ids: list[int],
-    members_sks_ids: list[int],
-    new_members_sks_ids: list[int] | None,
-    sks_to_stop_ids: list[int],
-    should_work_when_stopped: bool,
-):
-    """
-    *_ids params contain safekeeper node ids; it is assumed they are issued
-    from 1 and sequentially assigned to env.safekeepers.
-    """
-    members_sks = [env.safekeepers[i - 1] for i in members_sks_ids]
-    new_members_sks = (
-        [env.safekeepers[i - 1] for i in new_members_sks_ids] if new_members_sks_ids else None
-    )
-    sks_to_stop = [env.safekeepers[i - 1] for i in sks_to_stop_ids]
-
-    mconf = MembershipConfiguration(
-        generation=1,
-        members=Safekeeper.sks_to_safekeeper_ids(members_sks),
-        new_members=Safekeeper.sks_to_safekeeper_ids(new_members_sks) if new_members_sks else None,
-    )
-    members_sks = Safekeeper.mconf_sks(env, mconf)
-
-    tenant_id = env.initial_tenant
-    compute_sks_ids_str = "-".join([str(sk_id) for sk_id in compute_sks_ids])
-    members_sks_ids_str = "-".join([str(sk.id) for sk in mconf.members])
-    new_members_sks_ids_str = "-".join(
-        [str(sk.id) for sk in mconf.new_members] if mconf.new_members is not None else []
-    )
-    sks_to_stop_ids_str = "-".join([str(sk.id) for sk in sks_to_stop])
-    log.info(
-        f"running quorum_sanity_single with compute_sks={compute_sks_ids_str}, members_sks={members_sks_ids_str}, new_members_sks={new_members_sks_ids_str}, sks_to_stop={sks_to_stop_ids_str}, should_work_when_stopped={should_work_when_stopped}"
-    )
-    branch_name = f"test_quorum_single_c{compute_sks_ids_str}_m{members_sks_ids_str}_{new_members_sks_ids_str}_s{sks_to_stop_ids_str}"
-    timeline_id = env.create_branch(branch_name)
-
-    # create timeline on `members_sks`
-    Safekeeper.create_timeline(tenant_id, timeline_id, env.pageservers[0], mconf, members_sks)
-
-    config_lines = [
-        "neon.safekeeper_proto_version = 3",
-    ]
-    ep = env.endpoints.create(branch_name, config_lines=config_lines)
-    ep.start(safekeeper_generation=1, safekeepers=compute_sks_ids)
-    ep.safe_psql("create table t(key int, value text)")
-
-    # stop specified sks and check whether writes work
-    for sk in sks_to_stop:
-        sk.stop()
-    if should_work_when_stopped:
-        log.info("checking that writes still work")
-        ep.safe_psql("insert into t select generate_series(1, 100), 'Papaya'")
-        # restarting ep should also be fine
-        ep.stop()
-        ep.start()
-        ep.safe_psql("insert into t select generate_series(1, 100), 'plum'")
-        bg_query = None
-    else:
-        log.info("checking that writes hang")
-        bg_query = await assert_query_hangs(
-            ep, "insert into t select generate_series(1, 100), 'Papaya'"
-        )
-    # start again; now they should work
-    for sk in sks_to_stop:
-        sk.start()
-    if bg_query:
-        log.info("awaiting query")
-        await bg_query
-
-
-# It's a bit tempting to iterate over all possible combinations, but let's stick
-# with this for now.
-async def run_quorum_sanity(env: NeonEnv):
-    # 3 members, all up, should work
-    await quorum_sanity_single(env, [1, 2, 3], [1, 2, 3], None, [], True)
-    # 3 members, 2/3 up, should work
-    await quorum_sanity_single(env, [1, 2, 3], [1, 2, 3], None, [3], True)
-    # 3 members, 1/3 up, should not work
-    await quorum_sanity_single(env, [1, 2, 3], [1, 2, 3], None, [2, 3], False)
-
-    # 3 members, all up, should work; wp redundantly talks to 4th.
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], None, [], True)
-    # 3 members, all up, should work with wp talking to 2 of these 3 + plus one redundant
-    await quorum_sanity_single(env, [2, 3, 4], [1, 2, 3], None, [], True)
-    # 3 members, 2/3 up, could work but wp talks to different 3s, so it shouldn't
-    await quorum_sanity_single(env, [2, 3, 4], [1, 2, 3], None, [3], False)
-
-    # joint conf of 1-2-3 and 4, all up, should work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [4], [], True)
-    # joint conf of 1-2-3 and 4, 4 down, shouldn't work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [4], [4], False)
-
-    # joint conf of 1-2-3 and 2-3-4, all up, should work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [], True)
-    # joint conf of 1-2-3 and 2-3-4, 1 and 4 down, should work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [1, 4], True)
-    # joint conf of 1-2-3 and 2-3-4, 2 down, should work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [2], True)
-    # joint conf of 1-2-3 and 2-3-4, 3 down, should work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [3], True)
-    # joint conf of 1-2-3 and 2-3-4, 1 and 2 down, shouldn't work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [1, 2], False)
-    # joint conf of 1-2-3 and 2-3-4, 2 and 4 down, shouldn't work
-    await quorum_sanity_single(env, [1, 2, 3, 4], [1, 2, 3], [2, 3, 4], [2, 4], False)
-
-    # joint conf of 1-2-3 and 2-3-4 with wp talking to 2-3-4 only.
-    await quorum_sanity_single(env, [2, 3, 4], [1, 2, 3], [2, 3, 4], [], True)
-    # with 1 down should still be ok
-    await quorum_sanity_single(env, [2, 3, 4], [1, 2, 3], [2, 3, 4], [1], True)
-    # but with 2 down not ok
-    await quorum_sanity_single(env, [2, 3, 4], [1, 2, 3], [2, 3, 4], [2], False)
-
-
-# Test various combinations of membership configurations / neon.safekeepers
-# (list of safekeepers endpoint connects to) values / up & down safekeepers and
-# check that endpont can start and write data when we have quorum and can't when
-# we don't.
-def test_quorum_sanity(neon_env_builder: NeonEnvBuilder):
-    neon_env_builder.num_safekeepers = 4
-    env = neon_env_builder.init_start()
-
-    asyncio.run(run_quorum_sanity(env))
-
-
 async def run_segment_init_failure(env: NeonEnv):
     env.create_branch("test_segment_init_failure")
     ep = env.endpoints.create_start("test_segment_init_failure")