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

Add stop function for roles #1148

Closed
jbesraa opened this issue Aug 27, 2024 · 6 comments
Closed

Add stop function for roles #1148

jbesraa opened this issue Aug 27, 2024 · 6 comments

Comments

@jbesraa
Copy link
Contributor

jbesraa commented Aug 27, 2024

After merging #1092 #1095 and #1097 we would like to start completing #1066. One requirement we need is a function to terminate a role, for example for PoolSV2 we need to add PoolSV2::stop. Any ideas on how best to approach this would be appreciated!

@jbesraa jbesraa changed the title Add stop functions for roles Add stop function for roles Aug 27, 2024
@plebhash
Copy link
Collaborator

plebhash commented Aug 27, 2024

@jbesraa can you elaborate on the motivations for this?

what kind of scenario are we trying to mitigate?

my understanding is that we just want to avoid zombie tasks that stay alive after the test has finished running... if that's the case, I would try to keep handles and use them to explicitly shut down all tasks

@plebhash
Copy link
Collaborator

overall, we should be as conservative as possible, because otherwise we will start going down very deep rabbit holes

I was originally excited about #1120 because it felt like a low-hanging-fruit that would allow us to migrate our CI away from MG and patch bugs on the PoC roles in a more sustainable and scalable way on the near future

but if this task will impose the need for deep refactors into the roles, then the trade-offs are no longer worth it in my view

@GitGab19
Copy link
Collaborator

GitGab19 commented Sep 3, 2024

Hey @jbesraa did you find a workaround for this?
Have you explored the task collector option suggested by @Shourya742 last call?

@Shourya742
Copy link
Contributor

@GitGab19 I am seeing new changes here: #1066. @jbesraa has introduced state machine in roles crate. I am not very certain why this approach is taken, as in one of the meetings it was decided to not add code specific to integration test suite in production code. But I guess thats the workaround, rather than killing the joinhandles.

@jbesraa
Copy link
Contributor Author

jbesraa commented Sep 3, 2024

I am closing this as its irrelevant atm. please move the discussion to #1066

@jbesraa jbesraa closed this as completed Sep 3, 2024
@jbesraa
Copy link
Contributor Author

jbesraa commented Sep 3, 2024

#1066 (comment)

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

No branches or pull requests

4 participants