Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .tabularium
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@
"DATETIME": "TIMESTAMP",
"JSON": "JSONB"
},
"settings": [
{
"key": "poolMaxSize",
"label": "Pool Max Size",
"type": "number",
"default": 10,
"description": "Maximum number of PostgreSQL connections kept in the pool."
}
],
"data_types": [
{"name": "SMALLINT", "category": "numeric", "requires_length": false, "requires_precision": false},
{"name": "INTEGER", "category": "numeric", "requires_length": false, "requires_precision": false},
Expand Down
13 changes: 11 additions & 2 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,13 @@ pub fn cleanup_idle_pools() {
/// When `connection_string` is set, it takes precedence over the discrete
/// host/port/database/username/password fields — matching the README's
/// documented behavior ("as an alternative to the discrete fields above").
///
/// Pool max size comes from the `poolMaxSize` setting received in the
/// `initialize` RPC (default 10 — the built-in `postgres` driver's pin,
/// *not* deadpool's `get_default_pool_max_size()`/cpu×2). Matches the
/// built-in's configurable pool size in `pool_manager.rs` (tabularis#681):
/// applied via the builder's `.max_size()`, which overwrites the
/// `PoolConfig` default unconditionally. See `src/settings.rs`.
async fn build_pool(params: &ConnectionParams) -> Result<Pool, String> {
let mut cfg = Config::new();

Expand Down Expand Up @@ -328,7 +335,8 @@ async fn build_pool(params: &ConnectionParams) -> Result<Pool, String> {
let mut builder = cfg
.builder(tls)
.map_err(|e| format!("Pool creation failed (TLS): {e}"))?
.runtime(Runtime::Tokio1);
.runtime(Runtime::Tokio1)
.max_size(crate::settings::pool_max_size());
if let Some(script) = script {
builder = builder.post_create(startup_script_hook(script));
}
Expand All @@ -342,7 +350,8 @@ async fn build_pool(params: &ConnectionParams) -> Result<Pool, String> {
let mut builder = cfg
.builder(NoTls)
.map_err(|e| format!("Pool creation failed: {e}"))?
.runtime(Runtime::Tokio1);
.runtime(Runtime::Tokio1)
.max_size(crate::settings::pool_max_size());
if let Some(script) = script {
builder = builder.post_create(startup_script_hook(script));
}
Expand Down
81 changes: 80 additions & 1 deletion src/client_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
use tokio::sync::Mutex;

use super::{
build_tls_connector, cleanup_idle_pools, connection_key, get_or_create_pool,
build_pool_pub, build_tls_connector, cleanup_idle_pools, connection_key, get_or_create_pool,
load_client_cert_from_pem, load_roots_from_pem, resolve_ssl_mode, NoCertVerifier,
VerifyCaCertVerifier, POOLS,
};
use crate::models::ConnectionParams;
use crate::settings;
use deadpool_postgres::SslMode;

// `POOLS` is a single process-wide static, and Rust's test harness runs
Expand Down Expand Up @@ -687,3 +688,81 @@ fn build_tls_connector_verify_ca_without_ssl_ca_returns_a_clear_error() {
"unexpected error message: {err}"
);
}

// Coverage for #61: `build_pool` previously never set `max_size`, so deadpool
// fell back to `PoolConfig::default()` = `get_default_pool_max_size()` =
// logical_cores × 2 (up to ~32 on a 16-thread machine) — up to ~3× more
// backend connections per target than the built-in's pinned 10. The parity
// suite doesn't catch this (82 tests compare query results, not connection
// counts). These tests close that gap by inspecting the built pool's
// `status().max_size` directly.
//
// deadpool's `Pool::new` is lazy (no connection at creation time when no
// startup script is set), so a fake host is enough — no live DB needed, and
// these run in CI's ordinary Test job. `build_pool_pub` calls
// `get_or_create_pool`, which caches into the shared `POOLS` static, so each
// test holds `POOLS_TEST_LOCK` (to serialize with the cache-count tests
// above) AND `POOL_MAX_SIZE_TEST_LOCK` (to serialize with the `settings`
// tests), then resets the pool-size global to the default first. Two locks
// are acquired settings-first; order is consistent with `settings_tests`.

#[tokio::test]
async fn build_pool_default_max_size_is_ten_not_cpu_times_two() {
let _settings_guard = settings::test_support::lock_and_reset().await;
let _pools_guard = POOLS_TEST_LOCK.lock().await;
settings::set_pool_max_size(&serde_json::json!({ "poolMaxSize": 10 }));
let p = params("pool-size-default-test-host", 5432, "db", "user");
let pool = build_pool_pub(&p)
.await
.expect("lazy pool builds without a live DB when no startup script is set");
let status = pool.status();
assert_eq!(
status.max_size,
10,
"default pool max size must be the built-in's pinned 10 (parity), not \
deadpool's cpu×2 default ({} on this machine)",
num_cpus_from_deadpool_default(),
);
}

#[tokio::test]
async fn build_pool_honors_custom_pool_max_size_setting() {
let _settings_guard = settings::test_support::lock_and_reset().await;
let _pools_guard = POOLS_TEST_LOCK.lock().await;
settings::set_pool_max_size(&serde_json::json!({ "poolMaxSize": 3 }));
let p = params("pool-size-custom-test-host", 5432, "db", "user");
let pool = build_pool_pub(&p)
.await
.expect("lazy pool builds without a live DB when no startup script is set");
assert_eq!(
pool.status().max_size,
3,
"a poolMaxSize of 3 must be applied to the built pool's max_size"
);
}

#[tokio::test]
async fn build_pool_clamps_oversized_pool_max_size_to_cap() {
let _settings_guard = settings::test_support::lock_and_reset().await;
let _pools_guard = POOLS_TEST_LOCK.lock().await;
settings::set_pool_max_size(&serde_json::json!({ "poolMaxSize": 10_000 }));
let p = params("pool-size-cap-test-host", 5432, "db", "user");
let pool = build_pool_pub(&p)
.await
.expect("lazy pool builds without a live DB when no startup script is set");
assert_eq!(
pool.status().max_size,
64,
"an oversized poolMaxSize must be clamped to the 64 cap before the pool is built"
);
}

/// What deadpool's cpu×2 default *would* be on this machine — only used in
/// the failure message above, never as an assertion (the test must pass on
/// any core count). Computed from the live `num_cpus` rather than a constant
/// so the diagnostic stays accurate across CI runners.
fn num_cpus_from_deadpool_default() -> usize {
std::thread::available_parallelism()
.map(|n| n.get() * 2)
.unwrap_or(20)
}
13 changes: 11 additions & 2 deletions src/handlers/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,18 @@ use serde_json::Value;
use crate::client;
use crate::models::{inner_params, ConnectionParams};
use crate::rpc::{error_response, ok_response};
use crate::settings;

/// Receive plugin settings from the host. Currently a no-op.
pub async fn initialize(id: Value, _params: &Value) -> Value {
/// Receive plugin settings from the host. The host sends
/// `json!({ "settings": settings })` (a `HashMap<String, Value>` built from
/// this plugin's `.tabularium` setting definitions — see `RpcDriver::new` in
/// `tabularis/src-tauri/src/plugins/driver.rs`) and silently ignores any
/// error or non-response, so this must never panic. Currently the only
/// setting is `poolMaxSize`; an absent/invalid value falls back to the
/// built-in's default (10) inside the parser.
pub async fn initialize(id: Value, params: &Value) -> Value {
let settings_value = params.get("settings").cloned().unwrap_or(Value::Null);
settings::set_pool_max_size(&settings_value);
ok_response(id, Value::Null)
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ mod extract_tests;
pub mod handlers;
pub mod models;
pub mod rpc;
pub mod settings;
pub mod utils;
137 changes: 137 additions & 0 deletions src/settings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
//! Plugin settings received from the host via the `initialize` RPC.
//!
//! The host sends `initialize` with `json!({ "settings": settings })`, where
//! `settings` is a `HashMap<String, serde_json::Value>` built from the
//! plugin's declared `.tabularium` setting definitions (see
//! `RpcDriver::new` in `tabularis/src-tauri/src/plugins/driver.rs`). The host
//! silently ignores any `initialize` error or non-response, so parsing here
//! must never panic — an invalid value falls back to a safe default rather
//! than killing the handshake.
//!
//! # poolMaxSize
//!
//! Mirrors the built-in `postgres` driver's configurable pool size
//! (tabularis#681): `postgres_pool_max_size_from_value` in
//! `src-tauri/src/pool_manager.rs`. Parsed from u64/i64/string, zero/invalid
//! → default, capped at 64. Defaults to **10** — the built-in's pin — rather
//! than deadpool's `get_default_pool_max_size()` (cpu×2), restoring parity
//! for the pgBouncer use case (tabularis#71) where a small client pool is
//! essential.

use std::sync::Mutex;

/// The built-in `postgres` driver's pinned pool size, and this plugin's
/// default when `poolMaxSize` is absent/invalid. Matches
/// `DEFAULT_POSTGRES_POOL_MAX_SIZE` in `tabularis/src-tauri/src/pool_manager.rs`.
pub(crate) const DEFAULT_POOL_MAX_SIZE: usize = 10;

/// Upper bound on a user-supplied pool size — matches
/// `MAX_POSTGRES_POOL_MAX_SIZE` in `tabularis/src-tauri/src/pool_manager.rs`.
/// Caps a wildly oversized setting (e.g. 10_000) at a sane ceiling rather
/// than letting one connection target exhaust server/backend slots. This is
/// the defense-in-depth bound: no matter how many times `initialize` runs
/// or what value arrives, `pool_max_size()` can never exceed 64.
const MAX_POOL_MAX_SIZE: usize = 64;

/// Process-wide pool max size, set from the `initialize` RPC and read by
/// every `build_pool` call. `Mutex` (not `OnceLock`): the value reflects the
/// *most recent* `initialize`, matching the built-in driver's behavior of
/// reading the current config value on each pool build
/// (`get_cached_config()` in `pool_manager.rs`). The host sends
/// `initialize` exactly once at startup, so in practice this is set once —
/// but a corrected re-init must not be ignored, and `Mutex` keeps the
/// parse/clamp logic testable without process-global ordering hazards.
/// Initialized to the default so pools are correctly sized even if
/// `initialize` never arrives or is silently dropped by the host.
static POOL_MAX_SIZE: Mutex<usize> = Mutex::new(DEFAULT_POOL_MAX_SIZE);

/// Parse a `poolMaxSize` setting value into a validated pool size, ported
/// verbatim from the built-in driver's `postgres_pool_max_size_from_value`
/// (`tabularis/src-tauri/src/pool_manager.rs`).
///
/// Accepts a u64, an i64 ≥ 0, or a decimal-string parsable as u64. Zero and
/// any non-parseable value (null, bool, object, negative, garbage string)
/// fall back to [`DEFAULT_POOL_MAX_SIZE`]. Any value above
/// [`MAX_POOL_MAX_SIZE`] is clamped down to it. The ordering of the
/// `or_else` chain matters: `as_u64` is tried first (the common JSON-number
/// path), then a non-negative `as_i64`, then a string parse.
pub(crate) fn pool_max_size_from_value(value: Option<&serde_json::Value>) -> usize {
value
.and_then(|value| {
value
.as_u64()
.or_else(|| value.as_i64().and_then(|item| u64::try_from(item).ok()))
.or_else(|| value.as_str().and_then(|item| item.parse::<u64>().ok()))
})
.and_then(|value| usize::try_from(value).ok())
.filter(|value| *value > 0)
.map(|value| value.min(MAX_POOL_MAX_SIZE))
.unwrap_or(DEFAULT_POOL_MAX_SIZE)
}

/// Store the parsed pool max size from the `initialize` RPC. Overwrites any
/// prior value — the built-in driver reads the current config on each pool
/// build, and the plugin's analog is "most recent `initialize` wins".
/// Falls back to [`DEFAULT_POOL_MAX_SIZE`] when the setting is absent, so
/// the feature degrades gracefully to parity with the built-in even if the
/// host sends no settings. The value is clamped to [`MAX_POOL_MAX_SIZE`]
/// before storage, so a poisoned/oversized input can never be retained.
pub(crate) fn set_pool_max_size(settings: &serde_json::Value) {
let size = pool_max_size_from_value(settings.get("poolMaxSize"));
if let Ok(mut guard) = POOL_MAX_SIZE.lock() {
*guard = size;
}
}

/// The pool max size to apply when building a pool. Returns the value set
/// by the most recent `initialize`, otherwise [`DEFAULT_POOL_MAX_SIZE`].
/// This is the single read-side entry point for `build_pool`. A poisoned
/// lock (impossible in practice — `set_pool_max_size` cannot panic while
/// holding the guard, since `pool_max_size_from_value` is infallible) falls
/// back to the default rather than propagating an error.
pub(crate) fn pool_max_size() -> usize {
POOL_MAX_SIZE
.lock()
.map(|guard| *guard)
.unwrap_or(DEFAULT_POOL_MAX_SIZE)
}

#[cfg(test)]
#[path = "settings_tests.rs"]
mod tests;

#[cfg(test)]
pub(crate) mod test_support {
use super::{set_pool_max_size, DEFAULT_POOL_MAX_SIZE};
use serde_json::json;
use tokio::sync::Mutex;

/// Single shared lock across every test that touches the process-global
/// `POOL_MAX_SIZE` — both `settings_tests` (here) and the `build_pool`
/// max-size tests in `client_tests.rs`. Without a shared lock, the two
/// test modules' concurrent `set_pool_max_size` calls interleave and a
/// test observes another's value (nondeterministic failures). Async
/// (`tokio::sync`) so the `client_tests` `#[tokio::test]`s can hold the
/// guard across their `.await` on `build_pool_pub` without tripping
/// clippy's `await_holding_lock` (which a std `Mutex` guard would).
pub(crate) static POOL_MAX_SIZE_TEST_LOCK: Mutex<()> = Mutex::const_new(());

/// Reset the global to the default under the shared lock and yield the
/// guard, for use in `#[tokio::test]`s that hold it across `.await`.
pub(crate) async fn lock_and_reset() -> tokio::sync::MutexGuard<'static, ()> {
let guard = POOL_MAX_SIZE_TEST_LOCK.lock().await;
set_pool_max_size(&json!({ "poolMaxSize": DEFAULT_POOL_MAX_SIZE }));
guard
}

/// Blocking variant for sync `#[test]`s (the storage tests in
/// `settings_tests.rs` don't `.await`, so they use this). Acquires the
/// same shared lock via `blocking_lock`, which is valid here because
/// those tests run on the harness thread rather than inside a runtime
/// task (which is what makes `blocking_lock` a misuse in general).
pub(crate) fn lock_and_reset_blocking() -> tokio::sync::MutexGuard<'static, ()> {
let guard = POOL_MAX_SIZE_TEST_LOCK.blocking_lock();
set_pool_max_size(&json!({ "poolMaxSize": DEFAULT_POOL_MAX_SIZE }));
guard
}
}
Loading
Loading