From 02641ce145e4f6a38e1f45673ebd0a10814f6999 Mon Sep 17 00:00:00 2001 From: James Suplizio Date: Fri, 24 Mar 2023 12:42:09 -0700 Subject: [PATCH] Update the scheduled event governor based upon the actions rate limit (#5816) * Update the scheduled event governor based upon the actions rate limit * Updates for feedback * remove a couple of unused constants --- .../MockGitHubEventClient.cs | 12 ++ .../Constants/RateLimitConstants.cs | 15 +++ .../ScheduledEventProcessing.cs | 123 ++++++++++-------- .../GitHubEventClient.cs | 29 +++++ 4 files changed, 124 insertions(+), 55 deletions(-) create mode 100644 tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Constants/RateLimitConstants.cs diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs index ecd22498043..ef6d16fb98e 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs @@ -455,5 +455,17 @@ public List GetGitHubIssuesToLock() { return _gitHubIssuesToLock; } + + /// + /// Override for the GitHubEventClient funcion which computes this based upon the repository's core rate limit. + /// Since this isn't necessary for the tests, just return the 100 which is the size of one page. + /// + /// int +#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously + public override async Task ComputeScheduledTaskUpdateLimit() +#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously + { + return 100; + } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Constants/RateLimitConstants.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Constants/RateLimitConstants.cs new file mode 100644 index 00000000000..e524e7bf4be --- /dev/null +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Constants/RateLimitConstants.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Azure.Sdk.Tools.GitHubEventProcessor.Constants +{ + public class RateLimitConstants + { + // https://docs.github.com/en/rest/search?apiVersion=2022-11-28#about-search + // The SearchIssues API has a rate limit of 1000 results which resets every 60 seconds + public const int SearchIssuesRateLimit = 1000; + } +} diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/ScheduledEventProcessing.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/ScheduledEventProcessing.cs index e6bd4b0eeb8..94feb5ca96e 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/ScheduledEventProcessing.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/ScheduledEventProcessing.cs @@ -14,63 +14,68 @@ namespace Azure.Sdk.Tools.GitHubEventProcessor.EventProcessing { public class ScheduledEventProcessing { - // Each repository has rate limit for actions of 1000/hour. Because scheduled tasks have the potential - // to cause a large number of updates, put an initial limit on the number of updates a scheduled task - // can make. - private static int ScheduledTaskUpdateLimit = 100; - // There's a secondary limit to the search API which is 1000 items. The default page size for the search - // is 100 items (which is also the max per page) - private static int MaxSearchResults = 1000; - /// - /// + /// Scheduled events, unlike regular actions, need to have which scheduled event to run, passed in otherwise, it require + /// looking at the cron schedule to determine the event. Doing it this way means if the schedule changes only the cron + /// in the yml file needs to change rather than having to coordinate actions processing changes with the yml update. /// /// Authenticated GitHubEventClient /// ScheduledEventGitHubPayload deserialized from the json event payload - /// String, the scheduled even + /// String, the scheduled event public static async Task ProcessScheduledEvent(GitHubEventClient gitHubEventClient, ScheduledEventGitHubPayload scheduledEventPayload, string cronTaskToRun) { - switch (cronTaskToRun) + // Scheduled events can make multiple calls to SearchIssues due to pagination. Any call to SearchIssues can + // run into a SecondaryRateLimitExceededException, regardless of the page, and there could be pending updates + // from previous pages that were processed. Because of this, the call to process pending updates should be made + // regardless of whether there is an exception or not. If there aren't any updates then the call is effectively + // a no-op so it's good either way. + try { - case RulesConstants.CloseAddressedIssues: - { - await CloseAddressedIssues(gitHubEventClient, scheduledEventPayload); - break; - } - case RulesConstants.CloseStaleIssues: - { - await CloseStaleIssues(gitHubEventClient, scheduledEventPayload); - break; - } - case RulesConstants.CloseStalePullRequests: - { - await CloseStalePullRequests(gitHubEventClient, scheduledEventPayload); - break; - } - case RulesConstants.IdentifyStaleIssues: - { - await IdentifyStaleIssues(gitHubEventClient, scheduledEventPayload); - break; - } - case RulesConstants.IdentifyStalePullRequests: - { - await IdentifyStalePullRequests(gitHubEventClient, scheduledEventPayload); - break; - } - case RulesConstants.LockClosedIssues: - { - await LockClosedIssues(gitHubEventClient, scheduledEventPayload); - break; - } - default: - { - Console.WriteLine($"{cronTaskToRun} is not valid Scheduled Event rule. Please ensure the scheduled event yml is correctly passing in the correct rules constant."); - break; - } + switch (cronTaskToRun) + { + case RulesConstants.CloseAddressedIssues: + { + await CloseAddressedIssues(gitHubEventClient, scheduledEventPayload); + break; + } + case RulesConstants.CloseStaleIssues: + { + await CloseStaleIssues(gitHubEventClient, scheduledEventPayload); + break; + } + case RulesConstants.CloseStalePullRequests: + { + await CloseStalePullRequests(gitHubEventClient, scheduledEventPayload); + break; + } + case RulesConstants.IdentifyStaleIssues: + { + await IdentifyStaleIssues(gitHubEventClient, scheduledEventPayload); + break; + } + case RulesConstants.IdentifyStalePullRequests: + { + await IdentifyStalePullRequests(gitHubEventClient, scheduledEventPayload); + break; + } + case RulesConstants.LockClosedIssues: + { + await LockClosedIssues(gitHubEventClient, scheduledEventPayload); + break; + } + default: + { + Console.WriteLine($"{cronTaskToRun} is not valid Scheduled Event rule. Please ensure the scheduled event yml is correctly passing in the correct rules constant."); + break; + } + } + } + finally + { + // The second argument is IssueOrPullRequestNumber which isn't applicable to scheduled events (cron tasks) + // since they're not going to be changing a single IssueUpdate like rules processing does. + await gitHubEventClient.ProcessPendingUpdates(scheduledEventPayload.Repository.Id); } - // The second argument is IssueOrPullRequestNumber which isn't applicable to scheduled events (cron tasks) - // since they're not going to be changing a single IssueUpdate like rules processing does. - await gitHubEventClient.ProcessPendingUpdates(scheduledEventPayload.Repository.Id); } /// @@ -89,6 +94,7 @@ public static async Task CloseAddressedIssues(GitHubEventClient gitHubEventClien { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseAddressedIssues)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); List includeLabels = new List { @@ -106,7 +112,7 @@ public static async Task CloseAddressedIssues(GitHubEventClient gitHubEventClien int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; for (request.Page = 1; request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); @@ -167,6 +173,8 @@ public static async Task CloseStaleIssues(GitHubEventClient gitHubEventClient, S { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseStaleIssues)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); + List includeLabels = new List { LabelConstants.NeedsAuthorFeedback, @@ -185,7 +193,7 @@ public static async Task CloseStaleIssues(GitHubEventClient gitHubEventClient, S int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; for (request.Page = 1;request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); @@ -241,6 +249,7 @@ public static async Task CloseStalePullRequests(GitHubEventClient gitHubEventCli { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseStalePullRequests)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); List includeLabels = new List { @@ -258,7 +267,7 @@ public static async Task CloseStalePullRequests(GitHubEventClient gitHubEventCli int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; for (request.Page = 1; request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); @@ -319,6 +328,7 @@ public static async Task IdentifyStalePullRequests(GitHubEventClient gitHubEvent { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.IdentifyStalePullRequests)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); List excludeLabels = new List { @@ -337,7 +347,8 @@ public static async Task IdentifyStalePullRequests(GitHubEventClient gitHubEvent int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; + for (request.Page = 1; request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); @@ -398,6 +409,7 @@ public static async Task IdentifyStaleIssues(GitHubEventClient gitHubEventClient { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.IdentifyStaleIssues)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); List includeLabels = new List { @@ -421,7 +433,7 @@ public static async Task IdentifyStaleIssues(GitHubEventClient gitHubEventClient int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; for (request.Page = 1; request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); @@ -480,6 +492,7 @@ public static async Task LockClosedIssues(GitHubEventClient gitHubEventClient, S { if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.LockClosedIssues)) { + int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit(); SearchIssuesRequest request = gitHubEventClient.CreateSearchRequest( scheduledEventPayload.Repository.Owner.Login, @@ -492,7 +505,7 @@ public static async Task LockClosedIssues(GitHubEventClient gitHubEventClient, S int numUpdates = 0; // In theory, maximumPage will be 10 since there's 100 results per-page returned by default but // this ensures that if we opt to change the page size - int maximumPage = MaxSearchResults / request.PerPage; + int maximumPage = RateLimitConstants.SearchIssuesRateLimit / request.PerPage; for (request.Page = 1; request.Page <= maximumPage; request.Page++) { SearchIssuesResult result = await gitHubEventClient.QueryIssues(request); diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs index 2fa7fa29e80..01094a4fbb7 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs @@ -125,6 +125,8 @@ public GitHubIssueToUpdate(long repositoryId, int issueOrPRNumber, IssueUpdate i // for action processing which uses a shared event. protected List _gitHubIssuesToUpdate = new List(); + public int CoreRateLimit { get; set; } = 0; + public RulesConfiguration RulesConfiguration { get { return _rulesConfiguration; } @@ -267,6 +269,7 @@ public int ComputeNumberOfExpectedUpdates() public async Task WriteRateLimits(string prependMessage = null) { var miscRateLimit = await GetRateLimits(); + CoreRateLimit = miscRateLimit.Resources.Core.Limit; // Get the Minutes till reset. TimeSpan span = miscRateLimit.Resources.Core.Reset.UtcDateTime.Subtract(DateTime.UtcNow); // In the message, cast TotalMinutes to an int to get a whole number of minutes. @@ -278,6 +281,32 @@ public async Task WriteRateLimits(string prependMessage = null) Console.WriteLine(rateLimitMessage); } + /// + /// Return the number of updates a scheduled task can make. The Core Rate Limit that GitHub Actions can make is 15000/hour + /// for enterprise and 1000/hour for non-enterprise. The max number of results that can be retried from SearchIssues is 1000. + /// The CoreRateLimit is set when WriteRateLimits is called and this is done at the start of processing in Main. If the core + /// rate limit is 15000, return 1000, otherwise return 100 which is 1/10th of the hourly limit for non-enterprise repository. + /// + /// The number updates a scheduled task can make. + public virtual async Task ComputeScheduledTaskUpdateLimit() + { + int updateLimit = 0; + + // CoreRateLimit will be set in WriteRateLimits but if that hasn't been called yet, call it now. + if (CoreRateLimit == 0) + { + var miscRateLimit = await GetRateLimits(); + CoreRateLimit = miscRateLimit.Resources.Core.Limit; + } + updateLimit = CoreRateLimit / 10; + if (updateLimit > RateLimitConstants.SearchIssuesRateLimit) + { + updateLimit = RateLimitConstants.SearchIssuesRateLimit; + } + Console.WriteLine($"Setting the scheduled task update limit to: {updateLimit}"); + return updateLimit; + } + /// /// Write the current rate limit and remaining number of transactions. ///