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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service manager #90

Merged
merged 14 commits into from
Apr 28, 2022
Merged

Service manager #90

merged 14 commits into from
Apr 28, 2022

Conversation

adzialocha
Copy link
Member

@adzialocha adzialocha commented Apr 24, 2022

Introduces ServiceManager which orchestrates long-running concurrent services with a communication bus between them. Also it kindly informs all services about a shutdown and waits for them to stop before quitting the process for good.

The next step will be to a) remove TaskManager and replace it with this in the Runtime b) refactor the GraphQL API to fit the new "service" API and move it into the manager. c) Make use of the axum graceful shutdown feature.

Later we will also add the "Materializer" with the worker as a service here and use the communication bus to inform it about new, incoming entries from the GraphQL API. Much later the Replication service will also live here ..

Closes: #86 and #84

馃搵 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@adzialocha adzialocha marked this pull request as ready for review April 27, 2022 20:19
@sandreae sandreae changed the base branch from main to development April 28, 2022 07:36
Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good to me. One thing I wondered is do we need to be able to get any more info from the service_manager about the state of the spawned services? I tried writing a test which had one service which completed it's job then stops, and I wanted to check the status of all services from the manager side, but this isn't possible now. Maybe it isn't something we need, but I just wondered. I approve of it greatly as it is now in any case, it has a nice simple API :-)

@adzialocha
Copy link
Member Author

Looks really good to me. One thing I wondered is do we need to be able to get any more info from the service_manager about the state of the spawned services? I tried writing a test which had one service which completed it's job then stops, and I wanted to check the status of all services from the manager side, but this isn't possible now. Maybe it isn't something we need, but I just wondered. I approve of it greatly as it is now in any case, it has a nice simple API :-)

Nice, thanks!

The manager will be used for services which will run as long as the whole program runs, at least for now, this is why I didn't add any more functionality in that direction. What would make sense though is to crash the whole program as soon as one service panicks (fitting the "all services or nothing" notion).

I would like to add this though in a more real-life pr, when integrating the http server and materializer, it will also need touches in the worker for propagating up the errors. Think there is an issue for that 馃憤

@adzialocha adzialocha merged commit b969330 into development Apr 28, 2022
@adzialocha adzialocha deleted the service-manager branch April 28, 2022 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Communicate graceful shutdown to all services Message bus for cross-service communication
2 participants