From e7b19e93a440a0beb9e49f6ccce3d9d142a78562 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 6 Nov 2021 14:39:43 +0000 Subject: [PATCH 1/6] Add `PULL_LIMIT` and `PUSH_LIMIT` to cron.update_mirror task Add `PULL_LIMIT` and `PUSH_LIMIT` to cron.update_mirror task to limit the number of mirrors added to the queue each time the cron task is run. Fix #16982 Signed-off-by: Andrew Thornton --- custom/conf/app.example.ini | 4 ++ .../doc/advanced/config-cheat-sheet.en-us.md | 2 + models/repo_mirror.go | 1 + models/repo_pushmirror.go | 1 + modules/cron/tasks_basic.go | 25 +++++++--- services/mirror/mirror.go | 47 ++++++++++++++++--- 6 files changed, 67 insertions(+), 13 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index eadc1c0d9625..7163d04c4623 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1705,6 +1705,10 @@ PATH = ;RUN_AT_START = false ;; Notice if not success ;NO_SUCCESS_NOTICE = true +;; Limit the number of mirrors added to the queue to this number (set to -1 to add all) +;PULL_LIMIT=50 +;; Limit the number of mirrors added to the queue to this number (set to -1 to add all) +;PUSH_LIMIT=50 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 6cc6043cae7d..e67776d20e75 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -791,6 +791,8 @@ NB: You must have `DISABLE_ROUTER_LOG` set to `false` for this option to take ef - `SCHEDULE`: **@every 10m**: Cron syntax for scheduling update mirrors, e.g. `@every 3h`. - `NO_SUCCESS_NOTICE`: **true**: The cron task for update mirrors success report is not very useful - as it just means that the mirrors have been queued. Therefore this is turned off by default. +- `PULL_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (set to -1 to add all). +- `PUSH_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (set to -1 to add all). #### Cron - Repository Health Check (`cron.repo_health_check`) diff --git a/models/repo_mirror.go b/models/repo_mirror.go index 35685b322096..e28d0d25174d 100644 --- a/models/repo_mirror.go +++ b/models/repo_mirror.go @@ -119,6 +119,7 @@ func MirrorsIterate(f func(idx int, bean interface{}) error) error { return db.GetEngine(db.DefaultContext). Where("next_update_unix<=?", time.Now().Unix()). And("next_update_unix!=0"). + OrderBy("updated_unix ASC"). Iterate(new(Mirror), f) } diff --git a/models/repo_pushmirror.go b/models/repo_pushmirror.go index c6207bae6df6..38a1a66947f9 100644 --- a/models/repo_pushmirror.go +++ b/models/repo_pushmirror.go @@ -107,5 +107,6 @@ func PushMirrorsIterate(f func(idx int, bean interface{}) error) error { return db.GetEngine(db.DefaultContext). Where("last_update + (`interval` / ?) <= ?", time.Second, time.Now().Unix()). And("`interval` != 0"). + OrderBy("last_update ASC"). Iterate(new(PushMirror), f) } diff --git a/modules/cron/tasks_basic.go b/modules/cron/tasks_basic.go index 6c61d628c52f..f0be11f203cd 100644 --- a/modules/cron/tasks_basic.go +++ b/modules/cron/tasks_basic.go @@ -17,13 +17,24 @@ import ( ) func registerUpdateMirrorTask() { - RegisterTaskFatal("update_mirrors", &BaseConfig{ - Enabled: true, - RunAtStart: false, - Schedule: "@every 10m", - NoSuccessNotice: true, - }, func(ctx context.Context, _ *models.User, _ Config) error { - return mirror_service.Update(ctx) + type UpdateMirrorTaskConfig struct { + BaseConfig + PullLimit int + PushLimit int + } + + RegisterTaskFatal("update_mirrors", &UpdateMirrorTaskConfig{ + BaseConfig: BaseConfig{ + Enabled: true, + RunAtStart: false, + Schedule: "@every 10m", + NoSuccessNotice: true, + }, + PullLimit: 50, + PushLimit: 50, + }, func(ctx context.Context, _ *models.User, cfg Config) error { + umtc := cfg.(*UpdateMirrorTaskConfig) + return mirror_service.Update(ctx, umtc.PullLimit, umtc.PushLimit) }) } diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index eb37639beff8..f885ea323c63 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -46,14 +46,18 @@ func doMirrorSync(ctx context.Context, req *SyncRequest) { } // Update checks and updates mirror repositories. -func Update(ctx context.Context) error { +func Update(ctx context.Context, pullLimit, pushLimit int) error { if !setting.Mirror.Enabled { log.Warn("Mirror feature disabled, but cron job enabled: skip update") return nil } log.Trace("Doing: Update") - handler := func(idx int, bean interface{}) error { + requested := 0 + + doneErr := fmt.Errorf("reached limit") + + handler := func(idx int, bean interface{}, limit int) error { var item SyncRequest if m, ok := bean.(*models.Mirror); ok { if m.Repo == nil { @@ -78,19 +82,50 @@ func Update(ctx context.Context) error { return nil } + // Check we've not been cancelled select { case <-ctx.Done(): - return fmt.Errorf("Aborted") + return fmt.Errorf("aborted") default: - return mirrorQueue.Push(&item) } + + // Check if this request is already in the queue + has, err := mirrorQueue.Has(&item) + if err != nil { + return err + } + if has { + return nil + } + + // Double check we've not been cancelled + select { + case <-ctx.Done(): + return fmt.Errorf("aborted") + default: + } + + // Push to the Queue + if err := mirrorQueue.Push(&item); err != nil { + return err + } + + requested++ + if limit > 0 && requested > limit { + return doneErr + } + return nil } - if err := models.MirrorsIterate(handler); err != nil { + if err := models.MirrorsIterate(func(idx int, bean interface{}) error { + return handler(idx, bean, pullLimit) + }); err != nil && err != doneErr { log.Error("MirrorsIterate: %v", err) return err } - if err := models.PushMirrorsIterate(handler); err != nil { + if err := models.PushMirrorsIterate(func(idx int, bean interface{}) error { + return handler(idx, bean, pushLimit) + }); err != nil && err != doneErr { log.Error("PushMirrorsIterate: %v", err) return err } From 7e4c3ae620938b2c0aef001209c1716e49080f09 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 20 Nov 2021 21:15:27 +0000 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: delvh --- custom/conf/app.example.ini | 4 ++-- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index a0980b5eda4e..9514e5616562 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1706,9 +1706,9 @@ PATH = ;RUN_AT_START = false ;; Notice if not success ;NO_SUCCESS_NOTICE = true -;; Limit the number of mirrors added to the queue to this number (set to -1 to add all) +;; Limit the number of mirrors added to the queue to this number (negative values mean no limit) ;PULL_LIMIT=50 -;; Limit the number of mirrors added to the queue to this number (set to -1 to add all) +;; Limit the number of mirrors added to the queue to this number (negative values mean no limit) ;PUSH_LIMIT=50 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 5c8807131b4a..764d4eb0ede0 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -791,8 +791,8 @@ NB: You must have `DISABLE_ROUTER_LOG` set to `false` for this option to take ef - `SCHEDULE`: **@every 10m**: Cron syntax for scheduling update mirrors, e.g. `@every 3h`. - `NO_SUCCESS_NOTICE`: **true**: The cron task for update mirrors success report is not very useful - as it just means that the mirrors have been queued. Therefore this is turned off by default. -- `PULL_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (set to -1 to add all). -- `PUSH_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (set to -1 to add all). +- `PULL_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit). +- `PUSH_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit). #### Cron - Repository Health Check (`cron.repo_health_check`) From bc46c8790c5456ad415bb5afc32228aa9d6338e2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 20 Nov 2021 21:29:39 +0000 Subject: [PATCH 3/6] as per review Signed-off-by: Andrew Thornton --- services/mirror/mirror.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index f885ea323c63..eb3879c0455e 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -45,6 +45,8 @@ func doMirrorSync(ctx context.Context, req *SyncRequest) { } } +var limitErr = fmt.Errorf("reached limit") + // Update checks and updates mirror repositories. func Update(ctx context.Context, pullLimit, pushLimit int) error { if !setting.Mirror.Enabled { @@ -55,8 +57,6 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { requested := 0 - doneErr := fmt.Errorf("reached limit") - handler := func(idx int, bean interface{}, limit int) error { var item SyncRequest if m, ok := bean.(*models.Mirror); ok { @@ -112,22 +112,26 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { requested++ if limit > 0 && requested > limit { - return doneErr + return limitErr } return nil } - if err := models.MirrorsIterate(func(idx int, bean interface{}) error { - return handler(idx, bean, pullLimit) - }); err != nil && err != doneErr { - log.Error("MirrorsIterate: %v", err) - return err + if pullLimit != 0 { + if err := models.MirrorsIterate(func(idx int, bean interface{}) error { + return handler(idx, bean, pullLimit) + }); err != nil && err != limitErr { + log.Error("MirrorsIterate: %v", err) + return err + } } - if err := models.PushMirrorsIterate(func(idx int, bean interface{}) error { - return handler(idx, bean, pushLimit) - }); err != nil && err != doneErr { - log.Error("PushMirrorsIterate: %v", err) - return err + if pushLimit != 0 { + if err := models.PushMirrorsIterate(func(idx int, bean interface{}) error { + return handler(idx, bean, pushLimit) + }); err != nil && err != limitErr { + log.Error("PushMirrorsIterate: %v", err) + return err + } } log.Trace("Finished: Update") return nil From d0e4e2d0e54df319393818e1f062854c950c1361 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 20 Nov 2021 22:04:22 +0000 Subject: [PATCH 4/6] placate lint Signed-off-by: Andrew Thornton --- services/mirror/mirror.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index eb3879c0455e..ed55d42cad4c 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -45,7 +45,7 @@ func doMirrorSync(ctx context.Context, req *SyncRequest) { } } -var limitErr = fmt.Errorf("reached limit") +var errLimit = fmt.Errorf("reached limit") // Update checks and updates mirror repositories. func Update(ctx context.Context, pullLimit, pushLimit int) error { @@ -112,7 +112,7 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { requested++ if limit > 0 && requested > limit { - return limitErr + return errLimit } return nil } @@ -120,7 +120,7 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { if pullLimit != 0 { if err := models.MirrorsIterate(func(idx int, bean interface{}) error { return handler(idx, bean, pullLimit) - }); err != nil && err != limitErr { + }); err != nil && err != errLimit { log.Error("MirrorsIterate: %v", err) return err } @@ -128,7 +128,7 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { if pushLimit != 0 { if err := models.PushMirrorsIterate(func(idx int, bean interface{}) error { return handler(idx, bean, pushLimit) - }); err != nil && err != limitErr { + }); err != nil && err != errLimit { log.Error("PushMirrorsIterate: %v", err) return err } From f5c7b46c75f04ee063d7e68e00bddd50cb5d6a51 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 22 Nov 2021 17:36:54 +0000 Subject: [PATCH 5/6] as per review Signed-off-by: Andrew Thornton --- services/mirror/mirror.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index ed55d42cad4c..dae6f2807b6b 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -98,13 +98,6 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { return nil } - // Double check we've not been cancelled - select { - case <-ctx.Done(): - return fmt.Errorf("aborted") - default: - } - // Push to the Queue if err := mirrorQueue.Push(&item); err != nil { return err From fe40616e491f3928a58374cc84e1f2975cf92e26 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 22 Nov 2021 18:49:00 +0000 Subject: [PATCH 6/6] Add explicit documentation Signed-off-by: Andrew Thornton --- custom/conf/app.example.ini | 6 ++++-- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 9ebd37d9fadf..18985e04e954 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1719,9 +1719,11 @@ PATH = ;RUN_AT_START = false ;; Notice if not success ;NO_SUCCESS_NOTICE = true -;; Limit the number of mirrors added to the queue to this number (negative values mean no limit) +;; Limit the number of mirrors added to the queue to this number +;; (negative values mean no limit, 0 will result in no result in no mirrors being queued effectively disabling pull mirror updating.) ;PULL_LIMIT=50 -;; Limit the number of mirrors added to the queue to this number (negative values mean no limit) +;; Limit the number of mirrors added to the queue to this number +;; (negative values mean no limit, 0 will result in no mirrors being queued effectively disabling push mirror updating) ;PUSH_LIMIT=50 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 06229c2a6090..3e9b27961418 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -828,8 +828,8 @@ NB: You must have `DISABLE_ROUTER_LOG` set to `false` for this option to take ef - `SCHEDULE`: **@every 10m**: Cron syntax for scheduling update mirrors, e.g. `@every 3h`. - `NO_SUCCESS_NOTICE`: **true**: The cron task for update mirrors success report is not very useful - as it just means that the mirrors have been queued. Therefore this is turned off by default. -- `PULL_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit). -- `PUSH_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit). +- `PULL_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit, 0 will result in no mirrors being queued effectively disabling pull mirror updating). +- `PUSH_LIMIT`: **50**: Limit the number of mirrors added to the queue to this number (negative values mean no limit, 0 will result in no mirrors being queued effectively disabling push mirror updating). #### Cron - Repository Health Check (`cron.repo_health_check`)