workflows/backport: cancel concurrent runs#414800
workflows/backport: cancel concurrent runs#414800wolfgangwalther merged 1 commit intoNixOS:masterfrom
Conversation
When backporting a change to 24.11 and 25.05 at the same time by adding the two labels immediately *after* merging the PR, three backport jobs will run concurrently: One for the merge and one for each label added. Each of those jobs will try to create both PRs, which will lead to two of the jobs failing for sure. With a concurrency group and cancelling in-progress jobs, only one of those jobs will remain. This reduces notification noise.
|
I was actually thinking about this after running into the same issue with the workflow in Nixvim.
I may be wrong, but I believe cancelled jobs still (kinda) count as a failure, at least enough to trigger an email notification? There may also be the potential for race conditions here; as it's the already running job that gets cancelled. E.g. maybe the job could get cancelled after pushing the backport branch but before creating the PR? Or at least, after doing some backporting work but before finishing. Not sure if these are practical or theoretical issues though. Maybe there's a way to use a non-cancelling concurrency group, but have the subsequent jobs "skip" if the backport has already happened? Or maybe I'm just over-thinking this? |
Yes. I already brought this up in #412608, so those cancelled jobs are happening more frequently already. The only solution is to set up an email filter for them, because https://github.com/orgs/community/discussions/13015 is far from being solved.
I thought about this case, too. For #412608, I looked quite a bit into how cancellation of jobs works and there is a grace period before forced shutdown. So I'm hoping that once the backport action is ready to push stuff, the push + creation of the PR will actually be rather fast - thus they might still finish in time. Of course.. that depends on how this action deals with cancellation requests. If this becomes a problem, we might look into #412659 (comment), aka rolling our own backport action, which could do better in this regard. So, for now, I'm willing to try and see.
That would be an alternative, if the backport action would support it silently. It does not, right now. When you already had a backport for 25.05, for example, and then add a backport for 24.11 later, the action will try to create both again and fail loudly for 25.05. If this would happen in an idempotent way, then that would be much better, yes. |
MattSturgeon
left a comment
There was a problem hiding this comment.
Yes. I already brought this up in #412608, so those cancelled jobs are happening more frequently already.
So, for now, I'm willing to try and see.
SGTM
|
Successfully created backport PR for |
|
Successfully created backport PR for |
|
This didn't work too well, yet:
|
😢 I guess the only solution is to look into rolling our own backport action, and/or contributing fixes/features to the existing upstream action? Maybe we can try and go for something that skips jobs instead of cancelling them, as discussed earlier 🤔 If nixpkgs did come up with a first-party action, would there be value in having it in a separate repo? Either a NixOS or nix-community org repo. Currently at least Stylix, Nixvim, and home-manager have implemented backport workflows based on the nixpkgs workflow, using the same upstream action. So maybe they'd want to make use of whatever implementation nixpkgs came up with? I guess one step at a time 😁 |
|
Yeah, an idempotent backport action seems best. If the backport already happened - do nothing. Also, some failure cases I have observed were like "branch was created, put PR creation failed". In this case it would be great, if the later action could pick up where the earlier one left off and just create the PR for the existing branch. |
Or at least force-push over the earlier attempt's branch |
|
I think force-pushing would potentially result in triggering the CI on the backport PR up to 3x - and whenever someone plays with the labels after the PR had already been created. Although that brings up an interesting aspect: If the backport label is removed, a still-open PR could also be closed automatically. |
For now, I opened an upstream issue at korthout/backport-action#470. |
When backporting a change to 24.11 and 25.05 at the same time by adding the two labels immediately after merging the PR, three backport jobs will run concurrently: One for the merge and one for each label added. Each of those jobs will try to create both PRs, which will lead to two of the jobs failing for sure.
With a concurrency group and cancelling in-progress jobs, only one of those jobs will remain. This reduces notification noise.
Example of 3 runs at the same time: #414784.
Things done
Add a 👍 reaction to pull requests you find important.