Skip to content

Commit

Permalink
Update the scheduled event governor based upon the actions rate limit (…
Browse files Browse the repository at this point in the history 
…#5816)

* Update the scheduled event governor based upon the actions rate limit

* Updates for feedback

* remove a couple of unused constants
  • Loading branch information
@JimSuplizio
JimSuplizio authored Mar 24, 2023
1 parent b70dac6 commit 02641ce
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 55 deletions.
12 changes: 12 additions & 0 deletions ...ithub-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,5 +455,17 @@ public List<GitHubIssueToLock> GetGitHubIssuesToLock()
{
return _gitHubIssuesToLock;
}

/// <summary>
/// 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.
/// </summary>
/// <returns>int</returns>
#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
public override async Task<int> ComputeScheduledTaskUpdateLimit()
#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
{
return 100;
}
}
}
15 changes: 15 additions & 0 deletions ...thub-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Constants/RateLimitConstants.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
123 changes: 68 additions & 55 deletions ...rocessor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/ScheduledEventProcessing.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/// <summary>
///
/// 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.
/// </summary>
/// <param name="gitHubEventClient">Authenticated GitHubEventClient</param>
/// <param name="scheduledEventPayload">ScheduledEventGitHubPayload deserialized from the json event payload</param>
/// <param name="cronTaskToRun">String, the scheduled even</param>
/// <param name="cronTaskToRun">String, the scheduled event</param>
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);
}

/// <summary>
Expand All @@ -89,6 +94,7 @@ public static async Task CloseAddressedIssues(GitHubEventClient gitHubEventClien
{
if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseAddressedIssues))
{
int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit();

List<string> includeLabels = new List<string>
{
Expand All @@ -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);
Expand Down Expand Up @@ -167,6 +173,8 @@ public static async Task CloseStaleIssues(GitHubEventClient gitHubEventClient, S
{
if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseStaleIssues))
{
int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit();

List<string> includeLabels = new List<string>
{
LabelConstants.NeedsAuthorFeedback,
Expand All @@ -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);
Expand Down Expand Up @@ -241,6 +249,7 @@ public static async Task CloseStalePullRequests(GitHubEventClient gitHubEventCli
{
if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.CloseStalePullRequests))
{
int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit();

List<string> includeLabels = new List<string>
{
Expand All @@ -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);
Expand Down Expand Up @@ -319,6 +328,7 @@ public static async Task IdentifyStalePullRequests(GitHubEventClient gitHubEvent
{
if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.IdentifyStalePullRequests))
{
int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit();

List<string> excludeLabels = new List<string>
{
Expand All @@ -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);
Expand Down Expand Up @@ -398,6 +409,7 @@ public static async Task IdentifyStaleIssues(GitHubEventClient gitHubEventClient
{
if (gitHubEventClient.RulesConfiguration.RuleEnabled(RulesConstants.IdentifyStaleIssues))
{
int ScheduledTaskUpdateLimit = await gitHubEventClient.ComputeScheduledTaskUpdateLimit();

List<string> includeLabels = new List<string>
{
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
29 changes: 29 additions & 0 deletions tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ public GitHubIssueToUpdate(long repositoryId, int issueOrPRNumber, IssueUpdate i
// for action processing which uses a shared event.
protected List<GitHubIssueToUpdate> _gitHubIssuesToUpdate = new List<GitHubIssueToUpdate>();

public int CoreRateLimit { get; set; } = 0;

public RulesConfiguration RulesConfiguration
{
get { return _rulesConfiguration; }
Expand Down Expand Up @@ -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.
Expand All @@ -278,6 +281,32 @@ public async Task WriteRateLimits(string prependMessage = null)
Console.WriteLine(rateLimitMessage);
}

/// <summary>
/// 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.
/// </summary>
/// <returns>The number updates a scheduled task can make.</returns>
public virtual async Task<int> 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;
}

/// <summary>
/// Write the current rate limit and remaining number of transactions.
/// </summary>
Expand Down

0 comments on commit 02641ce

Please sign in to comment.