From 1e7cd6ac9f3568ffe9db952cb89f8036330d27b5 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Fri, 3 May 2024 19:15:38 +0200 Subject: [PATCH] refactor: move `NodeMetadata` to `pageserver_api`; use it from `neon_local` (#7606) This is the first step towards representing all of Pageserver configuration as clean `serde::Serialize`able Rust structs in `pageserver_api`. The `neon_local` code will then use those structs instead of the crude `toml_edit` / string concatenation that it does today. refs https://github.com/neondatabase/neon/issues/7555 --------- Co-authored-by: Alex Chi Z --- control_plane/src/bin/neon_local.rs | 8 +++---- control_plane/src/pageserver.rs | 13 ++++++----- libs/pageserver_api/src/config.rs | 31 +++++++++++++++++++++++++ libs/pageserver_api/src/config/tests.rs | 22 ++++++++++++++++++ libs/pageserver_api/src/lib.rs | 6 +---- pageserver/src/config.rs | 24 ++----------------- pageserver/src/control_plane_client.rs | 6 ++--- 7 files changed, 69 insertions(+), 41 deletions(-) create mode 100644 libs/pageserver_api/src/config.rs create mode 100644 libs/pageserver_api/src/config/tests.rs diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index bdd64c8687..14b83c1252 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -14,15 +14,15 @@ use control_plane::pageserver::{PageServerNode, PAGESERVER_REMOTE_STORAGE_DIR}; use control_plane::safekeeper::SafekeeperNode; use control_plane::storage_controller::StorageController; use control_plane::{broker, local_env}; +use pageserver_api::config::{ + DEFAULT_HTTP_LISTEN_PORT as DEFAULT_PAGESERVER_HTTP_PORT, + DEFAULT_PG_LISTEN_PORT as DEFAULT_PAGESERVER_PG_PORT, +}; use pageserver_api::controller_api::PlacementPolicy; use pageserver_api::models::{ ShardParameters, TenantCreateRequest, TimelineCreateRequest, TimelineInfo, }; use pageserver_api::shard::{ShardCount, ShardStripeSize, TenantShardId}; -use pageserver_api::{ - DEFAULT_HTTP_LISTEN_PORT as DEFAULT_PAGESERVER_HTTP_PORT, - DEFAULT_PG_LISTEN_PORT as DEFAULT_PAGESERVER_PG_PORT, -}; use postgres_backend::AuthType; use postgres_connection::parse_host_port; use safekeeper_api::{ diff --git a/control_plane/src/pageserver.rs b/control_plane/src/pageserver.rs index 52accc5890..1a64391306 100644 --- a/control_plane/src/pageserver.rs +++ b/control_plane/src/pageserver.rs @@ -248,12 +248,13 @@ impl PageServerNode { // situation: the metadata is written by some other script. std::fs::write( metadata_path, - serde_json::to_vec(&serde_json::json!({ - "host": "localhost", - "port": self.pg_connection_config.port(), - "http_host": "localhost", - "http_port": http_port, - })) + serde_json::to_vec(&pageserver_api::config::NodeMetadata { + postgres_host: "localhost".to_string(), + postgres_port: self.pg_connection_config.port(), + http_host: "localhost".to_string(), + http_port, + other: HashMap::new(), + }) .unwrap(), ) .expect("Failed to write metadata file"); diff --git a/libs/pageserver_api/src/config.rs b/libs/pageserver_api/src/config.rs new file mode 100644 index 0000000000..d996a62349 --- /dev/null +++ b/libs/pageserver_api/src/config.rs @@ -0,0 +1,31 @@ +use std::collections::HashMap; + +use const_format::formatcp; + +#[cfg(test)] +mod tests; + +pub const DEFAULT_PG_LISTEN_PORT: u16 = 64000; +pub const DEFAULT_PG_LISTEN_ADDR: &str = formatcp!("127.0.0.1:{DEFAULT_PG_LISTEN_PORT}"); +pub const DEFAULT_HTTP_LISTEN_PORT: u16 = 9898; +pub const DEFAULT_HTTP_LISTEN_ADDR: &str = formatcp!("127.0.0.1:{DEFAULT_HTTP_LISTEN_PORT}"); + +// Certain metadata (e.g. externally-addressable name, AZ) is delivered +// as a separate structure. This information is not neeed by the pageserver +// itself, it is only used for registering the pageserver with the control +// plane and/or storage controller. +// +#[derive(PartialEq, Eq, Debug, serde::Serialize, serde::Deserialize)] +pub struct NodeMetadata { + #[serde(rename = "host")] + pub postgres_host: String, + #[serde(rename = "port")] + pub postgres_port: u16, + pub http_host: String, + pub http_port: u16, + + // Deployment tools may write fields to the metadata file beyond what we + // use in this type: this type intentionally only names fields that require. + #[serde(flatten)] + pub other: HashMap, +} diff --git a/libs/pageserver_api/src/config/tests.rs b/libs/pageserver_api/src/config/tests.rs new file mode 100644 index 0000000000..edeefc156e --- /dev/null +++ b/libs/pageserver_api/src/config/tests.rs @@ -0,0 +1,22 @@ +use super::*; + +#[test] +fn test_node_metadata_v1_backward_compatibilty() { + let v1 = serde_json::to_vec(&serde_json::json!({ + "host": "localhost", + "port": 23, + "http_host": "localhost", + "http_port": 42, + })); + + assert_eq!( + serde_json::from_slice::(&v1.unwrap()).unwrap(), + NodeMetadata { + postgres_host: "localhost".to_string(), + postgres_port: 23, + http_host: "localhost".to_string(), + http_port: 42, + other: HashMap::new(), + } + ) +} diff --git a/libs/pageserver_api/src/lib.rs b/libs/pageserver_api/src/lib.rs index 1b948d60c3..532185a366 100644 --- a/libs/pageserver_api/src/lib.rs +++ b/libs/pageserver_api/src/lib.rs @@ -1,6 +1,5 @@ #![deny(unsafe_code)] #![deny(clippy::undocumented_unsafe_blocks)] -use const_format::formatcp; pub mod controller_api; pub mod key; @@ -11,7 +10,4 @@ pub mod shard; /// Public API types pub mod upcall_api; -pub const DEFAULT_PG_LISTEN_PORT: u16 = 64000; -pub const DEFAULT_PG_LISTEN_ADDR: &str = formatcp!("127.0.0.1:{DEFAULT_PG_LISTEN_PORT}"); -pub const DEFAULT_HTTP_LISTEN_PORT: u16 = 9898; -pub const DEFAULT_HTTP_LISTEN_ADDR: &str = formatcp!("127.0.0.1:{DEFAULT_HTTP_LISTEN_PORT}"); +pub mod config; diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 96fff1f0c0..258eed0b12 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -9,7 +9,7 @@ use pageserver_api::shard::TenantShardId; use remote_storage::{RemotePath, RemoteStorageConfig}; use serde; use serde::de::IntoDeserializer; -use std::{collections::HashMap, env}; +use std::env; use storage_broker::Uri; use utils::crashsafe::path_with_suffix_extension; use utils::id::ConnectionId; @@ -51,7 +51,7 @@ pub mod defaults { use crate::tenant::config::defaults::*; use const_format::formatcp; - pub use pageserver_api::{ + pub use pageserver_api::config::{ DEFAULT_HTTP_LISTEN_ADDR, DEFAULT_HTTP_LISTEN_PORT, DEFAULT_PG_LISTEN_ADDR, DEFAULT_PG_LISTEN_PORT, }; @@ -335,26 +335,6 @@ impl BuilderValue { } } -// Certain metadata (e.g. externally-addressable name, AZ) is delivered -// as a separate structure. This information is not neeed by the pageserver -// itself, it is only used for registering the pageserver with the control -// plane and/or storage controller. -// -#[derive(serde::Deserialize)] -pub(crate) struct NodeMetadata { - #[serde(rename = "host")] - pub(crate) postgres_host: String, - #[serde(rename = "port")] - pub(crate) postgres_port: u16, - pub(crate) http_host: String, - pub(crate) http_port: u16, - - // Deployment tools may write fields to the metadata file beyond what we - // use in this type: this type intentionally only names fields that require. - #[serde(flatten)] - pub(crate) other: HashMap, -} - // needed to simplify config construction #[derive(Default)] struct PageServerConfigBuilder { diff --git a/pageserver/src/control_plane_client.rs b/pageserver/src/control_plane_client.rs index db0032891e..26e7cc7ef8 100644 --- a/pageserver/src/control_plane_client.rs +++ b/pageserver/src/control_plane_client.rs @@ -14,10 +14,8 @@ use tokio_util::sync::CancellationToken; use url::Url; use utils::{backoff, failpoint_support, generation::Generation, id::NodeId}; -use crate::{ - config::{NodeMetadata, PageServerConf}, - virtual_file::on_fatal_io_error, -}; +use crate::{config::PageServerConf, virtual_file::on_fatal_io_error}; +use pageserver_api::config::NodeMetadata; /// The Pageserver's client for using the control plane API: this is a small subset /// of the overall control plane API, for dealing with generations (see docs/rfcs/025-generation-numbers.md)