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

Simplify Boost/Pause logic #18673

Merged
merged 4 commits into from
Feb 8, 2022

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Feb 8, 2022

#18658 has added a check to see if we need to boost because there is still work to do
however the check is slightly complex and not ideal. There's no point boosting if
the queue is paused or can't scale. Therefore merge the two selects into one and add
a check to p.paused.

Also we should we should also boost on Resume if we have things in the queue but no workers

Signed-off-by: Andrew Thornton art27@cantab.net

go-gitea#18658 has added a check to see if we need to boost because there is still work to do
however the check is slightly complex and not ideal. There's no point boosting if
the queue is paused or can't scale. Therefore merge the two selects into one and add
a check to p.paused.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added type/enhancement An improvement of existing functionality skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Feb 8, 2022
@zeripath zeripath added this to the 1.17.0 milestone Feb 8, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 8, 2022
zeripath and others added 3 commits February 8, 2022 14:45
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 8, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #18673 (cb38ae7) into main (4d93984) will increase coverage by 0.01%.
The diff coverage is 39.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18673      +/-   ##
==========================================
+ Coverage   46.64%   46.66%   +0.01%     
==========================================
  Files         846      846              
  Lines      121331   121362      +31     
==========================================
+ Hits        56595    56631      +36     
- Misses      57859    57860       +1     
+ Partials     6877     6871       -6     
Impacted Files Coverage Δ
modules/queue/workerpool.go 53.28% <38.70%> (-2.59%) ⬇️
services/mirror/mirror.go 15.70% <41.17%> (+4.19%) ⬆️
modules/queue/queue_channel.go 78.30% <0.00%> (-4.72%) ⬇️
modules/queue/queue_disk_channel.go 60.66% <0.00%> (-1.26%) ⬇️
services/pull/pull.go 40.24% <0.00%> (+0.46%) ⬆️
models/issue_comment.go 52.42% <0.00%> (+1.01%) ⬆️
models/unit/unit.go 48.67% <0.00%> (+1.76%) ⬆️
modules/log/file.go 73.50% <0.00%> (+1.98%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c14ca34...cb38ae7. Read the comment docs.

@techknowlogick techknowlogick merged commit f8b21ac into go-gitea:main Feb 8, 2022
@zeripath zeripath deleted the simplify-the-boost-check branch February 8, 2022 19:39
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 9, 2022
* giteaofficial/main:
  Prevent security failure due to bad APP_ID (go-gitea#18678)
  [skip ci] Updated translations via Crowdin
  Let `MinUnitAccessMode` return correct perm (go-gitea#18675)
  Simplify Boost/Pause logic (go-gitea#18673)
  update the comparison documents (go-gitea#18669)
  Restart zero worker if there is still work to do (go-gitea#18658)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
* Simplify Boost/Pause logic

go-gitea#18658 has added a check to see if we need to boost because there is still work to do
however the check is slightly complex and not ideal. There's no point boosting if
the queue is paused or can't scale. Therefore merge the two selects into one and add
a check to p.paused.

Signed-off-by: Andrew Thornton <art27@cantab.net>

* And on resume add a zeroboost if necessary

Signed-off-by: Andrew Thornton <art27@cantab.net>

* simplify

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lauris BH <lauris@nix.lv>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants