sort order to get issues/pullrequests should be `updated` not `created`

[nakamasato]

Description:

Strictly speaking, it's not a bug but the current behavior makes the stale feature less efficient.

As the default value of sort is created, when having many old issues, the retrieved issues are almost same until they're gradually closed especially when setting operations-per-run is set. https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-repository-issues

Action version:
Specify the action version

Platform:

• Ubuntu
• macOS
• Windows

Runner type:

• Hosted
• Self-hosted

Repro steps:
A description with steps to reproduce the issue. If your have a public example or repo to share, please provide the link.

Expected behavior:
A description of what you expected to happen.

Sort by updated

Actual behavior:
A description of what is actually happening.

Sort by created

https://github.com/nakamasato/stale/blob/ee7ef89499a3de6e4fe1fc1acb994e67c64e0a2a/src/classes/issues-processor.ts#L565-L576

[suyashgaonkar]

Hi @nakamasato ,Thank you for creating this issue. We will investigate it and provide feedback as soon as we have some updates.

[nakamasato]

Thanks!

[suyashgaonkar]

Hi @nakamasato , Similar kind of discussion has happened in issue #792 where optimization about actions/stale has been discussed. You can refer to following comment thread in the same issue where effects of sorting issues by date updated has been discussed. As you can see in the issue #792 , PR #1033 was raised for performance optimization with help of actions/cache where it and was successfully merged and new version was released . Please refer to the following release notes (v 9.0.0).

[nakamasato]

@suyashgaonkar Thank you for the information and I checked the links and one of our executions.

I found the following failure.

The saved state was not found, the process starts from the first issue.
...
Failed to save: Unable to reserve cache with key _state, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/main, Key: _state, Version: xxxx

I think it's same as #1090. I'm testing with the mentioned permission

permissions:
  actions: write

And it's successfully stored.

Cache Size: ~0 MB (705 B)
Cache saved successfully

Also as discussed in the issue #1090, it'd be nice to mention the permission in the readme.

[nakamasato]

Though I confirmed the state was successfully saved to the cache on a PR but after running on the default branch, it failed to save and find the state.

Failed to save: Unable to reserve cache with key _state, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/main, Key: _state, Version: xxx

Is it possible to specify a cache key?

[rcomer]

@nakamasato perhaps you are running into #1136? There is an open PR #1169 to configure the cache key.

[suyashgaonkar]

Hi @nakamasato , Following issue needs a permanent fix in actions/cache, which is working under the hood for actions/stale to handle statefulness in stale. You can also refer to issue #1361 which is created in actions/cache addressing the same issue, where you can keep a track for checking the progress on the same. We feel it won’t be ideal to add this work around in the README as above mentioned issue is already addressed and needs a permanent fix.

[nakamasato]

@rcomer I'm not pretty sure if it's because of having many caches but thank you for the information. would be nice if we can set a prefix for cache key.

[nakamasato]

@suyashgaonkar Thank you for the information. I understand but at the same time, we can't use the written feature if we just follow the readme right now. so back to my original issue, the issue would be resolved (at least partly) only if we change the order to updated.

I reread the comment (#792 (comment)) but I think the following statement is not always true.

I can imagine that the oldest stale issues will not get closed with this strategy.

The oldest stale pr would get labeled for the first time and it indeed becomes most recently updated one, so it won't be checked until all the older prs are updated in some way (manually or by the actions/stale). If we have too many stale prs, it'd take long to start closing until all the stale prs need to get labeled until the first labeled pr gets closed. However in this case, we can increase the frequency of the action execution as long as the API limit allows and sooner or later the stale action will catch up with all the old stale prs eventually.

On the other hand, with the current order with created, stale actions might be completely stuck if the number of prs with the exempt label reached the number of prs that can be processed each time that is limited by operations-per-run. Even if it's not completely stuck, it's extremely inefficient to process same prs for many days (e.g. 30+30 days if you set days-before-stale and days-before-close to 30).