axum icon indicating copy to clipboard operation
axum copied to clipboard

Should we require `Sync` for services?

Open davidpdrsn opened this issue 2 years ago • 6 comments

See https://github.com/tokio-rs/axum/pull/1477 for more context.

davidpdrsn avatar Apr 21 '23 15:04 davidpdrsn

Hey - just wanted to check in: Is this still relevant? Do we only need Sync if services should work with work-stealing executors? Otherwise, it's not needed, right?

rakshith-ravi avatar Sep 29 '23 18:09 rakshith-ravi

It’s still needed. Router is not Sync because it contains services that aren’t Sync. That limits our options to use things like Arc internally.

So it’s not related to work stealing.

davidpdrsn avatar Sep 29 '23 19:09 davidpdrsn

Considering that once things are setup, we only use the router for reading (routing), what's wrong with using Arc internally? Sure, it incurs a cost of setting things up, but an initial cost shouldn't be a big deal when compared to the fact that once things are setup, when the server is running, we don't lose out on performance.

I went through the linked issue and I can't help but feel like requiring Sync would impact the kinds of services that can be used (although I'm super curious on what service was causing issues at Embark though, but I guess that's probably proprietary stuff). Anyways, what're we losing out on by using Arcs internally?

rakshith-ravi avatar Sep 29 '23 19:09 rakshith-ravi

Considering that once things are setup, we only use the router for reading (routing), what's wrong with using Arc internally?

Service::call takes &mut self so it’s not just for reading.

Anyways, what're we losing out on by using Arcs internally?

Not using Arc makes the router more expensive to Clone. It’s is cloned once per new connection.

davidpdrsn avatar Sep 29 '23 20:09 davidpdrsn

Been mulling over this for a few days, and had an idea. How about we wrap everything in an Arc, and when a particular service needs to be called, we clone that particular service alone, and since we would have ownership of that service, we can call it with the &mut self.

Sorry if these suggestions seems stupid, but I'm not the most well-versed with axum internals. I'm just randomly throwing ideas in hopes of seeing if something sticks

rakshith-ravi avatar Oct 01 '23 20:10 rakshith-ravi

Been mulling over this for a few days, and had an idea. How about we wrap everything in an Arc, and when a particular service needs to be called, we clone that particular service alone, and since we would have ownership of that service, we can call it with the &mut self.

That won't work. For Arc<T> to be Send, T must be Sync and axum's Router and futures need to be Send.

davidpdrsn avatar Oct 02 '23 06:10 davidpdrsn