Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Add/Get New Pooled Connections #212

Closed
tneely opened this issue Jul 26, 2024 · 1 comment · Fixed by #214
Closed

[feature] Add/Get New Pooled Connections #212

tneely opened this issue Jul 26, 2024 · 1 comment · Fixed by #214

Comments

@tneely
Copy link
Contributor

tneely commented Jul 26, 2024

I have a use case where I want to periodically re-evaluate my login credentials against the underlying database.

Using something like Pool::get() doesn't work, because it doesn't necessarily mean that the connection is new - it could be from the pool instead. Luckily, this is pretty trivial today with Pool::dedicated_connection():

let pool = Pool::builder().build(mgr).await.unwrap();
let mut interval = time::interval(time::Duration::from_secs(5));
loop {
  tokio::select! {
    _ = interval.tick() => match pool.dedicated_connection().await {
      Ok(_) => (),
      Err(err) => handle_connection_error(err),
  }
}

However, establishing a connection against my database is expensive, and I don't want to just throw away the connection after its established. Instead, it would be great if that new connection could be added to the pool.

I see two obvious ways of going about this:

  1. A method to add a connection to the pool:
/// Adds a connection to the pool.
pub fn add(&self, conn: M::Connection) -> ()
match pool.dedicated_connection().await {
  Ok(conn) => pool.add(conn),
  Err(err) => handle_connection_error(err),
}
  1. A method to get a new connection from the pool:
/// Retrieves a new connection from the pool.
pub async fn get_new(&self) -> Result<PooledConnection<'_, M>, RunError<M::Error>>
match pool.get_new().await {
  Ok(_pooled_connection) => (), // returned to pool on drop
  Err(err) => handle_connection_error(err),
}

I'm leaning towards the first option, since it's both easier to implement and more flexible. For example, you can create connections independent from the pool and add them to be managed later.

I would be happy to write the code for this feature if it's something you're ok with adding to the project.

@tneely tneely changed the title [feature] Get New Pooled Connections [feature] Add/Get New Pooled Connections Jul 26, 2024
@djc
Copy link
Owner

djc commented Jul 26, 2024

Seems reasonable, happy to review a PR for Pool::add().

tneely added a commit to tneely/bb8 that referenced this issue Jul 27, 2024
Fixes djc#212

This adds `Pool::add`, which allows for externally created
connections to be added and managed by the pool. If the pool
is at maximum capacity when this method is called, it will
return the input connection as part of the Err response.

I considered allowing `Pool:add` to ignore `max_size` when
adding to the pool, but felt it could lead to confusion if
the pool is allowed to exceed its capacity in this specific
case.

This change required making PoolInternals::approvals visible
within the crate to get the approval needed to add a new
connection. The alternative would have required defining a
new pub(crate) method for this specific use case, which feels
worse. I'm open to suggestions on how to more cleanly integrate
this change into the package.
@tneely tneely mentioned this issue Jul 27, 2024
tneely added a commit to tneely/bb8 that referenced this issue Aug 27, 2024
Fixes djc#212

This adds `Pool::add`, which allows for externally created
connections to be added and managed by the pool. If the pool
is at maximum capacity when this method is called, it will
return the input connection as part of the Err response.

I considered allowing `Pool:add` to ignore `max_size` when
adding to the pool, but felt it could lead to confusion if
the pool is allowed to exceed its capacity in this specific
case.

This change required making PoolInternals::approvals visible
within the crate to get the approval needed to add a new
connection. The alternative would have required defining a
new pub(crate) method for this specific use case, which feels
worse. I'm open to suggestions on how to more cleanly integrate
this change into the package.
tneely added a commit to tneely/bb8 that referenced this issue Aug 28, 2024
Fixes djc#212

This adds `Pool::add`, which allows for externally created
connections to be added and managed by the pool. If the pool
is at maximum capacity when this method is called, it will
return the input connection as part of the Err response.

I considered allowing `Pool:add` to ignore `max_size` when
adding to the pool, but felt it could lead to confusion if
the pool is allowed to exceed its capacity in this specific
case.

This change required making PoolInternals::approvals visible
within the crate to get the approval needed to add a new
connection. The alternative would have required defining a
new pub(crate) method for this specific use case, which feels
worse. I'm open to suggestions on how to more cleanly integrate
this change into the package.
djc pushed a commit that referenced this issue Aug 28, 2024
Fixes #212

This adds `Pool::add`, which allows for externally created
connections to be added and managed by the pool. If the pool
is at maximum capacity when this method is called, it will
return the input connection as part of the Err response.

I considered allowing `Pool:add` to ignore `max_size` when
adding to the pool, but felt it could lead to confusion if
the pool is allowed to exceed its capacity in this specific
case.

This change required making PoolInternals::approvals visible
within the crate to get the approval needed to add a new
connection. The alternative would have required defining a
new pub(crate) method for this specific use case, which feels
worse. I'm open to suggestions on how to more cleanly integrate
this change into the package.
djc pushed a commit to tneely/bb8 that referenced this issue Aug 28, 2024
Fixes djc#212

This adds `Pool::add`, which allows for externally created
connections to be added and managed by the pool. If the pool
is at maximum capacity when this method is called, it will
return the input connection as part of the Err response.

I considered allowing `Pool:add` to ignore `max_size` when
adding to the pool, but felt it could lead to confusion if
the pool is allowed to exceed its capacity in this specific
case.

This change required making PoolInternals::approvals visible
within the crate to get the approval needed to add a new
connection. The alternative would have required defining a
new pub(crate) method for this specific use case, which feels
worse. I'm open to suggestions on how to more cleanly integrate
this change into the package.
@djc djc closed this as completed in #214 Aug 28, 2024
@djc djc closed this as completed in 165ca59 Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants