Refactor Cron and merge dashboard tasks #10745

zeripath · 2020-03-16T22:47:37Z

Merge Cron and Dashboard tasks and always run in the background
Make every cron task report a system notice on completion
Refactor the creation of these tasks
Ensure that execution counts of tasks is correct
Allow cron tasks to be started from the cron page
Make cron tasks appear as processes in the process manager

* Merge Cron and Dashboard tasks * Make every cron task report a system notice on completion * Refactor the creation of these tasks * Ensure that execution counts of tasks is correct * Allow cron tasks to be started from the cron page

lunny · 2020-03-17T00:56:50Z

This PR still not resolve the probblem that multiple gitea instances will run tasks duplicated.

zeripath · 2020-03-17T20:51:31Z

@lunny I guess you mean if you have a cluster of giteas you only want one task running on one gitea at a time?

I think that would need a DB flag?

guillep2k · 2020-03-18T01:58:04Z

Handling aborts properly is a problem in such a unique queue. Is Gitea currently prepared for multi-instance setups, or we're just trying to be future proof? Because if the later is true, we could just implement better queue implementations in future PRs.

lunny · 2020-03-19T06:14:43Z

@zeripath Out topic. My idea is we need a cluster mod, that there will be an etcd to store all the gitea servers information and one will be elected as master. Then only one instance(master) will create the cron task when the time is in. But the tasks will be queued, so that the tasks will be handled by all the gitea servers load balance.

For a standalone mode, only a master node, it will produce tasks when the time in and handle the tasks itself.

There is no DB flag, because all instances will have the shared DB data. But the tasks could be stored in database task table.

guillep2k

Wow! It was intimidating, but not as hard to review as I thought. 😅

Just some items to clarify. It looks very good! 🎉

models/user.go

modules/cron/cron.go

modules/cron/tasks.go

guillep2k · 2020-04-06T05:01:33Z

modules/cron/tasks.go

+
+	if config.IsEnabled() {
+		// We cannot use the entry return as there is no way to lock it
+		if _, err = c.AddJob(name, config.GetSchedule(), task); err != nil {


I guess this will not duplicate or re-schedule tasks registered from previous instances (like those with @anually schedule). 😶

TBH we need a database backend for these "@annually" scheduled tasks. That probably involves us either bringing cron in to Gitea or forking it.

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

guillep2k

Just one nit, non blocking. LG-TM.

modules/cron/cron.go

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

codecov-io · 2020-04-30T18:59:32Z

Codecov Report

Merging #10745 into master will increase coverage by 0.05%.
The diff coverage is 45.48%.

@@            Coverage Diff             @@
##           master   #10745      +/-   ##
==========================================
+ Coverage   43.84%   43.89%   +0.05%     
==========================================
  Files         607      611       +4     
  Lines       86893    87207     +314     
==========================================
+ Hits        38098    38283     +185     
- Misses      44093    44206     +113     
- Partials     4702     4718      +16

Impacted Files	Coverage Δ
models/error.go	`31.94% <0.00%> (-0.51%)`	⬇️
modules/auth/admin.go	`0.00% <ø> (ø)`
modules/git/git.go	`35.95% <0.00%> (ø)`
modules/repository/check.go	`0.00% <0.00%> (ø)`
modules/repository/hooks.go	`20.30% <0.00%> (-0.16%)`	⬇️
modules/setting/setting.go	`45.19% <ø> (-0.09%)`	⬇️
routers/admin/admin.go	`11.30% <0.00%> (+1.01%)`	⬆️
services/mirror/mirror.go	`18.56% <0.00%> (-0.19%)`	⬇️
models/user.go	`49.18% <6.89%> (-0.49%)`	⬇️
modules/cron/cron.go	`25.00% <20.00%> (-22.20%)`	⬇️
... and 16 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 b7c82cd...8f779b1. Read the comment docs.

guillep2k · 2020-05-02T06:03:05Z

@lafriks can you review again?

lunny · 2020-05-02T06:33:22Z

I think we can create a timer events source to create tasks to queue and reuse our old queue style.

zeripath · 2020-05-03T21:42:12Z

@lunny Mostly that's orthogonal to this work.

We'd need a different type of unique queue, one that leaves the item in the set until it's been processed. It's possible - we'd need to sort adjust the handler to remove the item as a defer after handling.

But as I say that's mostly orthogonal to these changes.

zeripath · 2020-05-12T22:17:43Z

@lunny do you have any further comments?

lafriks · 2020-05-16T21:25:43Z

@zeripath should we move this to 1.13 also?

zeripath · 2020-05-16T21:43:23Z

Unlike the other one I moved to 1.13 this is far less likely to cause problems.

I think it'd be a shame for this not to get in to 1.12.

techknowlogick · 2020-05-16T21:44:31Z

I prefer this to get into 1.12 as well

Fixed

6543 · 2020-05-16T23:30:38Z

🚀

After go-gitea#10745 was merged, the number of System Notices increased obviously. Add the type filter is useful to help users to quickly check out the useful message they want. Signed-off-by: a1012112796 <1012112796@qq.com>

somera · 2020-06-18T22:00:53Z

Merge Cron and Dashboard tasks and always run in the background

Make every cron task report a system notice on completion

Refactor the creation of these tasks

Ensure that execution counts of tasks is correct

Allow cron tasks to be started from the cron page

Make cron tasks appear as processes in the process manager

Fix #10299

The GC isn't working in the 1.12.0 release.

6543 · 2020-06-18T22:49:25Z

@somera can you create a new issue - referencing old pulls/issue is the way to go - but dont append new stuff to closed/merged one :)

Unfortunately go-gitea#10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net>

Unfortunately #10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv>

Backport go-gitea#12139 Unfortunately go-gitea#10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv>

Backport #12139 Unfortunately #10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: techknowlogick <techknowlogick@gitea.io>

@guillep2k

* Refactor Cron and merge dashboard tasks * Merge Cron and Dashboard tasks * Make every cron task report a system notice on completion * Refactor the creation of these tasks * Ensure that execution counts of tasks is correct * Allow cron tasks to be started from the cron page * golangci-lint fixes * Enforce that only one task with the same name can be registered Signed-off-by: Andrew Thornton <art27@cantab.net> * fix name check Signed-off-by: Andrew Thornton <art27@cantab.net> * as per @guillep2k * as per @lafriks Signed-off-by: Andrew Thornton <art27@cantab.net> * Add git.CommandContext variants Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io>

Unfortunately go-gitea#10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv>

Refactor Cron and merge dashboard tasks

5cf1bde

* Merge Cron and Dashboard tasks * Make every cron task report a system notice on completion * Refactor the creation of these tasks * Ensure that execution counts of tasks is correct * Allow cron tasks to be started from the cron page

zeripath added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 16, 2020

zeripath added this to the 1.12.0 milestone Mar 16, 2020

zeripath mentioned this pull request Mar 16, 2020

Make admin git gc tasks run in background #10629

Closed

golangci-lint fixes

36bfaf7

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 17, 2020

guillep2k reviewed Apr 6, 2020

View reviewed changes

zeripath added 3 commits April 6, 2020 15:32

Enforce that only one task with the same name can be registered

e99ea6e

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

Merge branch 'master' into refactor-cron

0968401

fix name check

fb70660

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

guillep2k approved these changes Apr 8, 2020

View reviewed changes

modules/cron/cron.go Show resolved Hide resolved

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 8, 2020

zeripath and others added 3 commits April 8, 2020 16:16

as per @guillep2k

040defa

Merge branch 'master' into refactor-cron

c3edaac

Merge branch 'master' into refactor-cron

1906a1c

lafriks requested changes Apr 27, 2020

View reviewed changes

modules/cron/cron.go Show resolved Hide resolved

modules/cron/cron.go Outdated Show resolved Hide resolved

zeripath added 3 commits April 28, 2020 17:42

as per @lafriks

9fe4984

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

Merge remote-tracking branch 'origin/master' into refactor-cron

5396293

Merge remote-tracking branch 'origin/master' into refactor-cron

f1db32c

Merge branch 'master' into refactor-cron

8f779b1

Merge branch 'master' into refactor-cron

94fad5c

Merge branch 'master' into refactor-cron

f59cfa3

techknowlogick approved these changes May 16, 2020

View reviewed changes

lafriks requested a review from lunny May 16, 2020 19:41

Merge branch 'master' into refactor-cron

d52e277

techknowlogick merged commit 9a2e47b into go-gitea:master May 16, 2020

zeripath deleted the refactor-cron branch May 17, 2020 00:42

6543 mentioned this pull request May 17, 2020

Fix "Unknown task: git_fsck" error on admin dashboard #11468

Merged

a1012112796 mentioned this pull request Jun 16, 2020

Add type filter to System Notices view #11909

Closed

jolheiser mentioned this pull request Jun 18, 2020

Garbage-Collection not working #10299

Closed

7 tasks

somera mentioned this pull request Jun 18, 2020

Garbage-Collection not working #11967

Closed

7 tasks

zeripath added a commit to zeripath/gitea that referenced this pull request Jul 4, 2020

Remove spurious logging

e485ae3

Unfortunately go-gitea#10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath mentioned this pull request Jul 4, 2020

Remove spurious logging of " Delete all repository archives" at startup #12139

Merged

lunny pushed a commit that referenced this pull request Jul 5, 2020

Remove spurious logging (#12139)

c6d6c51

Unfortunately #10745 merged a spurious logging message. This PR removes this. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv>

zeripath mentioned this pull request Jul 5, 2020

Remove spurious logging (#12139) #12148

Merged

kolaente mentioned this pull request Oct 11, 2020

Update golangci-lint to version 1.31.0 #13102

Merged

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Cron and merge dashboard tasks #10745

Refactor Cron and merge dashboard tasks #10745

zeripath commented Mar 16, 2020 •

edited

Loading

lunny commented Mar 17, 2020

zeripath commented Mar 17, 2020

guillep2k commented Mar 18, 2020

lunny commented Mar 19, 2020 •

edited

Loading

guillep2k left a comment

guillep2k Apr 6, 2020

zeripath Apr 6, 2020

guillep2k left a comment

codecov-io commented Apr 30, 2020 •

edited

Loading

guillep2k commented May 2, 2020

lunny commented May 2, 2020

zeripath commented May 3, 2020

zeripath commented May 12, 2020

lafriks commented May 16, 2020

zeripath commented May 16, 2020

techknowlogick commented May 16, 2020

6543 commented May 16, 2020

somera commented Jun 18, 2020

6543 commented Jun 18, 2020

Refactor Cron and merge dashboard tasks #10745

Refactor Cron and merge dashboard tasks #10745

Conversation

zeripath commented Mar 16, 2020 • edited Loading

lunny commented Mar 17, 2020

zeripath commented Mar 17, 2020

guillep2k commented Mar 18, 2020

lunny commented Mar 19, 2020 • edited Loading

guillep2k left a comment

Choose a reason for hiding this comment

guillep2k Apr 6, 2020

Choose a reason for hiding this comment

zeripath Apr 6, 2020

Choose a reason for hiding this comment

guillep2k left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 30, 2020 • edited Loading

Codecov Report

guillep2k commented May 2, 2020

lunny commented May 2, 2020

zeripath commented May 3, 2020

zeripath commented May 12, 2020

lafriks commented May 16, 2020

zeripath commented May 16, 2020

techknowlogick commented May 16, 2020

6543 commented May 16, 2020

somera commented Jun 18, 2020

6543 commented Jun 18, 2020

zeripath commented Mar 16, 2020 •

edited

Loading

lunny commented Mar 19, 2020 •

edited

Loading

codecov-io commented Apr 30, 2020 •

edited

Loading