From e1c9664023f40c39b3df2ee3a438676b5f6d95d6 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Mon, 13 Apr 2020 22:39:52 -0500 Subject: [PATCH 01/31] sketching out some of the code to add ability to prune hook_task table by repository --- models/migrations/migrations.go | 2 ++ models/migrations/v136.go | 28 ++++++++++++++++++++++++++++ models/repo.go | 2 ++ modules/auth/repo_form.go | 2 ++ modules/repository/create.go | 2 ++ modules/setting/repository.go | 4 ++++ options/locale/locale_en-US.ini | 2 ++ templates/repo/settings/options.tmpl | 8 ++++++++ 8 files changed, 50 insertions(+) create mode 100644 models/migrations/v136.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cad7f05f15989..4b819b197a9e0 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -204,6 +204,8 @@ var migrations = []Migration{ NewMigration("Refix merge base for merged pull requests", refixMergeBase), // v135 -> 136 NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), + // v136 -> 137 + NewMigration("Add EnableHookTaskPurge and NumberWebhookDeliveriesToKeep columns to Repository table", addHookTaskPurge) } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v136.go b/models/migrations/v136.go new file mode 100644 index 0000000000000..eab3c3ea09baa --- /dev/null +++ b/models/migrations/v136.go @@ -0,0 +1,28 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/modules/setting" + + "xorm.io/xorm" +) + +func addHookTaskPurge(x *xorm.Engine) error { + + type Repository struct { + ID int64 `xorm:"pk autoincr"` + EnableHookTaskPurge bool `xorm:"NOT NULL DEFAULT true"` + NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` + } + + if err := x.Sync2(new(Repository)); err != nil { + return err + } + + _, err := x.Exec("UPDATE repository SET enable_hook_task_purge = ?", number_webhook_deliveries_to_keep = ? + setting.Repository.DefaultEnableHookTaskPurge, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) + return err +} diff --git a/models/repo.go b/models/repo.go index a901c3476b421..445b6653af8c6 100644 --- a/models/repo.go +++ b/models/repo.go @@ -189,6 +189,8 @@ type Repository struct { StatsIndexerStatus *RepoIndexerStatus `xorm:"-"` IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` + EnableHookTaskPurge bool `xorm:"NOT NULL DEFAULT true"` + NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` Topics []string `xorm:"TEXT JSON"` // Avatar: ID(10-20)-md5(32) - must fit into 64 symbols diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 80ed314196665..4cf4dd45790e7 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -142,6 +142,8 @@ type RepoSettingForm struct { // Admin settings EnableHealthCheck bool EnableCloseIssuesViaCommitInAnyBranch bool + EnableHookTaskPurge bool + NumberWebhookDeliveriesToKeep int64 } // Validate validates the fields diff --git a/modules/repository/create.go b/modules/repository/create.go index 5c0aae30da6bb..3cc03998cd772 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -35,6 +35,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m IsPrivate: opts.IsPrivate, IsFsckEnabled: !opts.IsMirror, CloseIssuesViaCommitInAnyBranch: setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch, + EnableHookTaskPurge setting.Repository.DefaultEnableHookTaskPurge + NumberWebhookDeliveriesToKeep setting.Repository.DefaultNumberWebhookDeliveriesToKeep Status: opts.Status, IsEmpty: !opts.AutoInit, } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 8af3eaaf46933..1b3d5addf9296 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -35,6 +35,8 @@ var ( AccessControlAllowOrigin string UseCompatSSHURI bool DefaultCloseIssuesViaCommitsInAnyBranch bool + DefaultEnableHookTaskPurge bool + DefaultNumberWebhookDeliveriesToKeep int64 EnablePushCreateUser bool EnablePushCreateOrg bool DisabledRepoUnits []string @@ -99,6 +101,8 @@ var ( AccessControlAllowOrigin: "", UseCompatSSHURI: false, DefaultCloseIssuesViaCommitsInAnyBranch: false, + DefaultEnableHookTaskPurge: true, + DefaultNumberWebhookDeliveriesToKeep: 10, EnablePushCreateUser: false, EnablePushCreateOrg: false, DisabledRepoUnits: []string{}, diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 8b43115c07a3b..a565867728b4a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1330,6 +1330,8 @@ settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits settings.admin_settings = Administrator Settings settings.admin_enable_health_check = Enable Repository Health Checks (git fsck) settings.admin_enable_close_issues_via_commit_in_any_branch = Close an issue via a commit made in a non default branch +settings.admin_enable_hook_task_purge_1 = Purge webhook delivery notifications, keep most recent +settings.admin_enable_hook_task_purge_2 = deliveries settings.danger_zone = Danger Zone settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name. settings.convert = Convert to Regular Repository diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index c674fcf7f962e..1c9f25788fd5f 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -343,6 +343,14 @@ + +
+ + + + + +
From ee83fe7eca51e7187e58b630f3eedc9fa744ce76 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sun, 19 Apr 2020 22:30:02 -0500 Subject: [PATCH 02/31] stubbed out more code for prune hook_task --- custom/conf/app.ini.sample | 5 +++++ .../doc/advanced/config-cheat-sheet.en-us.md | 5 +++++ models/migrations/migrations.go | 2 +- models/migrations/v136.go | 4 ++-- models/repo.go | 2 +- modules/auth/repo_form.go | 2 +- modules/cron/cron.go | 20 +++++++++++++++++ modules/repository/create.go | 4 ++-- modules/repository/generate.go | 22 ++++++++++--------- modules/repository/prune_hook_task.go | 21 ++++++++++++++++++ modules/setting/cron.go | 14 ++++++++++++ modules/setting/repository.go | 4 ++-- routers/repo/setting.go | 8 +++++++ templates/repo/settings/options.tmpl | 4 ++-- 14 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 modules/repository/prune_hook_task.go diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index 86224ab8c6920..240f45418dfe1 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -857,6 +857,11 @@ UPDATE_EXISTING = true ; Interval as a duration between each synchronization. (default every 24h) SCHEDULE = @every 24h +; Prune hook_task table +[cron.prune_hook_task_table] +RUN_AT_START = true +SCHEDULE = @every 24h + [git] ; The path of git executable. If empty, Gitea searches through the PATH environment. PATH = 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 2306dd2281e61..500ccab64b952 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -545,6 +545,11 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` - `SCHEDULE`: **@every 24h** : Interval as a duration between each synchronization, it will always attempt synchronization when the instance starts. +### Cron - Prune hook_task Table (`cron.prune_hook_task_table`) + +- `RUN_AT_START`: **true**: Run prune hook_task at start time. +- `SCHEDULE`: **@every 24h**: Cron syntax for pruning hook_task table. + ## Git (`git`) - `PATH`: **""**: The path of git executable. If empty, Gitea searches through the PATH environment. diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4b819b197a9e0..4b6b3779570cf 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -205,7 +205,7 @@ var migrations = []Migration{ // v135 -> 136 NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), // v136 -> 137 - NewMigration("Add EnableHookTaskPurge and NumberWebhookDeliveriesToKeep columns to Repository table", addHookTaskPurge) + NewMigration("Add IsHookTaskPurgeEnabled and NumberWebhookDeliveriesToKeep columns to Repository table", addHookTaskPurge), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v136.go b/models/migrations/v136.go index eab3c3ea09baa..448448416282d 100644 --- a/models/migrations/v136.go +++ b/models/migrations/v136.go @@ -14,7 +14,7 @@ func addHookTaskPurge(x *xorm.Engine) error { type Repository struct { ID int64 `xorm:"pk autoincr"` - EnableHookTaskPurge bool `xorm:"NOT NULL DEFAULT true"` + IsHookTaskPurgeEnabled bool `xorm:"NOT NULL DEFAULT true"` NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` } @@ -23,6 +23,6 @@ func addHookTaskPurge(x *xorm.Engine) error { } _, err := x.Exec("UPDATE repository SET enable_hook_task_purge = ?", number_webhook_deliveries_to_keep = ? - setting.Repository.DefaultEnableHookTaskPurge, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) + setting.Repository.DefaultIsHookTaskPurgeEnabled, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) return err } diff --git a/models/repo.go b/models/repo.go index 445b6653af8c6..ea555fbb83381 100644 --- a/models/repo.go +++ b/models/repo.go @@ -189,7 +189,7 @@ type Repository struct { StatsIndexerStatus *RepoIndexerStatus `xorm:"-"` IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` - EnableHookTaskPurge bool `xorm:"NOT NULL DEFAULT true"` + IsHookTaskPurgeEnabled bool `xorm:"NOT NULL DEFAULT true"` NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` Topics []string `xorm:"TEXT JSON"` diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 4cf4dd45790e7..d6e5f34bfb55a 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -142,7 +142,7 @@ type RepoSettingForm struct { // Admin settings EnableHealthCheck bool EnableCloseIssuesViaCommitInAnyBranch bool - EnableHookTaskPurge bool + IsHookTaskPurgeEnabled bool NumberWebhookDeliveriesToKeep int64 } diff --git a/modules/cron/cron.go b/modules/cron/cron.go index 692642e4cec2a..64b8904fc9ad6 100644 --- a/modules/cron/cron.go +++ b/modules/cron/cron.go @@ -29,6 +29,7 @@ const ( syncExternalUsers = "sync_external_users" deletedBranchesCleanup = "deleted_branches_cleanup" updateMigrationPosterID = "update_migration_post_id" + pruneHookTaskTable = "prune_hook_task_table" ) var c = cron.New() @@ -132,6 +133,25 @@ func NewContext() { go WithUnique(deletedBranchesCleanup, models.RemoveOldDeletedBranches)() } } + if setting.Cron.PruneHookTaskTable.Enabled { + entry, err = c.AddFunc("Prune hook_task table", setting.Cron.PruneHookTaskTable.Schedule, WithUnique(pruneHookTaskTable, func(ctx context.Context) { + if err := repo_module.PruneHookTaskTable(ctx); err != nil { + log.Error("PruneHookTaskTable: %s", err) + } + })) + if err != nil { + log.Fatal("Cron[Repository prune hook_task table]: %v", err) + } + if setting.Cron.RepoHealthCheck.RunAtStart { + entry.Prev = time.Now() + entry.ExecTimes++ + go WithUnique(pruneHookTaskTable, func(ctx context.Context) { + if err := repo_module.PruneHookTaskTable(ctx); err != nil { + log.Error("PruneHookTaskTable: %s", err) + } + })() + } + } entry, err = c.AddFunc("Update migrated repositories' issues and comments' posterid", setting.Cron.UpdateMigrationPosterID.Schedule, WithUnique(updateMigrationPosterID, migrations.UpdateMigrationPosterID)) if err != nil { diff --git a/modules/repository/create.go b/modules/repository/create.go index 3cc03998cd772..98934b02ddc10 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -35,8 +35,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m IsPrivate: opts.IsPrivate, IsFsckEnabled: !opts.IsMirror, CloseIssuesViaCommitInAnyBranch: setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch, - EnableHookTaskPurge setting.Repository.DefaultEnableHookTaskPurge - NumberWebhookDeliveriesToKeep setting.Repository.DefaultNumberWebhookDeliveriesToKeep + IsHookTaskPurgeEnabled: setting.Repository.DefaultIsHookTaskPurgeEnabled, + NumberWebhookDeliveriesToKeep: setting.Repository.DefaultNumberWebhookDeliveriesToKeep, Status: opts.Status, IsEmpty: !opts.AutoInit, } diff --git a/modules/repository/generate.go b/modules/repository/generate.go index 6d80488de786c..6e4f4cd9b3afd 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -232,16 +232,18 @@ func GenerateGitContent(ctx models.DBContext, templateRepo, generateRepo *models // GenerateRepository generates a repository from a template func GenerateRepository(ctx models.DBContext, doer, owner *models.User, templateRepo *models.Repository, opts models.GenerateRepoOptions) (_ *models.Repository, err error) { generateRepo := &models.Repository{ - OwnerID: owner.ID, - Owner: owner, - OwnerName: owner.Name, - Name: opts.Name, - LowerName: strings.ToLower(opts.Name), - Description: opts.Description, - IsPrivate: opts.Private, - IsEmpty: !opts.GitContent || templateRepo.IsEmpty, - IsFsckEnabled: templateRepo.IsFsckEnabled, - TemplateID: templateRepo.ID, + OwnerID: owner.ID, + Owner: owner, + OwnerName: owner.Name, + Name: opts.Name, + LowerName: strings.ToLower(opts.Name), + Description: opts.Description, + IsPrivate: opts.Private, + IsEmpty: !opts.GitContent || templateRepo.IsEmpty, + IsFsckEnabled: templateRepo.IsFsckEnabled, + IsHookTaskPurgeEnabled: templateRepo.IsHookTaskPurgeEnabled, + NumberWebhookDeliveriesToKeep: templateRepo.NumberWebhookDeliveriesToKeep, + TemplateID: templateRepo.ID, } if err = models.CreateRepository(ctx, doer, owner, generateRepo); err != nil { diff --git a/modules/repository/prune_hook_task.go b/modules/repository/prune_hook_task.go new file mode 100644 index 0000000000000..e9c6001fe1ed7 --- /dev/null +++ b/modules/repository/prune_hook_task.go @@ -0,0 +1,21 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repository + +import ( + "context" + + "code.gitea.io/gitea/modules/log" +) + +// PruneHookTaskTable deletes rows from hook_task as needed. +func PruneHookTaskTable(ctx context.Context) error { + log.Trace("Doing: PruneHookTaskTable") + + //TODO implement me!!!! + + log.Trace("Finished: PruneHookTaskTable") + return nil +} diff --git a/modules/setting/cron.go b/modules/setting/cron.go index 77f55168aa3b7..d26ff985f6d13 100644 --- a/modules/setting/cron.go +++ b/modules/setting/cron.go @@ -52,6 +52,11 @@ var ( UpdateMigrationPosterID struct { Schedule string } `ini:"cron.update_migration_poster_id"` + PruneHookTaskTable struct { + Enabled bool + RunAtStart bool + Schedule string + } `ini:"cron.prune_hook_task_table"` }{ UpdateMirror: struct { Enabled bool @@ -122,6 +127,15 @@ var ( }{ Schedule: "@every 24h", }, + PruneHookTaskTable: struct { + Enabled bool + RunAtStart bool + Schedule string + }{ + Enabled: true, + RunAtStart: true, + Schedule: "@every 24h", + }, } ) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 1b3d5addf9296..0e6602b6cb818 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -35,7 +35,7 @@ var ( AccessControlAllowOrigin string UseCompatSSHURI bool DefaultCloseIssuesViaCommitsInAnyBranch bool - DefaultEnableHookTaskPurge bool + DefaultIsHookTaskPurgeEnabled bool DefaultNumberWebhookDeliveriesToKeep int64 EnablePushCreateUser bool EnablePushCreateOrg bool @@ -101,7 +101,7 @@ var ( AccessControlAllowOrigin: "", UseCompatSSHURI: false, DefaultCloseIssuesViaCommitsInAnyBranch: false, - DefaultEnableHookTaskPurge: true, + DefaultIsHookTaskPurgeEnabled: true, DefaultNumberWebhookDeliveriesToKeep: 10, EnablePushCreateUser: false, EnablePushCreateOrg: false, diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 7a2db88c1f422..1bd745f2fe8cd 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -322,6 +322,14 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { repo.CloseIssuesViaCommitInAnyBranch = form.EnableCloseIssuesViaCommitInAnyBranch } + if repo.IsHookTaskPurgeEnabled != form.IsHookTaskPurgeEnabled { + repo.IsHookTaskPurgeEnabled = form.IsHookTaskPurgeEnabled + } + + if repo.NumberWebhookDeliveriesToKeep != form.NumberWebhookDeliveriesToKeep { + repo.NumberWebhookDeliveriesToKeep = form.NumberWebhookDeliveriesToKeep + } + if err := models.UpdateRepository(repo, false); err != nil { ctx.ServerError("UpdateRepository", err) return diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 1c9f25788fd5f..0ca809b654104 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -345,10 +345,10 @@
- + - +
From c4502dca1c521e2fc237f07dfd9dcb8ca10f3978 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Mon, 20 Apr 2020 22:37:32 -0500 Subject: [PATCH 03/31] work on prune hook_task --- models/migrations/v136.go | 4 ++-- models/webhook.go | 6 +++++ modules/cron/cron.go | 3 +++ modules/repository/prune_hook_task.go | 33 ++++++++++++++++++++++++--- templates/repo/settings/options.tmpl | 23 +++++++++++-------- 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/models/migrations/v136.go b/models/migrations/v136.go index 448448416282d..885822e52da64 100644 --- a/models/migrations/v136.go +++ b/models/migrations/v136.go @@ -14,7 +14,7 @@ func addHookTaskPurge(x *xorm.Engine) error { type Repository struct { ID int64 `xorm:"pk autoincr"` - IsHookTaskPurgeEnabled bool `xorm:"NOT NULL DEFAULT true"` + IsHookTaskPurgeEnabled bool `xorm:"NOT NULL DEFAULT true"` NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` } @@ -22,7 +22,7 @@ func addHookTaskPurge(x *xorm.Engine) error { return err } - _, err := x.Exec("UPDATE repository SET enable_hook_task_purge = ?", number_webhook_deliveries_to_keep = ? + _, err := x.Exec("UPDATE repository SET is_hook_task_purge_enabled = ?, number_webhook_deliveries_to_keep = ?", setting.Repository.DefaultIsHookTaskPurgeEnabled, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) return err } diff --git a/models/webhook.go b/models/webhook.go index 5f3a2856915a0..6012d0a803c30 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -798,3 +798,9 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { } return tasks, nil } + +// DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered basedon the paramter +func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { + log.Info("delete delivered hook tasks on repo %d with num to keep %d", repoID, numberDeliveriesToKeep) + return nil +} diff --git a/modules/cron/cron.go b/modules/cron/cron.go index 64b8904fc9ad6..2d7f201d3255a 100644 --- a/modules/cron/cron.go +++ b/modules/cron/cron.go @@ -134,6 +134,7 @@ func NewContext() { } } if setting.Cron.PruneHookTaskTable.Enabled { + log.Error("prune hook task enabled") entry, err = c.AddFunc("Prune hook_task table", setting.Cron.PruneHookTaskTable.Schedule, WithUnique(pruneHookTaskTable, func(ctx context.Context) { if err := repo_module.PruneHookTaskTable(ctx); err != nil { log.Error("PruneHookTaskTable: %s", err) @@ -151,6 +152,8 @@ func NewContext() { } })() } + } else { + log.Error("prune hook task not enabled :(") } entry, err = c.AddFunc("Update migrated repositories' issues and comments' posterid", setting.Cron.UpdateMigrationPosterID.Schedule, WithUnique(updateMigrationPosterID, migrations.UpdateMigrationPosterID)) diff --git a/modules/repository/prune_hook_task.go b/modules/repository/prune_hook_task.go index e9c6001fe1ed7..d02036f0a1194 100644 --- a/modules/repository/prune_hook_task.go +++ b/modules/repository/prune_hook_task.go @@ -6,16 +6,43 @@ package repository import ( "context" + "fmt" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" + "xorm.io/builder" ) // PruneHookTaskTable deletes rows from hook_task as needed. func PruneHookTaskTable(ctx context.Context) error { - log.Trace("Doing: PruneHookTaskTable") + log.Error("Doing: PruneHookTaskTable") - //TODO implement me!!!! + if err := models.Iterate( + models.DefaultDBContext(), + new(models.Repository), + builder.Expr("id>0 AND is_hook_task_purge_enabled=?", true), + func(idx int, bean interface{}) error { + select { + case <-ctx.Done(): + return fmt.Errorf("Aborted due to shutdown") + default: + } + repo := bean.(*models.Repository) + repoPath := repo.RepoPath() + log.Trace("Running prune hook_task table on repository %s", repoPath) + if err := models.DeleteDeliveredHookTasks(repo.ID, repo.NumberWebhookDeliveriesToKeep); err != nil { + desc := fmt.Sprintf("Failed to prune hook_task on repository (%s): %v", repoPath, err) + log.Warn(desc) + if err = models.CreateRepositoryNotice(desc); err != nil { + log.Error("CreateRepositoryNotice: %v", err) + } + } + return nil + }, + ); err != nil { + return err + } - log.Trace("Finished: PruneHookTaskTable") + log.Error("Finished: PruneHookTaskTable") return nil } diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 0ca809b654104..af7b7961d0fc2 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -339,19 +339,22 @@ -
- - +
+
+ + +
-
- - - - - +
+
+ + + + + {{.i18n.Tr "repo.settings.admin_enable_hook_task_purge_2"}} +
-
From 739dfe715f4344b5d89285fb504e5bd960ed6bb8 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Wed, 22 Apr 2020 22:26:54 -0500 Subject: [PATCH 04/31] re-arranged on the settings page --- options/locale/locale_en-US.ini | 4 ++-- templates/repo/settings/options.tmpl | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a565867728b4a..5406b1d6b848f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1330,8 +1330,8 @@ settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits settings.admin_settings = Administrator Settings settings.admin_enable_health_check = Enable Repository Health Checks (git fsck) settings.admin_enable_close_issues_via_commit_in_any_branch = Close an issue via a commit made in a non default branch -settings.admin_enable_hook_task_purge_1 = Purge webhook delivery notifications, keep most recent -settings.admin_enable_hook_task_purge_2 = deliveries +settings.admin_enable_hook_task_purge_1 = Purge webhook delivery notifications +settings.admin_enable_hook_task_purge_2 = Webhook deliveries to keep settings.danger_zone = Danger Zone settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name. settings.convert = Convert to Regular Repository diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index af7b7961d0fc2..3565e99e0fefe 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -345,16 +345,16 @@
-
- - - - - {{.i18n.Tr "repo.settings.admin_enable_hook_task_purge_2"}} + +
+
+ + +
From 2cdecc19326c3adfd9c0d207ebd45fd64b0b4010 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Thu, 23 Apr 2020 22:21:37 -0500 Subject: [PATCH 05/31] got the form working for editing saving purging of webhook deliveries --- routers/repo/setting.go | 2 +- templates/repo/settings/options.tmpl | 10 +++++----- web_src/js/index.js | 11 +++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 1bd745f2fe8cd..81cebdb34edf8 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -326,7 +326,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { repo.IsHookTaskPurgeEnabled = form.IsHookTaskPurgeEnabled } - if repo.NumberWebhookDeliveriesToKeep != form.NumberWebhookDeliveriesToKeep { + if form.IsHookTaskPurgeEnabled && repo.NumberWebhookDeliveriesToKeep != form.NumberWebhookDeliveriesToKeep { repo.NumberWebhookDeliveriesToKeep = form.NumberWebhookDeliveriesToKeep } diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 3565e99e0fefe..5a8d40a8824fc 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -347,13 +347,13 @@
- - + +
-
- - +
+ +
diff --git a/web_src/js/index.js b/web_src/js/index.js index 63a5134bbd5bf..a8c41d928059f 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -756,6 +756,17 @@ async function initRepository() { if (typeof $(this).data('context') !== 'undefined') $($(this).data('context')).addClass('disabled'); } }); + + // Enable number of webhooks to keep. + $('.enable-webhooks-to-keep').change(function () { + if (this.checked) { + $($(this).data('target')).removeClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); + } else { + $($(this).data('target')).addClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); + } + }); } // Labels From dd5a870a2cc310fc2a5369f3324536a27e281574 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Thu, 7 May 2020 22:05:36 -0500 Subject: [PATCH 06/31] got the deletes running for prune hook_task --- models/webhook.go | 56 +++++++++++++++++++++++++++++++++++++++++++- modules/cron/cron.go | 5 +--- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/models/webhook.go b/models/webhook.go index 6012d0a803c30..fa0f3b42f818c 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -8,6 +8,7 @@ package models import ( "encoding/json" "fmt" + "math" "time" "code.gitea.io/gitea/modules/log" @@ -16,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" gouuid "github.com/satori/go.uuid" + "xorm.io/builder" ) // HookContentType is the content type of a web hook @@ -799,8 +801,60 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { return tasks, nil } -// DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered basedon the paramter +// DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the paramter func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { log.Info("delete delivered hook tasks on repo %d with num to keep %d", repoID, numberDeliveriesToKeep) + maxDeletesPerBatch := 2 + count, err := x. + Where("repo_id=? AND is_delivered=? ", repoID, 1). + Count(new(HookTask)) + + if err != nil { + return err + } + + if count > numberDeliveriesToKeep { + var numberToDelete = count - numberDeliveriesToKeep + log.Info("From repo %d will delete %d from hook_task", repoID, numberToDelete) + + var numberDeleted int64 = 0 + for numberDeleted < numberToDelete { + deletesThisBatch := maxDeletesPerBatch + numberLeftToDelete := numberToDelete - numberDeleted + if numberLeftToDelete < int64(maxDeletesPerBatch) { + if numberLeftToDelete < math.MaxInt32 { + deletesThisBatch = int(numberLeftToDelete) + } else { + deletesThisBatch = math.MaxInt32 + } + } + log.Info("From repo %d deleting %d from hook_task this iteration", repoID, deletesThisBatch) + var cond = builder.NewCond() + cond = cond.And(builder.Eq{"repo_id": repoID}) + cond = cond.And(builder.Eq{"is_delivered": 1}) + + var ids = make([]int64, 0, 10) + err := x.Table("hook_task"). + Where("repo_id = ? and is_delivered = ?", repoID, 1). + Cols("id"). + OrderBy("delivered asc"). + Limit(deletesThisBatch). + Find(&ids) + if err != nil { + return err + } + + //affected, err := x.Limit(deletesThisBatch).In("`id`", builder.Select("id").From("hook_task").Where(cond).OrderBy("delivered asc")). + // Delete(new(HookTask)) + affected, err := x.In("`id`", ids).Delete(new(HookTask)) + if err != nil { + return err + } + + numberDeleted += affected + log.Info("From repo %d deleted %d from hook_task this iteration, so far have deleted %d", repoID, affected, numberDeleted) + } + log.Info("From repo %d deleted in total %d from hook_task", repoID, numberDeleted) + } return nil } diff --git a/modules/cron/cron.go b/modules/cron/cron.go index 2d7f201d3255a..7c962323f3479 100644 --- a/modules/cron/cron.go +++ b/modules/cron/cron.go @@ -134,7 +134,6 @@ func NewContext() { } } if setting.Cron.PruneHookTaskTable.Enabled { - log.Error("prune hook task enabled") entry, err = c.AddFunc("Prune hook_task table", setting.Cron.PruneHookTaskTable.Schedule, WithUnique(pruneHookTaskTable, func(ctx context.Context) { if err := repo_module.PruneHookTaskTable(ctx); err != nil { log.Error("PruneHookTaskTable: %s", err) @@ -143,7 +142,7 @@ func NewContext() { if err != nil { log.Fatal("Cron[Repository prune hook_task table]: %v", err) } - if setting.Cron.RepoHealthCheck.RunAtStart { + if setting.Cron.PruneHookTaskTable.RunAtStart { entry.Prev = time.Now() entry.ExecTimes++ go WithUnique(pruneHookTaskTable, func(ctx context.Context) { @@ -152,8 +151,6 @@ func NewContext() { } })() } - } else { - log.Error("prune hook task not enabled :(") } entry, err = c.AddFunc("Update migrated repositories' issues and comments' posterid", setting.Cron.UpdateMigrationPosterID.Schedule, WithUnique(updateMigrationPosterID, migrations.UpdateMigrationPosterID)) From 1e800f5849f1e7e46b1cf7e3f6119fd9353979b6 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sun, 10 May 2020 22:11:16 -0500 Subject: [PATCH 07/31] added a couple unit tests; cleaned up delete routine a little --- models/webhook.go | 37 +++++++++++-------------------------- models/webhook_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/models/webhook.go b/models/webhook.go index fa0f3b42f818c..e2e3f8c2cb350 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -8,7 +8,6 @@ package models import ( "encoding/json" "fmt" - "math" "time" "code.gitea.io/gitea/modules/log" @@ -17,7 +16,6 @@ import ( "code.gitea.io/gitea/modules/timeutil" gouuid "github.com/satori/go.uuid" - "xorm.io/builder" ) // HookContentType is the content type of a web hook @@ -803,35 +801,25 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { // DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the paramter func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { - log.Info("delete delivered hook tasks on repo %d with num to keep %d", repoID, numberDeliveriesToKeep) - maxDeletesPerBatch := 2 - count, err := x. + maxDeletesPerBatch := 1000 + totalDeliveries, err := x. Where("repo_id=? AND is_delivered=? ", repoID, 1). Count(new(HookTask)) - if err != nil { return err } - if count > numberDeliveriesToKeep { - var numberToDelete = count - numberDeliveriesToKeep - log.Info("From repo %d will delete %d from hook_task", repoID, numberToDelete) + if totalDeliveries > numberDeliveriesToKeep { + var numberToDelete = totalDeliveries - numberDeliveriesToKeep + log.Trace("From repo %d will delete %d from hook_task", repoID, numberToDelete) var numberDeleted int64 = 0 for numberDeleted < numberToDelete { + deletesThisBatch := maxDeletesPerBatch - numberLeftToDelete := numberToDelete - numberDeleted - if numberLeftToDelete < int64(maxDeletesPerBatch) { - if numberLeftToDelete < math.MaxInt32 { - deletesThisBatch = int(numberLeftToDelete) - } else { - deletesThisBatch = math.MaxInt32 - } + if (numberToDelete - numberDeleted) < int64(maxDeletesPerBatch) { + deletesThisBatch = int(numberToDelete - numberDeleted) } - log.Info("From repo %d deleting %d from hook_task this iteration", repoID, deletesThisBatch) - var cond = builder.NewCond() - cond = cond.And(builder.Eq{"repo_id": repoID}) - cond = cond.And(builder.Eq{"is_delivered": 1}) var ids = make([]int64, 0, 10) err := x.Table("hook_task"). @@ -844,17 +832,14 @@ func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error return err } - //affected, err := x.Limit(deletesThisBatch).In("`id`", builder.Select("id").From("hook_task").Where(cond).OrderBy("delivered asc")). - // Delete(new(HookTask)) - affected, err := x.In("`id`", ids).Delete(new(HookTask)) + deletes, err := x.In("`id`", ids).Delete(new(HookTask)) if err != nil { return err } - numberDeleted += affected - log.Info("From repo %d deleted %d from hook_task this iteration, so far have deleted %d", repoID, affected, numberDeleted) + numberDeleted += deletes } - log.Info("From repo %d deleted in total %d from hook_task", repoID, numberDeleted) + log.Trace("From repo %d deleted in total %d from hook_task", repoID, numberDeleted) } return nil } diff --git a/models/webhook_test.go b/models/webhook_test.go index 5ee7f9159bd50..1fd4e5df78966 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -245,3 +245,33 @@ func TestUpdateHookTask(t *testing.T) { assert.NoError(t, UpdateHookTask(hook)) AssertExistsAndLoadBean(t, hook) } + +func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + hookTask := &HookTask{ + RepoID: 3, + HookID: 10, + IsDelivered: 1, + } + AssertNotExistsBean(t, hookTask) + assert.NoError(t, CreateHookTask(hookTask)) + AssertExistsAndLoadBean(t, hookTask) + + assert.NoError(t, DeleteDeliveredHookTasks(3, 0)) + AssertNotExistsBean(t, hookTask) +} + +func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + hookTask := &HookTask{ + RepoID: 3, + HookID: 15, + IsDelivered: 0, + } + AssertNotExistsBean(t, hookTask) + assert.NoError(t, CreateHookTask(hookTask)) + AssertExistsAndLoadBean(t, hookTask) + + assert.NoError(t, DeleteDeliveredHookTasks(3, 0)) + AssertExistsAndLoadBean(t, hookTask) +} From c016b7c24326eb4e538e6df6ebd2da4e2b9a63b8 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sun, 10 May 2020 22:22:13 -0500 Subject: [PATCH 08/31] added ENABLED setting in the ini --- custom/conf/app.ini.sample | 6 +++++- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index db19ad9e1a744..a6ceec08edb86 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -875,7 +875,11 @@ SCHEDULE = @every 24h ; Prune hook_task table [cron.prune_hook_task_table] -RUN_AT_START = true +; Whether to enable the job +ENABLED = true +; Whether to always run at start up time (if ENABLED) +RUN_AT_START = false +; Time interval for job to run SCHEDULE = @every 24h [git] 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 5cddd1e756036..faf0cef7074e3 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -560,7 +560,8 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` ### Cron - Prune hook_task Table (`cron.prune_hook_task_table`) -- `RUN_AT_START`: **true**: Run prune hook_task at start time. +- `ENABLED`: **true**: Enable service. +- `RUN_AT_START`: **false**: Run prune hook_task at start time (if ENABLED). - `SCHEDULE`: **@every 24h**: Cron syntax for pruning hook_task table. ## Git (`git`) From eea3a747c95daa246241c4df9163b7f5d08d0571 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sun, 10 May 2020 22:23:04 -0500 Subject: [PATCH 09/31] defaulting run at start to false on prune hook_task --- modules/setting/cron.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/setting/cron.go b/modules/setting/cron.go index d26ff985f6d13..cc6b8b8f72878 100644 --- a/modules/setting/cron.go +++ b/modules/setting/cron.go @@ -133,7 +133,7 @@ var ( Schedule string }{ Enabled: true, - RunAtStart: true, + RunAtStart: false, Schedule: "@every 24h", }, } From 66c0d8bca93e453de05719dfddadab7f1a48cb21 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Thu, 14 May 2020 22:36:06 -0500 Subject: [PATCH 10/31] added a condition to check is_hook_task_purge_enabled before deleting --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index e2e3f8c2cb350..00a51d76f4486 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -803,7 +803,7 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { maxDeletesPerBatch := 1000 totalDeliveries, err := x. - Where("repo_id=? AND is_delivered=? ", repoID, 1). + Where("repo_id=? AND is_delivered=? AND is_hook_task_purge_enabled=?", repoID, 1, 1). Count(new(HookTask)) if err != nil { return err From fde69372b9dfb130b6d86ee6ac2bdfb8d57b383d Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Thu, 14 May 2020 22:37:20 -0500 Subject: [PATCH 11/31] removing prior mistake --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index 00a51d76f4486..9b310f1172f8a 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -803,7 +803,7 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { maxDeletesPerBatch := 1000 totalDeliveries, err := x. - Where("repo_id=? AND is_delivered=? AND is_hook_task_purge_enabled=?", repoID, 1, 1). + Where("repo_id=? AND is_delivered=?", repoID, 1). Count(new(HookTask)) if err != nil { return err From dd0f4a1bee04fac83364300fface8efd697a55f0 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Fri, 15 May 2020 22:04:25 -0500 Subject: [PATCH 12/31] fixed error in webhook tests --- models/webhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/webhook_test.go b/models/webhook_test.go index 1fd4e5df78966..782a65c089701 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -251,7 +251,7 @@ func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { hookTask := &HookTask{ RepoID: 3, HookID: 10, - IsDelivered: 1, + IsDelivered: true, } AssertNotExistsBean(t, hookTask) assert.NoError(t, CreateHookTask(hookTask)) @@ -266,7 +266,7 @@ func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { hookTask := &HookTask{ RepoID: 3, HookID: 15, - IsDelivered: 0, + IsDelivered: false, } AssertNotExistsBean(t, hookTask) assert.NoError(t, CreateHookTask(hookTask)) From bc14884b0c5396a9aa7da904605ddd7a23f51721 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Fri, 15 May 2020 22:17:24 -0500 Subject: [PATCH 13/31] changed query in DeleteDeliveredHookTasks to respect the is_hook_task_purge_enabled flag --- models/webhook.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index 9b310f1172f8a..e452998f7e7b6 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -803,7 +803,9 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { maxDeletesPerBatch := 1000 totalDeliveries, err := x. - Where("repo_id=? AND is_delivered=?", repoID, 1). + Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, 1). + Join("INNER", "repository", "hook_task.repo_id = repository.id"). + And("repository.is_hook_task_purge_enabled = ?", 1). Count(new(HookTask)) if err != nil { return err From 125dc9955004276885f51b0027ea4fb9b027b17c Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Fri, 15 May 2020 22:37:09 -0500 Subject: [PATCH 14/31] fixed misspelling in comment --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index e452998f7e7b6..645db8c31688f 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -799,7 +799,7 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { return tasks, nil } -// DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the paramter +// DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the parameter func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { maxDeletesPerBatch := 1000 totalDeliveries, err := x. From d1db4f2720fc53f5bf5697016551e7b3b44bc248 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 16 May 2020 08:47:02 -0500 Subject: [PATCH 15/31] formatting --- models/migrations/v140.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v140.go b/models/migrations/v140.go index a45bc07f769dd..885822e52da64 100644 --- a/models/migrations/v140.go +++ b/models/migrations/v140.go @@ -25,4 +25,4 @@ func addHookTaskPurge(x *xorm.Engine) error { _, err := x.Exec("UPDATE repository SET is_hook_task_purge_enabled = ?, number_webhook_deliveries_to_keep = ?", setting.Repository.DefaultIsHookTaskPurgeEnabled, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) return err -} \ No newline at end of file +} From fd138125577477285a3c3182019a560b2aedfb51 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 16 May 2020 08:54:10 -0500 Subject: [PATCH 16/31] changed var declaration --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index 645db8c31688f..240c71d843c4b 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -815,7 +815,7 @@ func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error var numberToDelete = totalDeliveries - numberDeliveriesToKeep log.Trace("From repo %d will delete %d from hook_task", repoID, numberToDelete) - var numberDeleted int64 = 0 + var numberDeleted int64 for numberDeleted < numberToDelete { deletesThisBatch := maxDeletesPerBatch From a28df99e500c5c20b3a58971614c5481637244b8 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 16 May 2020 10:27:14 -0500 Subject: [PATCH 17/31] fix unit tests --- models/webhook_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/webhook_test.go b/models/webhook_test.go index 782a65c089701..106e752065af0 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -250,7 +250,7 @@ func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) hookTask := &HookTask{ RepoID: 3, - HookID: 10, + HookID: 3, IsDelivered: true, } AssertNotExistsBean(t, hookTask) @@ -264,8 +264,8 @@ func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 3, - HookID: 15, + RepoID: 2, + HookID: 4, IsDelivered: false, } AssertNotExistsBean(t, hookTask) From 465103026af7f412ae1fca86d5fc4d52287da662 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 16 May 2020 14:10:57 -0500 Subject: [PATCH 18/31] unit tests --- models/webhook_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/webhook_test.go b/models/webhook_test.go index 106e752065af0..f9c537c579010 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -251,6 +251,9 @@ func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { hookTask := &HookTask{ RepoID: 3, HookID: 3, + Type: GITEA, + URL: "http://www.example.com/unit_test", + Payloader: &api.PushPayload{}, IsDelivered: true, } AssertNotExistsBean(t, hookTask) @@ -266,6 +269,9 @@ func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { hookTask := &HookTask{ RepoID: 2, HookID: 4, + Type: GITEA, + URL: "http://www.example.com/unit_test", + Payloader: &api.PushPayload{}, IsDelivered: false, } AssertNotExistsBean(t, hookTask) From 38f6d30d501f7b0a8b89281814c843a440b169f7 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 16 May 2020 22:51:57 -0500 Subject: [PATCH 19/31] adjusted to cron refactor; added missing string for cron page --- modules/cron/tasks_basic.go | 11 +++++++++++ options/locale/locale_en-US.ini | 1 + 2 files changed, 12 insertions(+) diff --git a/modules/cron/tasks_basic.go b/modules/cron/tasks_basic.go index 438c4a5004fc6..18a1524a83625 100644 --- a/modules/cron/tasks_basic.go +++ b/modules/cron/tasks_basic.go @@ -107,6 +107,16 @@ func registerUpdateMigrationPosterID() { }) } +func registerPrunHookTaskTable() { + RegisterTaskFatal("prune_hook_task_table", &BaseConfig{ + Enabled: true, + RunAtStart: false, + Schedule: "@every 24h", + }, func(ctx context.Context, _ *models.User, _ Config) error { + return repository_service.PruneHookTaskTable(ctx) + }) +} + func initBasicTasks() { registerUpdateMirrorTask() registerRepoHealthCheck() @@ -115,4 +125,5 @@ func initBasicTasks() { registerSyncExternalUsers() registerDeletedBranchesCleanup() registerUpdateMigrationPosterID() + registerPrunHookTaskTable() } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4b9fd1c7cef84..99ef256ccafc9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1875,6 +1875,7 @@ dashboard.resync_all_hooks = Resynchronize pre-receive, update and post-receive dashboard.reinit_missing_repos = Reinitialize all missing Git repositories for which records exist dashboard.sync_external_users = Synchronize external user data dashboard.git_fsck = Execute health checks on all repositories +dashboard.prune_hook_task_table = Prune hook_task table. dashboard.server_uptime = Server Uptime dashboard.current_goroutine = Current Goroutines dashboard.current_memory_usage = Current Memory Usage From 537c913ff39ed385cac5e46425e369065e2ab527 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Tue, 19 May 2020 22:57:41 -0500 Subject: [PATCH 20/31] simplified hook_task prune --- models/webhook.go | 47 ++++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/models/webhook.go b/models/webhook.go index 240c71d843c4b..bdfc12efd88f3 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -801,47 +801,28 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { // DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the parameter func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { - maxDeletesPerBatch := 1000 - totalDeliveries, err := x. + var ids = make([]int64, 0, 10) + err := x.Table("hook_task"). Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, 1). + Cols("hook_task.id"). Join("INNER", "repository", "hook_task.repo_id = repository.id"). And("repository.is_hook_task_purge_enabled = ?", 1). - Count(new(HookTask)) + OrderBy("hook_task.id desc"). + Limit(1, int(numberDeliveriesToKeep)). + Find(&ids) if err != nil { return err } - if totalDeliveries > numberDeliveriesToKeep { - var numberToDelete = totalDeliveries - numberDeliveriesToKeep - log.Trace("From repo %d will delete %d from hook_task", repoID, numberToDelete) - - var numberDeleted int64 - for numberDeleted < numberToDelete { - - deletesThisBatch := maxDeletesPerBatch - if (numberToDelete - numberDeleted) < int64(maxDeletesPerBatch) { - deletesThisBatch = int(numberToDelete - numberDeleted) - } - - var ids = make([]int64, 0, 10) - err := x.Table("hook_task"). - Where("repo_id = ? and is_delivered = ?", repoID, 1). - Cols("id"). - OrderBy("delivered asc"). - Limit(deletesThisBatch). - Find(&ids) - if err != nil { - return err - } - - deletes, err := x.In("`id`", ids).Delete(new(HookTask)) - if err != nil { - return err - } - - numberDeleted += deletes + if len(ids) > 0 { + deletes, err := x. + Where("repo_id = ? and is_delivered = ? and id <= ?", repoID, 1, ids[0]). + Delete(new(HookTask)) + if err != nil { + return err } - log.Trace("From repo %d deleted in total %d from hook_task", repoID, numberDeleted) + log.Trace("From repo %d deleted in total %d from hook_task", repoID, deletes) } + return nil } From b6c09c5dbb9e64467e60abc9b392bc031a24dfd9 Mon Sep 17 00:00:00 2001 From: Brad Albright <32200834+bhalbright@users.noreply.github.com> Date: Wed, 20 May 2020 21:50:06 -0500 Subject: [PATCH 21/31] Update models/webhook.go Co-authored-by: Lauris BH --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index bdfc12efd88f3..dcefea8bd3cf8 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -803,7 +803,7 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { var ids = make([]int64, 0, 10) err := x.Table("hook_task"). - Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, 1). + Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, true). Cols("hook_task.id"). Join("INNER", "repository", "hook_task.repo_id = repository.id"). And("repository.is_hook_task_purge_enabled = ?", 1). From ce8d93ad55ae82cebe1586b563249e92cb01ad54 Mon Sep 17 00:00:00 2001 From: Brad Albright <32200834+bhalbright@users.noreply.github.com> Date: Wed, 20 May 2020 21:50:17 -0500 Subject: [PATCH 22/31] Update models/webhook.go Co-authored-by: Lauris BH --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index dcefea8bd3cf8..7812614aa11b9 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -816,7 +816,7 @@ func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error if len(ids) > 0 { deletes, err := x. - Where("repo_id = ? and is_delivered = ? and id <= ?", repoID, 1, ids[0]). + Where("repo_id = ? and is_delivered = ? and id <= ?", repoID, true, ids[0]). Delete(new(HookTask)) if err != nil { return err From 3c4515981aff621f765d1b152b3b07c628bb976c Mon Sep 17 00:00:00 2001 From: Brad Albright <32200834+bhalbright@users.noreply.github.com> Date: Wed, 20 May 2020 21:50:29 -0500 Subject: [PATCH 23/31] Update models/webhook.go Co-authored-by: Lauris BH --- models/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index 7812614aa11b9..0079c09faabc1 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -806,7 +806,7 @@ func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, true). Cols("hook_task.id"). Join("INNER", "repository", "hook_task.repo_id = repository.id"). - And("repository.is_hook_task_purge_enabled = ?", 1). + And("repository.is_hook_task_purge_enabled = ?", true). OrderBy("hook_task.id desc"). Limit(1, int(numberDeliveriesToKeep)). Find(&ids) From 9b2b54648fa45fc4bfb2e2ba47eb076a56e61c99 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sun, 2 Aug 2020 15:10:49 -0500 Subject: [PATCH 24/31] updated hook task delete to find the most recent delivery to delete and delete that delivery and those older than it --- models/webhook.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/models/webhook.go b/models/webhook.go index d3bd86a1a52b3..667409f0e0801 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -801,22 +801,22 @@ func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { // DeleteDeliveredHookTasks deletes delivered hook tasks of one repository, leaving the most recent delivered based on the parameter func DeleteDeliveredHookTasks(repoID int64, numberDeliveriesToKeep int64) error { - var ids = make([]int64, 0, 10) + var deliveryDates = make([]int64, 0, 10) err := x.Table("hook_task"). Where("hook_task.repo_id = ? AND hook_task.is_delivered = ?", repoID, true). - Cols("hook_task.id"). + Cols("hook_task.delivered"). Join("INNER", "repository", "hook_task.repo_id = repository.id"). And("repository.is_hook_task_purge_enabled = ?", true). - OrderBy("hook_task.id desc"). + OrderBy("hook_task.delivered desc"). Limit(1, int(numberDeliveriesToKeep)). - Find(&ids) + Find(&deliveryDates) if err != nil { return err } - if len(ids) > 0 { + if len(deliveryDates) > 0 { deletes, err := x. - Where("repo_id = ? and is_delivered = ? and id <= ?", repoID, true, ids[0]). + Where("repo_id = ? and is_delivered = ? and delivered <= ?", repoID, true, deliveryDates[0]). Delete(new(HookTask)) if err != nil { return err From 83925c2694b5958abd4e94e64d99baf0a1c027d0 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Tue, 4 Aug 2020 22:17:50 -0500 Subject: [PATCH 25/31] added blank line --- modules/repository/prune_hook_task.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/repository/prune_hook_task.go b/modules/repository/prune_hook_task.go index d02036f0a1194..d62897d2442b9 100644 --- a/modules/repository/prune_hook_task.go +++ b/modules/repository/prune_hook_task.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" + "xorm.io/builder" ) From 5decea3cf9a28ae0a31edf0eccbb71bc72fc1d18 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Tue, 4 Aug 2020 22:30:11 -0500 Subject: [PATCH 26/31] gofmt migration files --- models/migrations/v145.go | 2 +- models/migrations/v146.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/v145.go b/models/migrations/v145.go index 554bb331ea186..436c95fab9cc0 100644 --- a/models/migrations/v145.go +++ b/models/migrations/v145.go @@ -57,4 +57,4 @@ func increaseLanguageField(x *xorm.Engine) error { } return sess.Commit() -} \ No newline at end of file +} diff --git a/models/migrations/v146.go b/models/migrations/v146.go index 218e194cc3f78..a0c09efcaa84c 100644 --- a/models/migrations/v146.go +++ b/models/migrations/v146.go @@ -24,4 +24,4 @@ func addHookTaskPurge(x *xorm.Engine) error { _, err := x.Exec("UPDATE repository SET is_hook_task_purge_enabled = ?, number_webhook_deliveries_to_keep = ?", setting.Repository.DefaultIsHookTaskPurgeEnabled, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) return err -} \ No newline at end of file +} From 9a09960d3964c93e243de7c76b54b8154349aa84 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Tue, 4 Aug 2020 22:34:26 -0500 Subject: [PATCH 27/31] gofmt file --- modules/repository/prune_hook_task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/repository/prune_hook_task.go b/modules/repository/prune_hook_task.go index d62897d2442b9..e2d18e5fcf581 100644 --- a/modules/repository/prune_hook_task.go +++ b/modules/repository/prune_hook_task.go @@ -10,7 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" - + "xorm.io/builder" ) From 49ea492744f41b3ace98894b2a035f2c20b83723 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Wed, 5 Aug 2020 22:11:16 -0500 Subject: [PATCH 28/31] added additional information about the prune hook_task process --- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f3010c19a62ff..025a82d7656f0 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -570,7 +570,7 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` ### Cron - Prune hook_task Table (`cron.prune_hook_task_table`) -- `ENABLED`: **true**: Enable service. +- `ENABLED`: **true**: Enable service. In the repository table, there are two columns to additionally override the behavior per repository: is_hook_task_purge_enabled - enable or disable purging of hook_task data by repository and number_webhook_deliveries_to_keep - the process will leave the most recently delivered webhooks, this value controls how many are kept. These settings can also be modified per repository in the Gitea UI. By default, the process is enabled for every repository and 10 webhooks are kept. - `RUN_AT_START`: **false**: Run prune hook_task at start time (if ENABLED). - `SCHEDULE`: **@every 24h**: Cron syntax for pruning hook_task table. From 1709bc9711b21de3cf5fdefe986ac42b34d8b258 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 2 Sep 2020 20:30:50 +0100 Subject: [PATCH 29/31] move migration out of the way Signed-off-by: Andrew Thornton --- models/migrations/{v146.go => v148.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/{v146.go => v148.go} (100%) diff --git a/models/migrations/v146.go b/models/migrations/v148.go similarity index 100% rename from models/migrations/v146.go rename to models/migrations/v148.go From dacb03bfdc826f465215e88b050ab0b46b10e6f5 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Wed, 9 Sep 2020 22:10:09 -0500 Subject: [PATCH 30/31] changed a couple log statmentes to trace, should not have been error --- modules/repository/prune_hook_task.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/repository/prune_hook_task.go b/modules/repository/prune_hook_task.go index e2d18e5fcf581..db511db9bb5cd 100644 --- a/modules/repository/prune_hook_task.go +++ b/modules/repository/prune_hook_task.go @@ -16,7 +16,7 @@ import ( // PruneHookTaskTable deletes rows from hook_task as needed. func PruneHookTaskTable(ctx context.Context) error { - log.Error("Doing: PruneHookTaskTable") + log.Trace("Doing: PruneHookTaskTable") if err := models.Iterate( models.DefaultDBContext(), @@ -44,6 +44,6 @@ func PruneHookTaskTable(ctx context.Context) error { return err } - log.Error("Finished: PruneHookTaskTable") + log.Trace("Finished: PruneHookTaskTable") return nil } From 63551dd74fc4f3ac21ec3647fe3deeca25802ee5 Mon Sep 17 00:00:00 2001 From: Brad Albright Date: Sat, 12 Sep 2020 15:02:59 -0500 Subject: [PATCH 31/31] created unit test to ensure that deleting delivered hook_task records will leave the most recent based if 1 delivery is to be kept --- models/webhook_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/models/webhook_test.go b/models/webhook_test.go index f9c537c579010..6f4066ba1453e 100644 --- a/models/webhook_test.go +++ b/models/webhook_test.go @@ -7,6 +7,7 @@ package models import ( "encoding/json" "testing" + "time" api "code.gitea.io/gitea/modules/structs" @@ -281,3 +282,22 @@ func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { assert.NoError(t, DeleteDeliveredHookTasks(3, 0)) AssertExistsAndLoadBean(t, hookTask) } + +func TestDeleteDeliveredHookTasks_LeavesMostRecentTask(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + hookTask := &HookTask{ + RepoID: 2, + HookID: 4, + Type: GITEA, + URL: "http://www.example.com/unit_test", + Payloader: &api.PushPayload{}, + IsDelivered: true, + Delivered: time.Now().UnixNano(), + } + AssertNotExistsBean(t, hookTask) + assert.NoError(t, CreateHookTask(hookTask)) + AssertExistsAndLoadBean(t, hookTask) + + assert.NoError(t, DeleteDeliveredHookTasks(3, 1)) + AssertExistsAndLoadBean(t, hookTask) +}