Stale action doesn't seem to be working

[mmarchini]

https://github.com/nodejs/node/actions/runs/247170963

It seems like the Action is not faring well with the size of our repository. I'm not sure if it is closing some issues, or if it tries to detect all issues before closing, but fails to do so because it reaches some limit. Based on the number of lines in the output, we're probably iterating over all issues (even though we don't have to) and therefore we're hitting GitHub API rate limit (but that's just a guess). We probably need some changes on the upstream action to better handle days-before-stale: -1 scenarios.

@phillipj fyi

[phillipj]

Thanks @mmarchini!

I'll see if there's any obvious optimisations we can try to avoid hitting that limit.

[targos]

We can probably optimize it by using the only-labels option (with a value of stalled) because it's used to filter the query: https://github.com/actions/stale/blob/13b324e4b28a2708236aadb11361fa65af60d201/src/IssueProcessor.ts#L319

[phillipj]

Spot on @targos, that looks really promising indeed, thx!

Actually had that in place at some point, but removed it before opening #34555 as it worked in practise on the smaller test repos I used and the benefit wasn't obvious -- until now 😄

New PR incoming.

[targos]

The action closed a few issues but still reach a maximum number of operations: https://github.com/nodejs/node/actions/runs/252939090

I don't understand, because the number of operations per run is a required action input but we don't set it and the action still runs.

[phillipj]

Very strange indeed. Expanding the logs of the run you mentioned, shows quite a lot of verbose logging; 405 lines logged before being killed 🤔

I'll dive in and see if there's any hints in there, or GitHub docs describing what kind of limits there are.

[phillipj]

Just found the culprit; the maximum number of operations error is self imposed by the stale GitHub Action. It counts the number of operations (read: github API requests) performed, and exits with that error when having reached a certain threshold.

Luckily it's configurable via the operations-per-run option (default: 30).

As described in the option docs, it's a countermeasure to avoid hitting the request per hour rate limits. Meaning it might not be the best idea to bump operations-per-run sky high, as it could cause trouble for other GitHub actions we have, if the rate limit is hit.

There's improvements to be made in the stale action project to reduce the API requests made, which we in practise never use the result of (un-labelling due to comments), but that will naturally take some time to fix.

In the near future, we've got two alternatives as far as I see things:

1. leave operations-per-run as is and let the auto closing of issues/PRs work its way veeeery slowly through the list
2. bump operations-per-run slightly to increase the pace

Any thoughts?

[mmarchini]

30 is a very low number of operations, which APIs are used by the Action? Most endpoints tolerate up to 5000 requests per hour [1], 30 requests per minute for Search API [2], and 5000 point per hour (whatever that means) for GraphQL queries [3]. Unless the search API is being used, we can increase it without problem.

If the Search API is being used, we could have the stale action running every half hour or so, that way we'll drain the queue way faster (probably in a day or two).

[phillipj]

Here's the relevant requests it performs when fetching issues/PRs labelled stalled and figures out whether or not they should get closed:

• Getting all stalled issues: list repository issues
  • for each and every stalled issue/PR:
    • when was it labelled? list issue events
    • get all comments: list issue comments

Unless the search API is being used, we can increase it without problem.

Cool. I haven't found any use of the Search API.

I'm more than happy to open a new PR to bump operations-per-run. Shooting in the dark here; 500? 🤷‍♂️

[mmarchini]

Yeah let's go with 500 :)

[mmarchini]

Also I'm almost sure the request limits is scoped to the action run, meaning it wouldn't affect other runs (not entirely sure though)

[phillipj]

Huh, that would be nice indeed. In worst case it doesn't and we end up blowing our rate limits sometime in the future, we can adjust appropriately.

Appreciate the lightning quick responses!

[phillipj]

Bumping operations-per-run certainly seems to have fixed the "max number of operations" error we were struggling with.

The first close stalled action run after those changes excited cleanly with No more issues found to process. Exiting..

Still not as effective closing issues as I'd thought beforehand to be honest. That's because most of the issues it checks ends up evaluating to true for one/both of the below:

a. has been commented on by someone else than the issue author or a bot since it was labelled stalled
b. has been updated since it was labelled stalled

The results of those checks are logged into something like this per issue:

Stale pr is not old enough to close yet (hasComments? false, hasUpdate? true

Are we okey with this as is? Or do anyone fee there's improvements to be made the stale action to be more effective?

[mmarchini]

Does it close 30 days after someone comments? If so I think we're fine, otherwise we might need to revisit.

[phillipj]

Does it close 30 days after someone comments?

Yes, as long as no updates to the issue/PR has been made after the last comment (reflected by that issue's .updated_at timestamp).

Would that cause headache for us? Do we often update stalled issues frequently, which would in practise postpone the auto closing behaviour for too long?

[mmarchini]

That sounds totally reasonable for us. If it becomes a problem we can revisit. For now I believe we can close this issue.