diff --git a/.env.example b/.env.example index 07c1f9b..c498136 100644 --- a/.env.example +++ b/.env.example @@ -3,6 +3,11 @@ POSTGRES_USER=crank POSTGRES_PASSWORD=change-me POSTGRES_HOST=postgres POSTGRES_PORT=5432 +POSTGRES_MAX_CONNECTIONS=20 +POSTGRES_MIN_CONNECTIONS=2 +POSTGRES_ACQUIRE_TIMEOUT_MS=5000 +POSTGRES_IDLE_TIMEOUT_MS=600000 +POSTGRES_MAX_LIFETIME_MS=1800000 CRANK_ADMIN_API_IMAGE=crank/admin-api:dev CRANK_MCP_SERVER_IMAGE=crank/mcp-server:dev CRANK_UI_IMAGE=crank/ui:dev diff --git a/TASKS.md b/TASKS.md index 4bd13b8..46c5a8d 100644 --- a/TASKS.md +++ b/TASKS.md @@ -2,18 +2,18 @@ ## Current -### `feat/postgres-pool-config` +### `feat/postgres-registry-modularization` Status: in_progress DoD: -- PostgreSQL pool settings are explicit instead of implicit sqlx defaults -- pool sizing and timeouts come from runtime config/env -- startup validates pool configuration and tests cover parsing/defaults +- `crates/crank-registry/src/postgres.rs` is split into domain modules under `postgres/` +- `PostgresRegistry` keeps only constructors and high-level surface wiring +- existing registry tests still pass after module split ## Next -- `feat/postgres-registry-modularization` +- `feat/sqlx-compile-time-verification` ## Backlog diff --git a/apps/admin-api/src/main.rs b/apps/admin-api/src/main.rs index 617b966..e01d7f5 100644 --- a/apps/admin-api/src/main.rs +++ b/apps/admin-api/src/main.rs @@ -8,7 +8,7 @@ mod storage; use std::{env, net::SocketAddr, path::PathBuf}; -use crank_registry::PostgresRegistry; +use crank_registry::{PostgresPoolConfig, PostgresRegistry}; use sqlx::postgres::PgConnectOptions; use tokio::net::TcpListener; use tracing::info; @@ -35,7 +35,12 @@ async fn main() -> Result<(), Box> { let bind_addr = env::var("CRANK_ADMIN_BIND").unwrap_or_else(|_| "0.0.0.0:3001".into()); let base_url = env::var("CRANK_BASE_URL").unwrap_or_else(|_| "http://localhost:3000".into()); let socket_addr: SocketAddr = bind_addr.parse()?; - let registry = PostgresRegistry::connect_with_options(database_options_from_env()?).await?; + let pool_config = PostgresPoolConfig::from_env()?; + let registry = PostgresRegistry::connect_with_options_and_pool_config( + database_options_from_env()?, + pool_config, + ) + .await?; let auth_settings = AuthSettings { session_secret: env::var("CRANK_SESSION_SECRET")?, password_pepper: env::var("CRANK_PASSWORD_PEPPER")?, @@ -61,6 +66,14 @@ async fn main() -> Result<(), Box> { let app = build_app(state); let listener = TcpListener::bind(socket_addr).await?; + info!( + max_connections = pool_config.max_connections, + min_connections = pool_config.min_connections, + acquire_timeout_ms = pool_config.acquire_timeout_ms, + idle_timeout_ms = pool_config.idle_timeout_ms, + max_lifetime_ms = pool_config.max_lifetime_ms, + "postgres pool configured" + ); info!("admin-api listening on {}", socket_addr); axum::serve(listener, app).await?; diff --git a/apps/mcp-server/src/main.rs b/apps/mcp-server/src/main.rs index 60c50ac..abf2d4f 100644 --- a/apps/mcp-server/src/main.rs +++ b/apps/mcp-server/src/main.rs @@ -5,7 +5,7 @@ mod session; use std::{env, net::SocketAddr, time::Duration}; -use crank_registry::PostgresRegistry; +use crank_registry::{PostgresPoolConfig, PostgresRegistry}; use crank_runtime::SecretCrypto; use sqlx::postgres::PgConnectOptions; use tokio::net::TcpListener; @@ -30,11 +30,24 @@ async fn main() -> Result<(), Box> { .map(Duration::from_millis) .unwrap_or_else(|| Duration::from_secs(5)); let socket_addr: SocketAddr = bind_addr.parse()?; - let registry = PostgresRegistry::connect_with_options(database_options_from_env()?).await?; + let pool_config = PostgresPoolConfig::from_env()?; + let registry = PostgresRegistry::connect_with_options_and_pool_config( + database_options_from_env()?, + pool_config, + ) + .await?; let secret_crypto = SecretCrypto::new(&env::var("CRANK_MASTER_KEY")?)?; let app = build_app(registry, refresh_interval, base_url, secret_crypto); let listener = TcpListener::bind(socket_addr).await?; + info!( + max_connections = pool_config.max_connections, + min_connections = pool_config.min_connections, + acquire_timeout_ms = pool_config.acquire_timeout_ms, + idle_timeout_ms = pool_config.idle_timeout_ms, + max_lifetime_ms = pool_config.max_lifetime_ms, + "postgres pool configured" + ); info!("mcp-server listening on {}", socket_addr); axum::serve(listener, app).await?; diff --git a/crates/crank-registry/src/lib.rs b/crates/crank-registry/src/lib.rs index 6e9d592..d297d33 100644 --- a/crates/crank-registry/src/lib.rs +++ b/crates/crank-registry/src/lib.rs @@ -21,4 +21,4 @@ pub use model::{ WorkspaceMembershipRecord, WorkspaceRecord, YamlImportJob, YamlImportJobCompletion, YamlImportJobId, YamlImportJobStatus, }; -pub use postgres::PostgresRegistry; +pub use postgres::{PostgresPoolConfig, PostgresPoolConfigError, PostgresRegistry}; diff --git a/crates/crank-registry/src/postgres.rs b/crates/crank-registry/src/postgres.rs index fe6fbce..e91d0ca 100644 --- a/crates/crank-registry/src/postgres.rs +++ b/crates/crank-registry/src/postgres.rs @@ -13,6 +13,8 @@ use sqlx::{ postgres::{PgConnectOptions, PgPoolOptions, PgRow}, types::Json, }; +use std::{collections::BTreeMap, env, time::Duration}; +use thiserror::Error; use crate::{ error::RegistryError, @@ -41,6 +43,97 @@ pub struct PostgresRegistry { pool: PgPool, } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PostgresPoolConfig { + pub max_connections: u32, + pub min_connections: u32, + pub acquire_timeout_ms: u64, + pub idle_timeout_ms: u64, + pub max_lifetime_ms: u64, +} + +#[derive(Debug, Error, PartialEq, Eq)] +pub enum PostgresPoolConfigError { + #[error("invalid postgres pool setting {name}={value}")] + InvalidValue { name: &'static str, value: String }, + #[error("POSTGRES_MAX_CONNECTIONS must be greater than zero")] + ZeroMaxConnections, + #[error("POSTGRES_MIN_CONNECTIONS must not exceed POSTGRES_MAX_CONNECTIONS")] + MinConnectionsExceedMax, +} + +impl Default for PostgresPoolConfig { + fn default() -> Self { + Self { + max_connections: 20, + min_connections: 2, + acquire_timeout_ms: 5_000, + idle_timeout_ms: 600_000, + max_lifetime_ms: 1_800_000, + } + } +} + +impl PostgresPoolConfig { + pub fn from_env() -> Result { + Self::from_vars(env::vars()) + } + + fn from_vars(vars: I) -> Result + where + I: IntoIterator, + K: AsRef, + V: AsRef, + { + let vars = vars + .into_iter() + .map(|(name, value)| (name.as_ref().to_owned(), value.as_ref().to_owned())) + .collect::>(); + let defaults = Self::default(); + let config = Self { + max_connections: parse_u32_setting( + &vars, + "POSTGRES_MAX_CONNECTIONS", + defaults.max_connections, + )?, + min_connections: parse_u32_setting( + &vars, + "POSTGRES_MIN_CONNECTIONS", + defaults.min_connections, + )?, + acquire_timeout_ms: parse_u64_setting( + &vars, + "POSTGRES_ACQUIRE_TIMEOUT_MS", + defaults.acquire_timeout_ms, + )?, + idle_timeout_ms: parse_u64_setting( + &vars, + "POSTGRES_IDLE_TIMEOUT_MS", + defaults.idle_timeout_ms, + )?, + max_lifetime_ms: parse_u64_setting( + &vars, + "POSTGRES_MAX_LIFETIME_MS", + defaults.max_lifetime_ms, + )?, + }; + + config.validate()?; + Ok(config) + } + + fn validate(self) -> Result { + if self.max_connections == 0 { + return Err(PostgresPoolConfigError::ZeroMaxConnections); + } + if self.min_connections > self.max_connections { + return Err(PostgresPoolConfigError::MinConnectionsExceedMax); + } + + Ok(self) + } +} + impl PostgresRegistry { pub async fn connect(database_url: &str) -> Result { Self::connect_in_schema(database_url, None).await @@ -49,7 +142,22 @@ impl PostgresRegistry { pub async fn connect_with_options( connect_options: PgConnectOptions, ) -> Result { - let pool = PgPoolOptions::new().connect_with(connect_options).await?; + Self::connect_with_options_and_pool_config(connect_options, PostgresPoolConfig::default()) + .await + } + + pub async fn connect_with_options_and_pool_config( + connect_options: PgConnectOptions, + pool_config: PostgresPoolConfig, + ) -> Result { + let pool = PgPoolOptions::new() + .max_connections(pool_config.max_connections) + .min_connections(pool_config.min_connections) + .acquire_timeout(Duration::from_millis(pool_config.acquire_timeout_ms)) + .idle_timeout(Duration::from_millis(pool_config.idle_timeout_ms)) + .max_lifetime(Duration::from_millis(pool_config.max_lifetime_ms)) + .connect_with(connect_options) + .await?; migrations::apply_postgres(&pool).await?; Ok(Self { pool }) } @@ -3583,6 +3691,118 @@ impl PostgresRegistry { } } +fn parse_u32_setting( + vars: &BTreeMap, + name: &'static str, + default: u32, +) -> Result { + vars.get(name) + .map(|value| { + value + .parse::() + .map_err(|_| PostgresPoolConfigError::InvalidValue { + name, + value: value.clone(), + }) + }) + .transpose() + .map(|value| value.unwrap_or(default)) +} + +fn parse_u64_setting( + vars: &BTreeMap, + name: &'static str, + default: u64, +) -> Result { + vars.get(name) + .map(|value| { + value + .parse::() + .map_err(|_| PostgresPoolConfigError::InvalidValue { + name, + value: value.clone(), + }) + }) + .transpose() + .map(|value| value.unwrap_or(default)) +} + +#[cfg(test)] +mod pool_config_tests { + use super::{PostgresPoolConfig, PostgresPoolConfigError}; + + #[test] + fn pool_config_uses_explicit_defaults() { + let config = PostgresPoolConfig::from_vars(std::iter::empty::<(&str, &str)>()).unwrap(); + + assert_eq!( + config, + PostgresPoolConfig { + max_connections: 20, + min_connections: 2, + acquire_timeout_ms: 5_000, + idle_timeout_ms: 600_000, + max_lifetime_ms: 1_800_000, + } + ); + } + + #[test] + fn pool_config_parses_overrides() { + let config = PostgresPoolConfig::from_vars([ + ("POSTGRES_MAX_CONNECTIONS", "32"), + ("POSTGRES_MIN_CONNECTIONS", "4"), + ("POSTGRES_ACQUIRE_TIMEOUT_MS", "7000"), + ("POSTGRES_IDLE_TIMEOUT_MS", "900000"), + ("POSTGRES_MAX_LIFETIME_MS", "3600000"), + ]) + .unwrap(); + + assert_eq!( + config, + PostgresPoolConfig { + max_connections: 32, + min_connections: 4, + acquire_timeout_ms: 7_000, + idle_timeout_ms: 900_000, + max_lifetime_ms: 3_600_000, + } + ); + } + + #[test] + fn pool_config_rejects_invalid_numeric_value() { + let error = + PostgresPoolConfig::from_vars([("POSTGRES_MAX_CONNECTIONS", "abc")]).unwrap_err(); + + assert_eq!( + error, + PostgresPoolConfigError::InvalidValue { + name: "POSTGRES_MAX_CONNECTIONS", + value: "abc".to_owned(), + } + ); + } + + #[test] + fn pool_config_rejects_zero_max_connections() { + let error = PostgresPoolConfig::from_vars([("POSTGRES_MAX_CONNECTIONS", "0")]).unwrap_err(); + + assert_eq!(error, PostgresPoolConfigError::ZeroMaxConnections); + } + + #[test] + fn pool_config_rejects_min_connections_above_max() { + let error = PostgresPoolConfig::from_vars([ + ("POSTGRES_MAX_CONNECTIONS", "2"), + ("POSTGRES_MIN_CONNECTIONS", "3"), + ]) + .unwrap_err(); + + assert_eq!(error, PostgresPoolConfigError::MinConnectionsExceedMax); + } +} + async fn insert_version_row( tx: &mut Transaction<'_, Postgres>, snapshot: &RegistryOperation, diff --git a/docs/runtime-config.md b/docs/runtime-config.md index aa6e0cf..9db13ec 100644 --- a/docs/runtime-config.md +++ b/docs/runtime-config.md @@ -60,6 +60,11 @@ var/crank/ - `POSTGRES_DB` - `POSTGRES_USER` - `POSTGRES_PASSWORD` +- `POSTGRES_MAX_CONNECTIONS` +- `POSTGRES_MIN_CONNECTIONS` +- `POSTGRES_ACQUIRE_TIMEOUT_MS` +- `POSTGRES_IDLE_TIMEOUT_MS` +- `POSTGRES_MAX_LIFETIME_MS` - `CRANK_ADMIN_API_IMAGE` - `CRANK_MCP_SERVER_IMAGE` - `CRANK_UI_IMAGE` @@ -161,6 +166,19 @@ runtime env-переменной. Это значит, что: - `POSTGRES_DB` - `POSTGRES_USER` - `POSTGRES_PASSWORD` +- `POSTGRES_MAX_CONNECTIONS` +- `POSTGRES_MIN_CONNECTIONS` +- `POSTGRES_ACQUIRE_TIMEOUT_MS` +- `POSTGRES_IDLE_TIMEOUT_MS` +- `POSTGRES_MAX_LIFETIME_MS` + +Для pool behavior используются явные defaults: + +- `POSTGRES_MAX_CONNECTIONS=20` +- `POSTGRES_MIN_CONNECTIONS=2` +- `POSTGRES_ACQUIRE_TIMEOUT_MS=5000` +- `POSTGRES_IDLE_TIMEOUT_MS=600000` +- `POSTGRES_MAX_LIFETIME_MS=1800000` `CRANK_DATABASE_URL` допускается только как backward-compatible fallback для локальных тестов и переходного периода, но не как основная deployment-модель.