Merge branch 'tristan957/pgconf-port' of https://github.com/neondatabase/neon into tristan957/pgconf-port

compute_tools/src/compute.rs

@@ -1457,15 +1457,23 @@ impl ComputeNode {
         let pgdata_path = Path::new(&self.params.pgdata);
 
         let tls_config = self.tls_config(&pspec.spec);
+        let databricks_settings = spec.databricks_settings.as_ref();
 
+        let postgres_port = self
+            .params
+            .connstr
+            .port()
+            .expect("port must be present in connstr");
         // Remove/create an empty pgdata directory and put configuration there.
         self.create_pgdata()?;
         config::write_postgres_conf(
             pgdata_path,
             &self.params,
             &pspec.spec,
+            postgres_port,
             self.params.internal_http_port,
             tls_config,
+            databricks_settings,
         )?;
 
         // Syncing safekeepers is only safe with primary nodes: if a primary
@@ -1505,16 +1513,16 @@ impl ComputeNode {
             )
         })?;
 
-        if let Some(databricks_settings) = spec.databricks_settings.as_ref() {
+        if let Some(settings) = databricks_settings {
             copy_tls_certificates(
-                &databricks_settings.pg_compute_tls_settings.key_file,
-                &databricks_settings.pg_compute_tls_settings.cert_file,
+                &settings.pg_compute_tls_settings.key_file,
+                &settings.pg_compute_tls_settings.cert_file,
                 pgdata_path,
             )?;
 
             // Update pg_hba.conf received with basebackup including additional databricks settings.
-            update_pg_hba(pgdata_path, Some(&databricks_settings.databricks_pg_hba))?;
-            update_pg_ident(pgdata_path, Some(&databricks_settings.databricks_pg_ident))?;
+            update_pg_hba(pgdata_path, Some(&settings.databricks_pg_hba))?;
+            update_pg_ident(pgdata_path, Some(&settings.databricks_pg_ident))?;
         } else {
             // Update pg_hba.conf received with basebackup.
             update_pg_hba(pgdata_path, None)?;
@@ -1958,12 +1966,19 @@ impl ComputeNode {
 
         // Write new config
         let pgdata_path = Path::new(&self.params.pgdata);
+        let postgres_port = self
+            .params
+            .connstr
+            .port()
+            .expect("port must be present in connstr");
         config::write_postgres_conf(
             pgdata_path,
             &self.params,
             &spec,
+            postgres_port,
             self.params.internal_http_port,
             tls_config,
+            spec.databricks_settings.as_ref(),
         )?;
 
         self.pg_reload_conf()?;

compute_tools/src/config.rs

@@ -7,11 +7,14 @@ use std::io::prelude::*;
 use std::path::Path;
 
 use compute_api::responses::TlsConfig;
-use compute_api::spec::{ComputeAudit, ComputeMode, ComputeSpec, GenericOption};
+use compute_api::spec::{
+    ComputeAudit, ComputeMode, ComputeSpec, DatabricksSettings, GenericOption,
+};
 
 use crate::compute::ComputeNodeParams;
 use crate::pg_helpers::{
-    GenericOptionExt, GenericOptionsSearch, PgOptionsSerialize, escape_conf_value,
+    DatabricksSettingsExt as _, GenericOptionExt, GenericOptionsSearch, PgOptionsSerialize,
+    escape_conf_value,
 };
 use crate::tls::{self, SERVER_CRT, SERVER_KEY};
 
@@ -44,8 +47,10 @@ pub fn write_postgres_conf(
     pgdata_path: &Path,
     params: &ComputeNodeParams,
     spec: &ComputeSpec,
+    postgres_port: u16,
     extension_server_port: u16,
     tls_config: &Option<TlsConfig>,
+    databricks_settings: Option<&DatabricksSettings>,
 ) -> Result<()> {
     let path = pgdata_path.join("postgresql.conf");
     // File::create() destroys the file content if it exists.
@@ -279,12 +284,27 @@ pub fn write_postgres_conf(
         writeln!(file, "neon.disable_logical_replication_subscribers=false")?;
     }
 
+    // Explicitly set the port based on the connstr, overriding any previous port setting.
+    // Note: It is important that we don't specify a different port again after this.
+    writeln!(file, "port = {postgres_port}")?;
+
     // We need Postgres to send logs to rsyslog so that we can forward them
     // further to customers' log aggregation systems.
     if spec.logs_export_host.is_some() {
         writeln!(file, "log_destination='stderr,syslog'")?;
     }
 
+    // This is databricks specific settings.
+    // This should be at the end of the file but before `compute_ctl_temp_override.conf` below
+    // so that it can override any settings above.
+    // `compute_ctl_temp_override.conf` is intended to override any settings above during specific operations.
+    // To prevent potential breakage in the future, we keep it above `compute_ctl_temp_override.conf`.
+    writeln!(file, "# Databricks settings start")?;
+    if let Some(settings) = databricks_settings {
+        writeln!(file, "{}", settings.as_pg_settings())?;
+    }
+    writeln!(file, "# Databricks settings end")?;
+
     // This is essential to keep this line at the end of the file,
     // because it is intended to override any settings above.
     writeln!(file, "include_if_exists = 'compute_ctl_temp_override.conf'")?;

libs/compute_api/src/spec.rs

@@ -195,7 +195,6 @@ pub struct ComputeSpec {
     pub suspend_timeout_seconds: i64,
 
     // Databricks specific options for compute instance.
-    // These settings are not part of postgresql.conf.
     pub databricks_settings: Option<DatabricksSettings>,
 }