Add in a config option for api threadpool size #447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viniarck
reviewed
Feb 7, 2024
There was a problem hiding this comment.
@Ktmi, great PR, I'm glad now we have safer concurrency default values.
I'm still under the impression that 80 is too low. Is it?
-
- Did you also tried/explored with higher values like 128/256/512 to get an idea? Probably upstream had a low-ish value for a reason, and I haven't researched much, but I'm interested to hear more about it. If with a larger pool it doesn't consume much extra mem and out of the gate it's less succeptable to potentially lock then that's great.
-
- Which case did you end end up facing a lock up that you highlighted in the description? I had the impression that with the concurrency and thread pool limit at least it wouldn't result in a lock up anymore, assuming
ulimitfile descriptions with a reasonable value too.
- Which case did you end end up facing a lock up that you highlighted in the description? I had the impression that with the concurrency and thread pool limit at least it wouldn't result in a lock up anymore, assuming
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes kytos-ng/mef_eline#406
Summary
Adds in a new threadpool called
api. This is used to specify the number of threads used to handle API requests concurrently by the ASGI server. Also added in the configuration optionapi_concurrency_limitwhich specifies the maximum amount of concurrent requests to accept before requests are outright rejected.Local Tests
Raising the
apithreadpool size allows for handling more API requests which depend on other api requests without locking up. Locking up still a possibility, but now possible to mitigate with a larger pool size here.