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

feat: Add an option to configure subprocess pool size #273

Merged
merged 3 commits into from
Oct 1, 2020

Conversation

tonyo
Copy link
Contributor

@tonyo tonyo commented Sep 29, 2020

This can be useful when Symbolicator is not the only service running on the machine (e.g. in Kubernetes environment).

@tonyo tonyo requested a review from a team September 29, 2020 13:49
Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

You will also need to update this here, which creates our standard thread pools:

let runtime = tokio::runtime::Builder::new().build().unwrap();

Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

Sorry, meant to request changes.

@tonyo
Copy link
Contributor Author

tonyo commented Sep 29, 2020

@jan-auer you sure we want to have one setting for everything? Technically, there's also a worker pool in actix HttpServer where we can also adjust worker count
https://github.com/getsentry/symbolicator/blob/master/src/server.rs#L32

@jan-auer
Copy link
Member

That's a good question. We may want to split this, indeed.

Some of these thread pools used to perform CPU-intensive work, but do no longer due to subprocessing. So in that case, we may consider to dial them down independently, so I think your patch is fine.

Should we maybe clarify in docs then that this is the number of sub processes?

@tonyo
Copy link
Contributor Author

tonyo commented Sep 30, 2020

@jan-auer updated the docstring, but in the docs we already have this:

image

@tonyo tonyo requested a review from jan-auer September 30, 2020 15:52
@tonyo tonyo merged commit 548683d into master Oct 1, 2020
@tonyo tonyo deleted the feat/allow-configure-subproc-size branch October 1, 2020 10:53
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