Tune default timeout and max_threads #305
Conversation
|
30s might be a better default. For how to add a new config option, you can look at this: 4ebdac2 It implements both config and command line args for breadcrumbs and index generation. I don't think that a CLI arg is needed for the timeout parameter so you can ignore those portions. The code that deals with max_retries might be another thing to look at as it is similar to this (something internal that we want to make configurable for exceptional cases. Only settable via config. An integer value). |
dmsimard
changed the title
|
I've tweaked the default timeout to 20s (up from 10s), I think 30s is long but I agree that 10s is short. I snuck in a somewhat related commit to lower the default amount of threads so we don't get rate-limited as much. |
Because we didn't set a value for the timeout, we ended up falling back to the default aiohttp.get timeout which is 300s (5 minutes!) and that's a very long time to wait before retrying. 20s is long enough that it should be sufficient.
Multi-threading improves performance but services we are interacting with (such as galaxy) are rate limited. Having a higher number of threads means we are more likely to hammer the service and get HTTP 429 errors in return which means needing to timeout and retry which leads to increased overall build time. Having less threads won't be as fast but should also result in less HTTP 429s, timeouts and retries so better performance overwall.
|
@dmsimard thanks for the improvements! |
|
It turns out that this PR actually does not change the default for max_threads. #365 fixes this. |
We should wait less than 300s before retrying. Probably make it configurable.