Scheduled event processing update #7839
Merged
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.
The new rule, EnforceMaxLifeOfIssues, creates a comment, closes and locks issues. As it turns out, when the rule first ran in azure-sdk-for-net, it was there were several hundred issues that matched the criteria. When the rule started processing this backlog, it hit a SecondaryRateLimitExceededException while creating comments due to the fact it tried to update more than 80 in a 60 second period. Theoretically, can happen with any API that "creates content" which includes creating comments, closing and locking. Though we already know that locking 1000 issues never hits this. The reason we've never previously seen this is that the existing Scheduled events is because those events are all caught up and just processing a small handful if they process anything at all.
While the per-minute limit is 80, hitting the SecondaryRateLimitExceededException can affect more than just Actions/Scheduled events, it can also affect people doing things in the UI. I've split the pending update processing to create a method specifically for processing pending scheduled updates. This method will sleep for 60 seconds after processing 50 items and, if any update call hits a SecondaryRateLimitExceededException, it'll sleep before retry and try up to 5x. I'm not expecting these to be hit, with the delay that happens after every 50 updates.
The other thing that had to change was the overall limit to the number of items we'd process in any given scheduled event. In theory, this number is 500/hour. For scheduled events I've lowered it from 1/10th of the hourly limit, with cap of 1000, to 300. This is the same reason as the 50/minute. While exiting scheduled events are processing scraps, any new events being added that have to play catch up, will process more. The 300 max limit of updates is just a safety buffer. The solution to processing less events per iteration would be to have a given scheduled event run more frequently which would catch it up faster.