Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autoupdater: delay adding queue head label + minor improvements #108

Merged
merged 7 commits into from
Mar 13, 2024

Conversation

fho
Copy link
Contributor

@fho fho commented Mar 12, 2024

autoupdate: increase updateBranchPollInterval to 2 seconds

Triggering the update branch status often returned needed to be repeated 3x
because the branch updated happened while doing the 2. retry.
Increase the interval to 2 seconds, to reduce the number of API operations by
increasing the chance that it was already updated when it is queried the second
time.

-------------------------------------------------------------------------------
autoupdate: log commit for that the ready for merge status was retrieved

This will be helpful to determine if the API returned the status for an old or
the current HEAD commit.

-------------------------------------------------------------------------------
autoupdate: delay adding queue head label

The queue head label was added as soon as an PR became first in the active
queue.
When the github event for the added label was used to trigger CI jobs this
caused unnecessary runs:
- the PR might not meet the conditions and the label is removed by the update
  operation shortly afterwards again,
- the PR might not be first anymore in the queue when the belonging update
  operation runs,´

It can also cause that a PR is wrongly suspended, in the scenario:
- label added
- update of PR branch with base branch is scheduled
- label add triggered CI job run, CI job already runs on the updated HEAD commit
  because of delays,
- synchronize branch event caused by the update with the base branch, causes
  that the same ci jobs are triggered again (unnecessarily)
- previously triggered CI jobs are canceled and a failure check event is sent to
  github
- failure check event causes the PR to be suspended

-------------------------------------------------------------------------------
githubclt: fix: UpdateBranch logs debug up2date message with old head

UpdateBranch is retrieving the HEAD sha of the PR and then checking if the PR
branch contains all changes of the base branch by specifying the PR branch name
as reference.
Between retrieving the head sha and comparing the branches, the branch might
have changed which causes that UpdateBranch was logging that the branch is
up2date with an old head commit.

Fix it by passing the PR head SHA instead of the branch name as reference when
comparing the branch with it's base.

-------------------------------------------------------------------------------
autoupdater: improve debug log messages when suspending a PR

log when suspending a PR or removing it from the active queue causes another PR
to be the first one in the queue, this can be handy when debugging issues.

-------------------------------------------------------------------------------
autoupdate: do not abort suspend procedure in unlikely error case

If a PR should be in the suspend and active queue because of a bug, continue
with the suspend procedure and overwrite the existing PR in the suspend queue.
This ensures that everything continues to work, instead of not not removing the
PR label and not aborting an update task for it.

The error scenario never happened.

-------------------------------------------------------------------------------
githubclt: return error if BehindBy field in github response is nil

This never happens but if it does, it ensures we do not return wrongly that the
PR is uptodate because GetBehindBy() returns 0 if *BehindBy is nil.

-------------------------------------------------------------------------------

fho added 7 commits March 12, 2024 17:36
This never happens but if it does, it ensures we do not return wrongly that the
PR is uptodate because GetBehindBy() returns 0 if *BehindBy is nil.
If a PR should be in the suspend and active queue because of a bug, continue
with the suspend procedure and overwrite the existing PR in the suspend queue.
This ensures that everything continues to work, instead of not not removing the
PR label and not aborting an update task for it.

The error scenario never happened.
log when suspending a PR or removing it from the active queue causes another PR
to be the first one in the queue, this can be handy when debugging issues.
UpdateBranch is retrieving the HEAD sha of the PR and then checking if the PR
branch contains all changes of the base branch by specifying the PR branch name
as reference.
Between retrieving the head sha and comparing the branches, the branch might
have changed which causes that UpdateBranch was logging that the branch is
up2date with an old head commit.

Fix it by passing the PR head SHA instead of the branch name as reference when
comparing the branch with it's base.
The queue head label was added as soon as an PR became first in the active
queue.
When the github event for the added label was used to trigger CI jobs this
caused unnecessary runs:
- the PR might not meet the conditions and the label is removed by the update
  operation shortly afterwards again,
- the PR might not be first anymore in the queue when the belonging update
  operation runs,´

It can also cause that a PR is wrongly suspended, in the scenario:
- label added
- update of PR branch with base branch is scheduled
- label add triggered CI job run, CI job already runs on the updated HEAD commit
  because of delays,
- synchronize branch event caused by the update with the base branch, causes
  that the same ci jobs are triggered again (unnecessarily)
- previously triggered CI jobs are canceled and a failure check event is sent to
  github
- failure check event causes the PR to be suspended
This will be helpful to determine if the API returned the status for an old or
the current HEAD commit.
Triggering the update branch status often returned needed to be repeated 3x
because the branch updated happened while doing the 2. retry.
Increase the interval to 2 seconds, to reduce the number of API operations by
increasing the chance that it was already updated when it is queried the second
time.
@fho fho marked this pull request as ready for review March 13, 2024 09:37
@fho fho merged commit 0f84840 into main Mar 13, 2024
5 checks passed
@fho fho deleted the update_race branch March 13, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant