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

Tracking Issue for Asynchronous Database Pool Support #1117

Closed
githubaccount624 opened this issue Sep 4, 2019 · 23 comments
Closed

Tracking Issue for Asynchronous Database Pool Support #1117

githubaccount624 opened this issue Sep 4, 2019 · 23 comments
Labels
request Request for new functionality

Comments

@githubaccount624
Copy link

Why?

Creating a database connection can be an expensive operation. Connection pools can alleviate this and serve as an important performance optimization for web servers. The current connection pool in Rocket is synchronous. It would be desirable to have support for an asynchronous connection pool in the up-and-coming async Rocket.

How?

jebrosen elaborates in this issue reply. One of the main blockers is that Rocket and the database engine should first run within the same runtime (tokio 0.1 or tokio 0.2). This seems partially resolved as the async branch of tokio-postgres and Rocket are both currently using tokio=0.2.0-alpha.4.

Jeb's second point is that connect may be an asynchronous operation and whether or not FromRequest should return a Future. Connecting to a database can sometimes take 50+ milliseconds and tokio-postgres has already made connect an asynchronous operation, so it seems like a safe bet to accommodate this fact.

I'll just paste the rest of Jeb's minimum required tasks for reference:

  • Create a connection pool with tokio-postgres and/or bb8 and put it in managed state. This should tell us if on_attach and on_launch need to be async-capable.
  • Create a request guard type wrapping a connection (or a future that will resolve to a connection). This and the previous bullet point together are more or less what the guide demonstrated for databases in Rocket 0.3 before we had rocket_contrib::databases.
  • Use said connection via the request guard within a route to query/alter the database. This should work fine, because routes can now be async fn.
  • Test, figure out, and document cross-runtime compatibility (or lack thereof). That is, a definitive statement in the form of "Rocket uses tokio 0.2, so your database futures have to as well" if that's the case. Alternatively, maybe we could find a proof of concept of running two runtimes - tokio 0.2 (for rocket) alongside tokio 0.1 (for tokio-postgres) with some kind of message passing between them. I leave this intentionally vague because it's an annoying solution to an annoying problem and someone other than I will probably think of a better idea.
@jebrosen
Copy link
Collaborator

jebrosen commented Sep 5, 2019

Here's an updated list of things I think we would need to address in order to fully support async database pools. Importantly, I think it's not yet possible after all to set up a pool "the manual way" because some of Rocket's traits--such as FromRequest--are still synchronous.

  • We will want to keep the existing support for synchronous databases, simply because I doubt the entire database ecosystem will become async in time. We may need to create two sets of database macros and traits (e.g. AsyncPoolable and #[async_database] or #[database(async)])
  • bb8 is the async equivalent of r2d2. The latest release uses tokio-timer 0.2, which may make it incompatible with the more recent tokio runtime that Rocket's async branch uses. Migrate to (std::)futures 0.3 and tokio 0.2 djc/bb8#31 is the tracking issue for using the async-compatible Future and tokio versions.
  • Asynchronous pool initialization and initial connections. To do this asynchronously, Fairing::on_launch would need to become asynchronous and be run after starting the runtime (it is currently done synchronously before the runtime exists). Changing this is going to be relatively difficult.
  • Asynchronous connection acquisition. This would require FromRequest to become asynchronous, or perhaps a new trait such as AsyncFromRequest and blanket impl one for the other.

Some of these are more general changes than just databases that we will want to do anyway to support other use cases.

@jebrosen jebrosen added the request Request for new functionality label Sep 5, 2019
@jebrosen
Copy link
Collaborator

  • We will want to keep the existing support for synchronous databases

This might turn out to be pretty tricky. Doing this safely requires either marshaling certain requests to a thread pool or using executor-specific functionality like tokio_executor::blocking::run. In particular, retrieving a connection from a pool could stall the executor under contention. I'm not sure if using a connection could also stall the executor, and that might depend on the database engine being used.

djc/bb8#37 was just opened, which would make bb8 compatible with tokio 0.2. However, there are barely any databases that have bb8 adapters as it is and those would also need to be upgraded - postgres and redis, compared to the ten or so databases in rocket_contrib today.

@pimeys
Copy link

pimeys commented Oct 16, 2019

Hey, I just found this and it seems to give a pretty cool r2d2 style API for async connections. Works with async/await.

https://github.com/animalsiknow/tokio-resource-pool

Here's my example project where I tested it and seems to work:

https://github.com/pimeys/async-query

@pimeys
Copy link

pimeys commented Oct 16, 2019

And btw. please be careful with blocking::run. At least in the current alphas it limits the number of threads to 1000 and it is not controllable through the Runtime builder. There's a separate tokio_executor::threadpool::blocking that has better guarantees and control in this use case where we might not want to spawn 1000 threads for database access (tip: performance gets quite bad when you go over the magical limit of num_physical_cpus * 2 + magic_number, especially with hundreds of threads.

@jebrosen
Copy link
Collaborator

tokio-resource-pool looks perfect as an async version of the "do it yourself" database system described in the 0.3 guide. I will also take a look and see if building databases on top of that might be a reasonable alternative to bb8.

threadpool::blocking is the one I meant, sorry for the confusion.

@pimeys
Copy link

pimeys commented Oct 16, 2019

Yeah, we're needing similar crates here in Prisma as you do in Rocket. I almost coughed my coffee to my keyboard when I found out that tokio-resource-pool, and remembered how many times I've been in this issue trying to find out if the Rocket devs have found something already! It's a damn nice API, although the Manage trait can be a bit confusing to write at first. See my example repo for how to use it with MySQL/PostgreSQL/Sqlite.

Moving ahead and using these primitives to make our dynamic query builder and connector interface async on this week!

@fakeshadow
Copy link

I have a personal use connection pool. It's basically a simplified bb8 and l337 with async/await enabled. The repo is:

https://github.com/fakeshadow/tang_rs

I'm not sure if it's in good quality though as I don't have any tests and I'm the only one who use it.

@jebrosen
Copy link
Collaborator

FromRequestAsync has now been added, so it is possible to do asynchronous connection acquisition in a request guard. There are no asynchronous attach fairings yet (it requires significant redesign of a few things), so workarounds as in https://github.com/fakeshadow/tang_rs/blob/20ba7db4bf8ff2e2e6e8d810ae72a80d11ed9978/examples/rocket/src/main.rs#L31 are still necessary to set up the pool in the first place.

@jebrosen
Copy link
Collaborator

I got around to looking at a few APIs more closely:

  • bb8 is not compatible with the latest futures and tokio alphas (yet). Its API is actually fairly different from r2d2: instead of a pool.get() method that returns a pool, you instead call something like pool.run(|conn| /* use conn */). Making it work like the rocket_contrib API with request guards would be pretty involved but maybe not impossible.
  • tokio-resource-pool has an alpha release compatible with the latest futures and tokio alphas. It should be able to support a request guard style API like the one in rocket_contrib. However, some of the documentation needs some work - in particular, it's unclear what a correct implementation of recycle() should actually do.
  • tang looks like about the same kind of API as tokio-resource-pool

bb8's API may end up being the most straightforward for manually created connection pools, because it's easier to implement bb8::ManageConnection than tokio_resource_pool::Manage. tokio-resource-pool shows the most promise right now for an API using request guards for connections.

@githubaccount624
Copy link
Author

@fakeshadow Hi your library looks very well-developed! I'm not too sure on whether to use yours or tokio-resource-pool though. Could you elaborate on the differences and tradeoffs? Not sure which one would be better.

I'm using postgres with Rocket and would like to just call something like grab_connection_from_pool at the start of all my route handlers and use that connection for the duration of the function.

@fakeshadow
Copy link

fakeshadow commented Oct 23, 2019

@githubaccount624 I have not used tokio-resource-pool so I could be wrong but there are only small differences from what I see. tang is very simple and no channel used, no atomic magic used except an Arc. It's basically just a future aware Mutex from the async-std crate with a pool of connections inside.
I can't think of any trade off in any of the tokio-postgres connection pools including bb8 and l3-37 because most of the overhead comes from your actual SQL operations and not the pool itself.
I would assume bb8 and l3-37 are better when they migrate to std-future, only because they both are relative popular so they are more likely to be proven solutions.

@jebrosen
Copy link
Collaborator

jebrosen commented Nov 25, 2019

Here are the async-compatible connection pools I'm currently aware of, along with a description of whether each could be used in rocket_contrib to implement a similar API to #[database] today. I'm applying the following criteria:

  • The API uses a guard type
  • Futures 0.3 and/or tokio 0.2 is used
  • No new unstable (nightly) features are used
  • Broken connections and/or timeouts can be handled

Last updated 2020-01-25

Crate Version Usable in rocket_contrib?
bb8 0.4 Yes
mobc 0.5 Yes
deadpool 0.5 Yes
tang master Maybe; feature flag selection to choose between tokio 0.1 or tokio 0.2 is strange
tokio-resource-pool 0.5.0-alpha.1 No; uses nightly feature associated_type_defaults. Targets an alpha of tokio.
async-pool master No; can not recycle connections (khionu/async_pool#3)

@viniciusd
Copy link

Not sure how much that's useful, but djc/bb8#37 has just got merged, which brings async/await to bb8

@bikeshedder
Copy link

I'm currently reasoning if Pool::get_timeout is something that should be part of deadpool or not: bikeshedder/deadpool#9 - Feedback to this issue would be highly appreciated.

@jebrosen
Copy link
Collaborator

jebrosen commented Dec 4, 2019

You make a good point in that issue - it should be easy (especially now with async) to implement a configurable timeout directly in rocket_contrib anyway.

@bikeshedder
Copy link

Deadpool 0.4 now supports timeouts:

use deadpool::{Config, Timeouts};
use deadpool_postgres::Pool;

let pool_config = Config::new(16);
pool_config.timeouts = Timeouts::wait_millis(2000);
let pool = Pool::from_config(pool_config);

I added this to deadpool as it is the only way to differentiate the timeouts for wait, recycle and create: See https://docs.rs/deadpool/0.4.3/deadpool/struct.Timeouts.html

@djc
Copy link

djc commented Jan 21, 2020

bb8 now has a released 0.4.0 version -- I am the new maintainer.

@jebrosen
Copy link
Collaborator

Status update: everything exists in rocket's async branch that is necessary to implement asynchronous pools, and it's fairly straightforward to port #[database] for async - I've approximated it in one of my projects with 70 lines of macro_rules!, for example.

bb8, deadpool, and mobc all look like reasonable choices and have pretty similar APIs; however they only support postgres, redis, and one or two other miscellaneous databases. I am hesitant to commit to any particular integration in rocket_contrib until a few more adapters exist, e.g. for async clients for mysql and memcache.

@Songtronix
Copy link

sqlx has recently added SQlite support which means it supports PostgreSQL, MySQL, and SQLite. It's runtime agnostic, supports connection pools and optionally you can have type checked queries without buying into a DSL.

@Razican
Copy link
Contributor

Razican commented Apr 13, 2020

It would be nice to be able to use SQLx in rocket as a backend, yes, since currently Diesel seems to be the backend of choice. Not sure though if we could use the pool as the default pool, though, as it only supports 3 backends.

@jebrosen
Copy link
Collaborator

My thinking is that rocket_contrib will probably not use sqlx to back an #[async_database] attribute for two reasons:

  • The pool is specific to sqlx, so it can't be used with e.g. redis or memcache clients
  • sqlx's compile-time checking only appears to work with a single database at a time (due to using the DATABASE_ENV variable at compile-time), but a large part of the purpose of #[database] is to use multiple databases at the same time

That being said, sqlx appears almost as easy as deadpool to integrate yourself with managed state already - see also https://www.reddit.com/r/rust/comments/e9n6mx/creating_and_using_a_postgres_database_pool_is/.


Alternatively, perhaps connection pooling in contrib could be extended to multiple support different pool implementations instead of choosing one. That would also allow better integration of crates such as mongodb that unconditionally implement their own connection pooling.

@SergioBenitez
Copy link
Member

@jebrosen Give that you've done some work in the background on this issue, do you have any more insight into how we might proceed?

@jebrosen
Copy link
Collaborator

jebrosen commented Nov 3, 2020

I've been working on this proof-of-concept I called rocket-database-pool (name TBD), just updated recently for figment.

The high-level interface is described at https://git.jebrosen.com/jeb/sirus/src/tag/rocket-database-pool-20201102/rocket-database-pool/src/lib.rs#L370-L405, reproduced here:

rocket_database_pool::pool!("name" => POOL of CONN(TYPE));

Generates roughly the following code:

// A struct containing the connection pool.
pub struct POOL(/* secret sauce */);

impl POOL {
    // A fairing that attaches this connection pool to the server.
    pub fn fairing() -> rocket_database_pool::Fairing;

    // pool.get().await returns a connection from the pool (or an error)
    pub async fn get(&self) -> Result<CONN, <<TYPE as Poolable>::Pool as Pool<TYPE>>::Error> { }

    // pool.inner() and pool.inner_mut() allow direct access
    // to the underlying pool.
    pub fn inner(&self) -> &<TYPE as Poolable>::Pool { }
    pub fn inner_mut(&mut self) -> &mut <TYPE as Poolable>::Pool { }
}

// A struct containing a connection from the pool. It is guaranteed
// to be a tuple struct with one field.
pub struct CONN(TYPE);

// CONN implements Deref and DerefMut
impl Deref for CONN { type Target = TYPE; }
impl DerefMut for CONN { }

// CONN implements FromRequest
impl<'a, 'r> FromRequest<'a, 'r> for CONN { }

It's more or less the same as rocket_contrib's database support:

However I'm of two minds on most of the design and have been rewriting something about it every few weeks, such as:

  • Whether or not it's worth it to have the separate Pool trait
  • on_init, on_connect hooks and what guarantees they make
  • error types

I think the latest updates for configuration was the last "known big API change" on my list, so the next few weeks would be a perfect time to start making some of those decisions, getting more eyeballs on the code, and deciding on whether it or something similar should go in rocket_contrib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants