ref(registry): Remove global service registry #2022
Conversation
This PR finalizes migration to service dependencies injection instead of using the global registry. In this part the changes mostly localted in the endpoints, and their dependencies.
jan-auer
changed the title
| @@ -77,7 +78,7 @@ where | |||
| B: axum::body::HttpBody + Send + 'static, | |||
| B::Data: Send, | |||
| B::Error: Into<axum::BoxError>, | |||
| S: Send + Sync, | |||
| S: Send + Sync + ServiceRegistry, | |||
There was a problem hiding this comment.
Instead of this trait, you can make this non-generic and require ServiceState. This is also what we do in a few other extractors.
| @@ -87,6 +90,7 @@ pub fn create_runtime(name: &str, threads: usize) -> Runtime { | |||
| pub struct ServiceState { | |||
| config: Arc<Config>, | |||
There was a problem hiding this comment.
ServiceState is cloned very often by axum, and also from us. Cloning it is somewhat expensive now, since we clone a lot of fields: All the runtimes, and then all fields in the registry. It would now make sense to change the internals of ServiceState to something like:
#[derive(Clone, Debug)]
struct ServiceState {
inner: Arc<StateInner>,
}
impl ServiceState {
// accessor methods for every service here, accessing through `self.inner.*`
}
#[derive(Debug)] // no Clone!
struct StateInner {
// all Addrs here
// all runtimes here
// buffer guard
}
| let project_cache = services.project_cache.clone(); | ||
| let outcome_aggregator = services.outcome_aggregator.clone(); | ||
| let test_store = services.test_store.clone(); |
There was a problem hiding this comment.
I see that we clone all the services. Is it to avoid issues with a mutable reference? Or, why are we doing that?
There was a problem hiding this comment.
The clone is very cheap here and in fact it's all the same objects, which just contains the inner rx channel and the counter for number of messages in the queue. This way each service can own the addresses of the services its communicate with.
This PR finalizes migration to service dependencies injection instead of using the global registry.
In this part the changes mostly located in the endpoints, and their dependencies.
This also allowed to uncomment few
ignoredtests as they are no longer depend on the global registry or all the services.related to: #1818
fixes: #1610