From 2c132e45cb624a39ac7f23ea78f082078277a450 Mon Sep 17 00:00:00 2001 From: Conrad Ludgate Date: Fri, 8 Mar 2024 07:56:23 +0000 Subject: [PATCH] proxy: do not store ephemeral endpoints in http pool (#6819) ## Problem For the ephemeral endpoint feature, it's not really too helpful to keep them around in the connection pool. This isn't really pressing but I think it's still a bit better this way. ## Summary of changes Add `is_ephemeral` function to `NeonOptions`. Allow `serverless::ConnInfo::endpoint_cache_key()` to return an `Option`. Handle that option appropriately --- proxy/src/proxy.rs | 5 +++++ proxy/src/serverless/conn_pool.rs | 30 +++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/proxy/src/proxy.rs b/proxy/src/proxy.rs index aeba08bc4f..7848fc2ac2 100644 --- a/proxy/src/proxy.rs +++ b/proxy/src/proxy.rs @@ -380,6 +380,11 @@ impl NeonOptions { Self::parse_from_iter(StartupMessageParams::parse_options_raw(options)) } + pub fn is_ephemeral(&self) -> bool { + // Currently, neon endpoint options are all reserved for ephemeral endpoints. + !self.0.is_empty() + } + fn parse_from_iter<'a>(options: impl Iterator) -> Self { let mut options = options .filter_map(neon_option) diff --git a/proxy/src/serverless/conn_pool.rs b/proxy/src/serverless/conn_pool.rs index 7d705ba049..73f213d074 100644 --- a/proxy/src/serverless/conn_pool.rs +++ b/proxy/src/serverless/conn_pool.rs @@ -43,8 +43,13 @@ impl ConnInfo { (self.dbname.clone(), self.user_info.user.clone()) } - pub fn endpoint_cache_key(&self) -> EndpointCacheKey { - self.user_info.endpoint_cache_key() + pub fn endpoint_cache_key(&self) -> Option { + // We don't want to cache http connections for ephemeral endpoints. + if self.user_info.options.is_ephemeral() { + None + } else { + Some(self.user_info.endpoint_cache_key()) + } } } @@ -360,8 +365,11 @@ impl GlobalConnPool { conn_info: &ConnInfo, ) -> Result>, HttpConnError> { let mut client: Option> = None; + let Some(endpoint) = conn_info.endpoint_cache_key() else { + return Ok(None); + }; - let endpoint_pool = self.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key()); + let endpoint_pool = self.get_or_create_endpoint_pool(&endpoint); if let Some(entry) = endpoint_pool .write() .get_conn_entry(conn_info.db_and_user()) @@ -455,8 +463,10 @@ pub fn poll_client( span.in_scope(|| { info!(%conn_info, %session_id, "new connection"); }); - let pool = - Arc::downgrade(&global_pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key())); + let pool = match conn_info.endpoint_cache_key() { + Some(endpoint) => Arc::downgrade(&global_pool.get_or_create_endpoint_pool(&endpoint)), + None => Weak::new(), + }; let pool_clone = pool.clone(); let db_user = conn_info.db_and_user(); @@ -723,8 +733,9 @@ mod tests { dbname: "dbname".into(), password: "password".as_bytes().into(), }; - let ep_pool = - Arc::downgrade(&pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key())); + let ep_pool = Arc::downgrade( + &pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key().unwrap()), + ); { let mut client = Client::new(create_inner(), conn_info.clone(), ep_pool.clone()); assert_eq!(0, pool.get_global_connections_count()); @@ -780,8 +791,9 @@ mod tests { dbname: "dbname".into(), password: "password".as_bytes().into(), }; - let ep_pool = - Arc::downgrade(&pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key())); + let ep_pool = Arc::downgrade( + &pool.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key().unwrap()), + ); { let mut client = Client::new(create_inner(), conn_info.clone(), ep_pool.clone()); client.do_drop().unwrap()();