From c297099bd7f2a47f1bc50026c8b667244497aee8 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 18 Oct 2016 18:52:22 +0200 Subject: [PATCH 1/7] Cleanup projects which failed to delete --- app/workers/cleanup_pending_delete_worker.rb | 13 +++++++++++++ app/workers/project_destroy_worker.rb | 2 +- config/initializers/1_settings.rb | 4 ++++ .../workers/cleanup_pending_delete_worker_spec.rb | 15 +++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 app/workers/cleanup_pending_delete_worker.rb create mode 100644 spec/workers/cleanup_pending_delete_worker_spec.rb diff --git a/app/workers/cleanup_pending_delete_worker.rb b/app/workers/cleanup_pending_delete_worker.rb new file mode 100644 index 000000000000..cb7e09ca2918 --- /dev/null +++ b/app/workers/cleanup_pending_delete_worker.rb @@ -0,0 +1,13 @@ +class CleanupPendingDeleteWorker + include Sidekiq::Worker + + sidekiq_options retry: false # this job auto-repeats via sidekiq-cron + + def perform + admin = User.find_by(admin: true) + + Project.unscoped.where(pending_delete: true).find_each do |project| + ProjectDestroyWorker.perform_async(project.id, admin.id) + end + end +end diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb index b462327490ef..8d3ffe2f7e02 100644 --- a/app/workers/project_destroy_worker.rb +++ b/app/workers/project_destroy_worker.rb @@ -2,7 +2,7 @@ class ProjectDestroyWorker include Sidekiq::Worker include DedicatedSidekiqQueue - def perform(project_id, user_id, params) + def perform(project_id, user_id, params = {}) begin project = Project.unscoped.find(project_id) rescue ActiveRecord::RecordNotFound diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 9ddd15548111..af2d041098e1 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -312,6 +312,10 @@ def host(url) Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['cron'] ||= '20 0 * * *' Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['job_class'] = 'RemoveUnreferencedLfsObjectsWorker' +Settings.cron_jobs['cleanup_pending_delete_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['cleanup_pending_delete_worker']['cron'] = '25 */4 * * *' +Settings.cron_jobs['cleanup_pending_delete_worker']['job_class'] = 'CleanupPendingDeleteWorker' + # # GitLab Shell # diff --git a/spec/workers/cleanup_pending_delete_worker_spec.rb b/spec/workers/cleanup_pending_delete_worker_spec.rb new file mode 100644 index 000000000000..be7da9757e5d --- /dev/null +++ b/spec/workers/cleanup_pending_delete_worker_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe CleanupPendingDeleteWorker do + let(:project) { create(:empty_project, pending_delete: true) } + let(:admin) { create(:admin) } + + subject { CleanupPendingDeleteWorker.new } + + describe "#perform" do + it "queues projects for deletion" do + expect(ProjectDestroyWorker).to receive(:perform_async).with(project.id, admin.id) + subject.perform + end + end +end -- GitLab From 1bb70e127a7237d30863dc5c3b7491f7639c412f Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 25 Oct 2016 21:11:34 -0700 Subject: [PATCH 2/7] Delete Groups which failed to be deleted Also, this commit limits the amount of projects and groups we queue per call of the worker. At this time we have over 18k projects marked as pending_delete, so it wouldn't be wise to queue all these as removing a project for example is quite compute intensive for the disk and for PG. --- CHANGELOG.md | 31 +++++++++++++++++++ app/workers/cleanup_pending_delete_worker.rb | 9 +++++- .../cleanup_pending_delete_worker_spec.rb | 15 +++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f41cbc9228b..5589d8592567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,37 @@ entry. - Fix and improve `Sortable.highest_label_priority`. !7165 - Fixed sticky merge request tabs when sidebar is pinned. !7167 - Only remove right connector of first build of last stage. !7179 + - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814 + - Cronjob to delete projects and groups which failed previously !7067 + - Adds user project membership expired event to clarify why user was removed (Callum Dryden) + - Trim leading and trailing whitespace on project_path (Linus Thiel) + - Prevent award emoji via notes for issues/MRs authored by user (barthc) + - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) + - Fix extra space on Build sidebar on Firefox !7060 + - Fix mobile layout issues in admin user overview page !7087 + - Fix HipChat notifications rendering (airatshigapov, eisnerd) + - Add hover to trash icon in notes !7008 (blackst0ne) + - Fix sidekiq stats in admin area (blackst0ne) + - Removed delete branch tooltip !6954 + - Escape ref and path for relative links !6050 (winniehell) + - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose) + - Fix filtering of milestones with quotes in title (airatshigapov) + - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison) + - Update mail_room and enable sentinel support to Reply By Email (!7101) + - Simpler arguments passed to named_route on toggle_award_url helper method + - Fix typo in framework css class. !7086 (Daniel Voogsgerd) + - New issue board list dropdown stays open after adding a new list + - Fix: Backup restore doesn't clear cache + - API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh) + - Replace jquery.cookie plugin with js.cookie !7085 + - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method + - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens + - Show full status link on MR & commit pipelines + - Fix documents and comments on Build API `scope` + - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) + +## 8.13.2 + - Fix builds dropdown overlapping bug !7124 ## 8.13.1 (2016-10-25) diff --git a/app/workers/cleanup_pending_delete_worker.rb b/app/workers/cleanup_pending_delete_worker.rb index cb7e09ca2918..65d882923234 100644 --- a/app/workers/cleanup_pending_delete_worker.rb +++ b/app/workers/cleanup_pending_delete_worker.rb @@ -1,12 +1,19 @@ class CleanupPendingDeleteWorker include Sidekiq::Worker + include CronjobQueue + + MAX_NEW_JOBS = 256 sidekiq_options retry: false # this job auto-repeats via sidekiq-cron def perform admin = User.find_by(admin: true) - Project.unscoped.where(pending_delete: true).find_each do |project| + Group.with_deleted.where.not(deleted_at: nil).limit(MAX_NEW_JOBS).order(:deleted_at).each do |group| + GroupDestroyWorker.perform_async(group.id, admin.id) + end + + Project.unscoped.where(pending_delete: true).limit(MAX_NEW_JOBS).order(:updated_at).each do |project| ProjectDestroyWorker.perform_async(project.id, admin.id) end end diff --git a/spec/workers/cleanup_pending_delete_worker_spec.rb b/spec/workers/cleanup_pending_delete_worker_spec.rb index be7da9757e5d..7f0bc86b3b31 100644 --- a/spec/workers/cleanup_pending_delete_worker_spec.rb +++ b/spec/workers/cleanup_pending_delete_worker_spec.rb @@ -1,14 +1,25 @@ require 'spec_helper' describe CleanupPendingDeleteWorker do + let!(:admin) { create(:admin) } let(:project) { create(:empty_project, pending_delete: true) } - let(:admin) { create(:admin) } + let(:group) { create(:group) } - subject { CleanupPendingDeleteWorker.new } + subject { described_class.new } describe "#perform" do + it "queues groups for deletion" do + # We can't set deleted_at on 1.week.ago because of acts_as_paranoid + group.destroy + + expect(GroupDestroyWorker).to receive(:perform_async).with(group.id, admin.id) + + subject.perform + end + it "queues projects for deletion" do expect(ProjectDestroyWorker).to receive(:perform_async).with(project.id, admin.id) + subject.perform end end -- GitLab From c51834d2ff44eee7c6607640b0114177271fcf19 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 2 Nov 2016 21:05:35 +0100 Subject: [PATCH 3/7] Add changelog entry --- changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml diff --git a/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml b/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml new file mode 100644 index 000000000000..f3ccc09588cb --- /dev/null +++ b/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml @@ -0,0 +1,4 @@ +--- +title: Requeue projects pending deletion +merge_request: 7067 +author: -- GitLab From 7b8783b44c84536466b87c36e4abdc17c88f0725 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 7 Nov 2016 20:07:13 +0100 Subject: [PATCH 4/7] Revert "Cleanup projects which failed to delete" This reverts commit 614baa86ac6dbd82a2e89800052f177ef7c7f3c5. --- app/workers/cleanup_pending_delete_worker.rb | 20 -------------- app/workers/project_destroy_worker.rb | 2 +- config/initializers/1_settings.rb | 4 --- .../cleanup_pending_delete_worker_spec.rb | 26 ------------------- 4 files changed, 1 insertion(+), 51 deletions(-) delete mode 100644 app/workers/cleanup_pending_delete_worker.rb delete mode 100644 spec/workers/cleanup_pending_delete_worker_spec.rb diff --git a/app/workers/cleanup_pending_delete_worker.rb b/app/workers/cleanup_pending_delete_worker.rb deleted file mode 100644 index 65d882923234..000000000000 --- a/app/workers/cleanup_pending_delete_worker.rb +++ /dev/null @@ -1,20 +0,0 @@ -class CleanupPendingDeleteWorker - include Sidekiq::Worker - include CronjobQueue - - MAX_NEW_JOBS = 256 - - sidekiq_options retry: false # this job auto-repeats via sidekiq-cron - - def perform - admin = User.find_by(admin: true) - - Group.with_deleted.where.not(deleted_at: nil).limit(MAX_NEW_JOBS).order(:deleted_at).each do |group| - GroupDestroyWorker.perform_async(group.id, admin.id) - end - - Project.unscoped.where(pending_delete: true).limit(MAX_NEW_JOBS).order(:updated_at).each do |project| - ProjectDestroyWorker.perform_async(project.id, admin.id) - end - end -end diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb index 8d3ffe2f7e02..b462327490ef 100644 --- a/app/workers/project_destroy_worker.rb +++ b/app/workers/project_destroy_worker.rb @@ -2,7 +2,7 @@ class ProjectDestroyWorker include Sidekiq::Worker include DedicatedSidekiqQueue - def perform(project_id, user_id, params = {}) + def perform(project_id, user_id, params) begin project = Project.unscoped.find(project_id) rescue ActiveRecord::RecordNotFound diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index af2d041098e1..9ddd15548111 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -312,10 +312,6 @@ def host(url) Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['cron'] ||= '20 0 * * *' Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['job_class'] = 'RemoveUnreferencedLfsObjectsWorker' -Settings.cron_jobs['cleanup_pending_delete_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['cleanup_pending_delete_worker']['cron'] = '25 */4 * * *' -Settings.cron_jobs['cleanup_pending_delete_worker']['job_class'] = 'CleanupPendingDeleteWorker' - # # GitLab Shell # diff --git a/spec/workers/cleanup_pending_delete_worker_spec.rb b/spec/workers/cleanup_pending_delete_worker_spec.rb deleted file mode 100644 index 7f0bc86b3b31..000000000000 --- a/spec/workers/cleanup_pending_delete_worker_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'spec_helper' - -describe CleanupPendingDeleteWorker do - let!(:admin) { create(:admin) } - let(:project) { create(:empty_project, pending_delete: true) } - let(:group) { create(:group) } - - subject { described_class.new } - - describe "#perform" do - it "queues groups for deletion" do - # We can't set deleted_at on 1.week.ago because of acts_as_paranoid - group.destroy - - expect(GroupDestroyWorker).to receive(:perform_async).with(group.id, admin.id) - - subject.perform - end - - it "queues projects for deletion" do - expect(ProjectDestroyWorker).to receive(:perform_async).with(project.id, admin.id) - - subject.perform - end - end -end -- GitLab From 1266532fc3353c181bc77a6dc56e5d8aa3bff5dc Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 7 Nov 2016 20:08:35 +0100 Subject: [PATCH 5/7] Revert "Delete Groups which failed to be deleted" This reverts commit 5824a2b0e556631425ab709af2d79f58cd184395. --- CHANGELOG.md | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5589d8592567..9f41cbc9228b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,37 +134,6 @@ entry. - Fix and improve `Sortable.highest_label_priority`. !7165 - Fixed sticky merge request tabs when sidebar is pinned. !7167 - Only remove right connector of first build of last stage. !7179 - - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814 - - Cronjob to delete projects and groups which failed previously !7067 - - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - - Trim leading and trailing whitespace on project_path (Linus Thiel) - - Prevent award emoji via notes for issues/MRs authored by user (barthc) - - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) - - Fix extra space on Build sidebar on Firefox !7060 - - Fix mobile layout issues in admin user overview page !7087 - - Fix HipChat notifications rendering (airatshigapov, eisnerd) - - Add hover to trash icon in notes !7008 (blackst0ne) - - Fix sidekiq stats in admin area (blackst0ne) - - Removed delete branch tooltip !6954 - - Escape ref and path for relative links !6050 (winniehell) - - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose) - - Fix filtering of milestones with quotes in title (airatshigapov) - - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison) - - Update mail_room and enable sentinel support to Reply By Email (!7101) - - Simpler arguments passed to named_route on toggle_award_url helper method - - Fix typo in framework css class. !7086 (Daniel Voogsgerd) - - New issue board list dropdown stays open after adding a new list - - Fix: Backup restore doesn't clear cache - - API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh) - - Replace jquery.cookie plugin with js.cookie !7085 - - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method - - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens - - Show full status link on MR & commit pipelines - - Fix documents and comments on Build API `scope` - - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) - -## 8.13.2 - - Fix builds dropdown overlapping bug !7124 ## 8.13.1 (2016-10-25) -- GitLab From f5febf60ffcc53384ccf16fe5e0a67148cd2eb85 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 7 Nov 2016 20:09:05 +0100 Subject: [PATCH 6/7] Requeue all soft deleted namespaces and projects The first take on this problem involved a worker requeueing 256 of these per 4 hours. Than it was suggested that this might not be needed, and post deployment migrations could be used. Given we monitor the number of projects which are pending deletion, we could give this strategy a try and take a look about a few hours later to see how this fares. --- .../20161107200810_requeue_pending_delete.rb | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 db/post_migrate/20161107200810_requeue_pending_delete.rb diff --git a/db/post_migrate/20161107200810_requeue_pending_delete.rb b/db/post_migrate/20161107200810_requeue_pending_delete.rb new file mode 100644 index 000000000000..c959ed2f6f02 --- /dev/null +++ b/db/post_migrate/20161107200810_requeue_pending_delete.rb @@ -0,0 +1,32 @@ +class RequeuePendingDelete < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def up + admin = User.find_by(admin: true) + + Group.with_deleted.where.not(deleted_at: nil).find_each do |group| + GroupDestroyWorker.perform_async(group.id, admin.id) + end + + Project.unscoped.where(pending_delete: true).find_each do |project| + ProjectDestroyWorker.perform_async(project.id, admin.id) + end + end + + def down + # Noop + end +end -- GitLab From 2d246accff115740a2888706c919d2ffd392eeec Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 21 Nov 2016 13:05:29 +0100 Subject: [PATCH 7/7] WIP --- .../zj-cronjob-requeue-pending-delete.yml | 2 +- .../20161107200810_requeue_pending_delete.rb | 32 ------------- ...1121090906_delete_soft_deleted_entities.rb | 48 +++++++++++++++++++ db/schema.rb | 2 +- 4 files changed, 50 insertions(+), 34 deletions(-) delete mode 100644 db/post_migrate/20161107200810_requeue_pending_delete.rb create mode 100644 db/post_migrate/20161121090906_delete_soft_deleted_entities.rb diff --git a/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml b/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml index f3ccc09588cb..8dbb162b5882 100644 --- a/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml +++ b/changelogs/unreleased/zj-cronjob-requeue-pending-delete.yml @@ -1,4 +1,4 @@ --- -title: Requeue projects pending deletion +title: Delete soft deleted entities from the database merge_request: 7067 author: diff --git a/db/post_migrate/20161107200810_requeue_pending_delete.rb b/db/post_migrate/20161107200810_requeue_pending_delete.rb deleted file mode 100644 index c959ed2f6f02..000000000000 --- a/db/post_migrate/20161107200810_requeue_pending_delete.rb +++ /dev/null @@ -1,32 +0,0 @@ -class RequeuePendingDelete < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - - def up - admin = User.find_by(admin: true) - - Group.with_deleted.where.not(deleted_at: nil).find_each do |group| - GroupDestroyWorker.perform_async(group.id, admin.id) - end - - Project.unscoped.where(pending_delete: true).find_each do |project| - ProjectDestroyWorker.perform_async(project.id, admin.id) - end - end - - def down - # Noop - end -end diff --git a/db/post_migrate/20161121090906_delete_soft_deleted_entities.rb b/db/post_migrate/20161121090906_delete_soft_deleted_entities.rb new file mode 100644 index 000000000000..56705931a9a5 --- /dev/null +++ b/db/post_migrate/20161121090906_delete_soft_deleted_entities.rb @@ -0,0 +1,48 @@ +class DeleteSoftDeletedEntities < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + BATCH_SIZE = 128 + DOWNTIME = false + + disable_ddl_transaction! + + def up + [namespaces_sql, projects_sql].each do |sql| + loop do + deleted = connection.exec_query(sql) + + break if deleted.rows.count.zero? + + sleep(100) + end + end + end + + def down + # Noop + end + + def namespaces_sql + namespaces = Arel::Table.new(:namespaces) + Arel::DeleteManager.new(ActiveRecord::Base). + from(namespaces). + where(namespaces[:id].in( + namespaces.project(namespaces[:id]). + where(namespaces[:deleted_at].not_eq(nil)). + take(BATCH_SIZE)) + ). + to_sql + end + + def projects_sql + projects = Arel::Table.new(:projects) + Arel::DeleteManager.new(ActiveRecord::Base). + from(projects). + where(projects[:id].in( + projects.project(projects[:id]). + where(projects[:pending_delete].eq(true)). + take(BATCH_SIZE)) + ). + to_sql + end +end diff --git a/db/schema.rb b/db/schema.rb index 6b28e80f01d0..823941cea709 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161118183841) do +ActiveRecord::Schema.define(version: 20161121090906) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- GitLab