Repository navigation
Perform requests for one spec at a time - #30
Conversation
|
I wonder how this PR can affect the performances of the script. Let's say we have 300 specs, I expect this can take significantly more time to process and at the same time, it won't be slow enough to avoid getting caught by the github rate limits. |
|
I neither expect nor intend for this change to move the needle regarding the github rate limits specifically. This concerns every collector, which currently isn't a (known) concern, but will almost undoubtedly become one if we don't at least sequence the requests. Using your example number of 300 specs, my point in making this change is that attempting to perform 300 requests at the same time to every service our collectors depend on is nearly guaranteed to be perceived as abuse and ultimately blocked by most or all of them, and is just not a very considerate thing to do. It doesn't matter how much more requesting in serial rather than parallel would theoretically take if the current approach effectively constitutes an attack and prevents requests from succeeding. RE time elapsed: Testing the behavior on main vs. this branch even with only the 3 specs we have so far, results in terms of time vary widely on the I also attempted to test memory consumption, which may point to an even more important reason to make this change, not so much within this PR itself but as a first step in the correct direction for future PRs. While there is already a visible difference in the memory consumed by one spec's set of opened requests vs. opening all 3 sets at the same time, what is far more concerning is the spike in memory usage once all of the data is stored locally in memory. RSS easily peaks around 120MB for just 3 specs (roughly 40MB higher than it is before the first requests are sent). Moving to an in-series approach opens the opportunity to further optimize how much we're keeping in memory at one time. This also potentially gives me even more food for thought on the whole While testing, I have also identified that the cache in the |
|
While I'm not sure if the number of simultaneous requests will be considered as abusive (github.io/raw.githubusercontent.com should be fine and we have control over {api,labs}.w3.org), you raised a good point regarding the memory usage. I can see how it can significantly increase as we are adding more specs to the list. Also after second thoughts, I don't think it matters too much if the script takes 1 min or 1 hr so I'm fine with your proposed changes! Thanks. |
I originally included this fix in #28 but have split it, since the rest of that may be held up for a bit. (This only meaningfully touches a few lines of code, which will be clearer when viewed with whitespace ignored.)
This addresses a scalability/reliability issue, in that previously all requests for all specs would be sent at the same time, which would absolutely result in rate-limiting once we have more than a few specs in the list. Running all collectors at once for each spec should be fine, since each one hits a different service/endpoint.