databendlabs / openraft

rust raft with improvements
Apache License 2.0
1.41k stars 158 forks source link

Get rid of dependency of tokio if possible #249

Open drmingdrmer opened 2 years ago

drmingdrmer commented 2 years ago

Get rid of some tokio dependency to make it clear for building a runtime simulator.

Originally posted by @drmingdrmer in https://github.com/datafuselabs/openraft/issues/205#issuecomment-1059000844

github-actions[bot] commented 2 years ago

👋 Thanks for opening this issue!

Get help or engage by:

xanderio commented 2 years ago

tokio::sync::Mutex can be just replaced with std::sync::Mutex. They do not live across any await.

It's not just that the tokio mutex can live across an await, but also that it doesn't block the current thread. Using a std::sync::Mutex in an async environment could cause the runtime to exhausted all this thread.

Another possible benefit of using the tokio specific mutex, is that with the (currently unstable) tokio tracing feature, these mutex contain a tracing::Span.

A possible alternative would be futures::lock::Mutex as this mutex is IIRC runtime agnostic.

See tokio documentation for reference: https://docs.rs/tokio/1.19.2/tokio/sync/struct.Mutex.html

drmingdrmer commented 2 years ago

I did mean not to use tokio::Mutex at all.

I mean somewhere tokio::Mutex is used but actually, a std::Mutex is quite enough.

BTW the span embedded in the tokio sync primitives is really sweet.

schreter commented 2 years ago

Synchronous mutexes are sometimes in fact preferable (they anyway can't be locked over await). I didn't check where we do have mutexes in openraft, but the one for shutdown state, for example, is used very seldom (once when shutting down and once when querying the error), so that's a good fit.

Async mutexes (tokio or not) are typically quire heavy-weight, so depending on what they are protecting, you might end up with worse performance. Especially for uncontended mutexes, a synchronous mutex might be a good alternative. The best, do not use any mutex at all and just use message passing and similar (via queues, for instance).

BTW, there are also implicit synchronous mutexes in memory allocation (which is also one of the reasons to prevent memory allocation wherever possible, since they make it expensive - they are typically well-contended).

Other than that, @xanderio is in principle right - I also advocate against using synchronous primitives (coming from a team where we work on machines with hundreds of cores - that makes any contended synchronous mutex bring the system to a halt).