fix: move I/O calls to blocking threadpool #1190
Conversation
| @@ -36,7 +36,7 @@ docker_stop_spanner: | |||
| docker-compose -f docker-compose.spanner.yaml down | |||
|
|
|||
| run: | |||
| RUST_LOG=debug RUST_BACKTRACE=full cargo run -- --config config/local.toml --features tokenserver_test_mode | |||
| RUST_LOG=debug RUST_BACKTRACE=full cargo run --features tokenserver_test_mode -- --config config/local.toml | |||
There was a problem hiding this comment.
| &self, | ||
| params: params::GetOrCreateUser, | ||
| ) -> DbResult<results::GetOrCreateUser> { | ||
| let mut raw_users = self.get_users_sync(params::GetUsers { |
There was a problem hiding this comment.
I couldn't figure out how to have an async get_or_create_users method that runs on the blocking threadpool, since defining that method on the trait would result in the the method body running on an actix worker thread (though the database calls would still run on the blocking threadpool). This seemed preferable given those two options, but this method as it's defined will block a thread for quite a long time, since there are no awaits. Definitely open to suggestions here
There was a problem hiding this comment.
Hashing a few things out, so my apologies in advance.
There are two blocking operations in this thread, both of which potentially block the Actix event loop. get_users_sync, and allocate_user_sync, both of which use the dbpool which runs on it's own system thread. allocate_user_sync does three operations, while get_users_sync just does one GET. If we wanted to free up the actix event loop during those db calls, we could probably wrap those in futures.
We might want to just put some profile metrics on allocate_user_sync to see what sort of delays it introduces before we optimize things.
There was a problem hiding this comment.
It could potentially be async, since it no longer has any direct calls to diesel. From an async block it'd call the async versions of the individual db calls it makes -- it looks like it has 3 of them -- each would end up separately/individually called via the thread pool.
I'm not terribly concerned about the way it is currently but that's always an option.
Description
Moves blocking I/O method calls to the blocking threadpool.
Testing
Running the load test suite against Tokenserver with 100 users should result in no errors.
Issue(s)
Closes #1188