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

start client actor in a separate thread #6333

Merged
merged 8 commits into from
Mar 7, 2022

Conversation

mm-near
Copy link
Contributor

@mm-near mm-near commented Feb 22, 2022

It seems that starting the actor in the 'current' thread (as we do right now) - is causing a bunch of issues (impacting the messages that are going to other actors)

@matklad
Copy link
Contributor

matklad commented Feb 22, 2022

Yeah, this makes total sense! One thing to note that, as we now spawn ClientActor to a different thread, we exacerbate shutdown problems. Ideally, we'd return an Arbiter, rather than ArbiterHandle from this function, and .join() it in main to make sure that all cleanup work finished.

And yeah, I feel that the fundamental problem is that whatever actix is doing internally is poorly documented, opaque, and doesn't actually help us much. Given the size and complexity of near, it makes sense for us to own our event loop, rather than to hide it in third-party actor framework without significant usage.

Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Epic! I saw this failure in CI pthread destroy mutex: Device or resource busy which is interesting.

@nikurt
Copy link
Contributor

nikurt commented Feb 22, 2022

pthread destroy mutex: Device or resource busy

I hope that #6326 will fix this.

@near-bulldozer near-bulldozer bot merged commit f2bba08 into master Mar 7, 2022
@near-bulldozer near-bulldozer bot deleted the 0222_client_separate_thread branch March 7, 2022 15:55
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.

4 participants