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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate runtime setup in roles #1390

Open
Tracked by #1383
jbesraa opened this issue Jan 23, 2025 · 9 comments
Open
Tracked by #1383

Investigate runtime setup in roles #1390

jbesraa opened this issue Jan 23, 2025 · 9 comments
Assignees
Labels
refactor Implies refactoring code roles Pertains to all roles tokio
Milestone

Comments

@jbesraa
Copy link
Contributor

jbesraa commented Jan 23, 2025

Currently we use tokio::main and as a result the code behaves differently in different environments (test runtime for example). Investigate if we can introduce the runtime in the lib code. If runtime code moves to lib, we could allow users to provide a runtime input in case they are interested in running multiple roles on the same runtime or the lib code can take care of creating its own runtime.

@jbesraa
Copy link
Contributor Author

jbesraa commented Jan 24, 2025

I'll look into this

@jbesraa jbesraa changed the title Invistage the runtime setup in roles Investigate runtime setup in roles Jan 24, 2025
@plebhash plebhash added roles Pertains to all roles refactor Implies refactoring code tokio labels Jan 24, 2025
@jbesraa
Copy link
Contributor Author

jbesraa commented Jan 24, 2025

started with the pool crate as it generally looks good. after completing part of the migration integration tests started failing, so I am doing a bit of adaptation in there while looking into this. badly documented here https://github.com/jbesraa/stratum/pull/41/files

@plebhash
Copy link
Collaborator

plebhash commented Jan 31, 2025

@jbesraa can you elaborate more on the motivations for this investigation?

I assume this is a potential solution to #1266 and #1320, correct?

however, while discussing with @GitGab19 we started to wonder if there's other ways to deal with these problems.

for example, killing roles on ITs could be potentially solved by the shutdown APIs (needed for #1432).

alternatively, the ghost tasks could also be potentially solved by deeper debugging the threading model of the roles (which would allow us to narrow down what is the actual root cause problem)

so while this runtime approach might seem interesting at a first glance, we're wondering if it isn't a one-fits-all solution that could actually be done in more refined alternative approaches?

@jbesraa
Copy link
Contributor Author

jbesraa commented Feb 3, 2025

Shutting down the program does not exit all the running tasks in the background.
You can see how we can utilize tokio channels in order to shutdown multiple tasks running in the background: https://github.com/jbesraa/stratum/pull/44/files#diff-06dae24104fa490a27435dc587613d9fbfb65771d96ed742771b57c35df32f68R51

The main motivation is allowing us to control all async tasks in a more precise way and also allow us to expose sync api and not only async

@GitGab19
Copy link
Collaborator

GitGab19 commented Feb 3, 2025

If that's the reason, I would say that we need to separate more clearly the issues and the goals.

For example, I would open a specific issue for debugging the threading model of the roles.

I don't think that trying to sort out runtime stuff, tokio channels, and threading/task all together is a good idea, and the work on these could be more efficient if properly defined and assigned to multiple people.

What do you think @jbesraa?

@jbesraa
Copy link
Contributor Author

jbesraa commented Feb 3, 2025

Well, the problem is we dont have any threading model currently.

runtime + threading/tasks stuff are all the same. if you look in the link above you can see that instead of doing #[tokio::main] the runtime is started manually and then it is being used to spawn tasks, and also implemented functionality to cancel each task using tokio::select and tokio channels.

Channels are used in order to pass data/state across async jobs while runtime/tasks are responsible for running async jobs.

So currently I dont do any work on the channels, just working on a functionality to allow us to manage the runtime/tasks in a precise way.

@GitGab19
Copy link
Collaborator

GitGab19 commented Feb 3, 2025

I'm pretty sure that what you're doing in the link posted above should be exactly what tokio:main does under the hood, so I can't see how this would change things. When you exit the tokio runtime, in theory, every task spawned should return.

If we're experiencing the issue we're discussing, it probably mean it's something else, otherwise what could it be in your opinion?

we dont have any threading model currently.

That's why I think a better approach would be to deeply dive into how every role currently manages threads. So that we can get a proper overview of the current situation before trying to solve everything with a single shot. And this investigation would be a task to assign to a single person, as I was suggesting above.

@jbesraa
Copy link
Contributor Author

jbesraa commented Feb 3, 2025

I'm pretty sure that what you're doing in the link posted above should be exactly what tokio:main does under the hood, so I can't see how this would change things. (1)When you exit the tokio runtime, in theory, every task spawned should return.

(1) is incorrect - tokio does not guarantee async tasks shutdown when you stop rt.

If we're experiencing the issue we're discussing, it probably mean it's something else, otherwise what could it be in your opinion?

sorry, I dont get what you mean?

That's why I think a better approach would be to deeply dive into how every role currently manages threads. So that we can get a proper overview of the current situation before trying to solve everything with a single shot. And this investigation would be a task to assign to a single person, as I was suggesting above.

as mentioned above, roles dont manage any thread/async tasks - they just spawn them. what kind of overview do you think is missing?
currently I am the only one working on this..

@Shourya742
Copy link
Contributor

I'm pretty sure that what you're doing in the link posted above should be exactly what tokio:main does under the hood, so I can't see how this would change things. (1)When you exit the tokio runtime, in theory, every task spawned should return.

(1) is incorrect - tokio does not guarantee async tasks shutdown when you stop rt.

If we're experiencing the issue we're discussing, it probably mean it's something else, otherwise what could it be in your opinion?

sorry, I dont get what you mean?

That's why I think a better approach would be to deeply dive into how every role currently manages threads. So that we can get a proper overview of the current situation before trying to solve everything with a single shot. And this investigation would be a task to assign to a single person, as I was suggesting above.

as mentioned above, roles dont manage any thread/async tasks - they just spawn them. what kind of overview do you think is missing? currently I am the only one working on this..

I guess, what @GitGab19 meant is the runtime exits when all the task completes.

The reason for runtime not exiting is because the task are blocking the runtime, and because of which .await in spawn_blocking never change to Ready state.

I guess, with thread model he meant how we are managing our tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Implies refactoring code roles Pertains to all roles tokio
Projects
Status: Todo 📝
Development

No branches or pull requests

4 participants