Skip to content

Commit

Permalink
Feat: add task to release unused db conns (#1640)
Browse files Browse the repository at this point in the history
Feat: add task to release unused db conns
  • Loading branch information
taddes authored Dec 5, 2024
1 parent bc79ccb commit c01021b
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 2 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions syncserver/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,13 @@ impl Server {
&Metrics::from(&metrics),
blocking_threadpool.clone(),
)?;
// Spawns sweeper that calls Deadpool `retain` method, clearing unused connections.
db_pool.spawn_sweeper(Duration::from_secs(
settings
.syncstorage
.database_pool_sweeper_task_interval
.into(),
));
let glean_logger = Arc::new(GleanEventsLogger {
// app_id corresponds to probe-scraper entry.
// https://github.com/mozilla/probe-scraper/blob/main/repositories.yaml
Expand Down
14 changes: 14 additions & 0 deletions syncstorage-mysql/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ impl MysqlDbPool {
})
}

/// Spawn a task to periodically evict idle connections. Calls wrapper sweeper fn
/// to use pool.retain, retaining objects only if they are shorter in duration than
/// defined max_idle. Noop for mysql impl.
pub fn spawn_sweeper(&self, _interval: Duration) {
sweeper()
}

pub fn get_sync(&self) -> DbResult<MysqlDb> {
Ok(MysqlDb::new(
self.pool.get()?,
Expand All @@ -110,6 +117,13 @@ impl MysqlDbPool {
}
}

/// Sweeper to retain only the objects specified within the closure.
/// In this context, if a Spanner connection is unutilized, we want it
/// to release the given connections.
/// See: https://docs.rs/deadpool/latest/deadpool/managed/struct.Pool.html#method.retain
/// Noop for mysql impl
fn sweeper() {}

#[async_trait]
impl DbPool for MysqlDbPool {
type Error = DbError;
Expand Down
3 changes: 3 additions & 0 deletions syncstorage-settings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ pub struct Settings {
pub database_pool_connection_lifespan: Option<u32>,
/// Max time a connection should sit idle before being dropped.
pub database_pool_connection_max_idle: Option<u32>,
/// Interval for sweeper task releasing unused connections.
pub database_pool_sweeper_task_interval: u32,
#[cfg(debug_assertions)]
pub database_use_test_transactions: bool,
#[cfg(debug_assertions)]
Expand Down Expand Up @@ -115,6 +117,7 @@ impl Default for Settings {
database_pool_min_idle: None,
database_pool_connection_lifespan: None,
database_pool_connection_max_idle: None,
database_pool_sweeper_task_interval: 30,
database_pool_connection_timeout: Some(30),
#[cfg(debug_assertions)]
database_use_test_transactions: false,
Expand Down
1 change: 1 addition & 0 deletions syncstorage-spanner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ authors.workspace = true
edition.workspace = true

[dependencies]
actix-web.workspace = true
backtrace.workspace = true
cadence.workspace = true
deadpool.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion syncstorage-spanner/src/manager/deadpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::error::DbError;
pub(crate) type Conn = deadpool::managed::Object<SpannerSessionManager>;

pub(crate) struct SpannerSessionManager {
settings: SpannerSessionSettings,
pub settings: SpannerSessionSettings,
/// The gRPC environment
env: Arc<Environment>,
metrics: Metrics,
Expand Down
29 changes: 28 additions & 1 deletion syncstorage-spanner/src/pool.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{collections::HashMap, fmt, sync::Arc, time::Duration};

use actix_web::rt;
use async_trait::async_trait;
use syncserver_common::{BlockingThreadpool, Metrics};
use syncserver_db_common::{GetPoolState, PoolState};
Expand Down Expand Up @@ -49,7 +50,9 @@ impl SpannerDbPool {
let config = deadpool::managed::PoolConfig {
max_size,
timeouts,
..Default::default()
// Prefer LIFO to allow the sweeper task to evict least frequently
// used connections.
queue_mode: deadpool::managed::QueueMode::Lifo,
};
let pool = deadpool::managed::Pool::builder(manager)
.config(config)
Expand Down Expand Up @@ -84,6 +87,30 @@ impl SpannerDbPool {
self.quota,
))
}

/// Spawn a task to periodically evict idle connections. Calls wrapper sweeper fn
/// to use pool.retain, retaining objects only if they are shorter in duration than
/// defined max_idle.
pub fn spawn_sweeper(&self, interval: Duration) {
let Some(max_idle) = self.pool.manager().settings.max_idle else {
return;
};
let pool = self.pool.clone();
rt::spawn(async move {
loop {
sweeper(&pool, Duration::from_secs(max_idle.into()));
rt::time::sleep(interval).await;
}
});
}
}

/// Sweeper to retain only the objects specified within the closure.
/// In this context, if a Spanner connection is unutilized, we want it
/// to release the given connection.
/// See: https://docs.rs/deadpool/latest/deadpool/managed/struct.Pool.html#method.retain
fn sweeper(pool: &deadpool::managed::Pool<SpannerSessionManager>, max_idle: Duration) {
pool.retain(|_, metrics| metrics.last_used() < max_idle);
}

#[async_trait]
Expand Down

0 comments on commit c01021b

Please sign in to comment.