Remove soft removals related code

This removes all usage of soft removals except for the "pending delete"
system implemented for projects. This in turn simplifies all the query
plans of the models that used soft removals. Since we don't really use
soft removals for anything useful there's no point in keeping it around.

This _does_ mean that hard removals of issues (which only admins can do
if I'm not mistaken) can influence the "iid" values, but that code is
broken to begin with. More on this (and how to fix it) can be found in
https://gitlab.com/gitlab-org/gitlab-ce/issues/31114.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/37447
parent 33fb2f99
......@@ -381,9 +381,6 @@ gem 'ruby-prof', '~> 0.16.2'
# OAuth
gem 'oauth2', '~> 1.4'
# Soft deletion
gem 'paranoia', '~> 2.3.1'
# Health check
gem 'health_check', '~> 2.6.0'
......
......@@ -580,8 +580,6 @@ GEM
orm_adapter (0.5.0)
os (0.9.6)
parallel (1.12.0)
paranoia (2.3.1)
activerecord (>= 4.0, < 5.2)
parser (2.4.0.2)
ast (~> 2.3)
parslet (1.5.0)
......@@ -1110,7 +1108,6 @@ DEPENDENCIES
omniauth-twitter (~> 1.2.0)
omniauth_crowd (~> 2.2.0)
org-ruby (~> 0.9.12)
paranoia (~> 2.3.1)
peek (~> 1.0.1)
peek-gc (~> 0.0.2)
peek-host (~> 1.0.0)
......
......@@ -2,8 +2,9 @@ module Ci
class PipelineSchedule < ActiveRecord::Base
extend Gitlab::Ci::Model
include Importable
include IgnorableColumn
acts_as_paranoid
ignore_column :deleted_at
belongs_to :project
belongs_to :owner, class_name: 'User'
......
module Ci
class Trigger < ActiveRecord::Base
extend Gitlab::Ci::Model
include IgnorableColumn
acts_as_paranoid
ignore_column :deleted_at
belongs_to :project
belongs_to :owner, class_name: "User"
......
......@@ -10,7 +10,6 @@ module InternalId
if iid.blank?
parent = project || group
records = parent.public_send(self.class.name.tableize) # rubocop:disable GitlabSecurity/PublicSend
records = records.with_deleted if self.paranoid?
max_iid = records.maximum(:iid)
self.iid = max_iid.to_i + 1
......
......@@ -12,7 +12,7 @@ class Issue < ActiveRecord::Base
include ThrottledTouch
include IgnorableColumn
ignore_column :assignee_id, :branch_name
ignore_column :assignee_id, :branch_name, :deleted_at
DueDateStruct = Struct.new(:title, :name).freeze
NoDueDate = DueDateStruct.new('No Due Date', '0').freeze
......@@ -78,8 +78,6 @@ class Issue < ActiveRecord::Base
end
end
acts_as_paranoid
class << self
alias_method :in_parents, :in_projects
end
......
......@@ -11,7 +11,8 @@ class MergeRequest < ActiveRecord::Base
include Gitlab::Utils::StrongMemoize
ignore_column :locked_at,
:ref_fetched
:ref_fetched,
:deleted_at
belongs_to :target_project, class_name: "Project"
belongs_to :source_project, class_name: "Project"
......@@ -150,8 +151,6 @@ class MergeRequest < ActiveRecord::Base
after_save :keep_around_commit
acts_as_paranoid
def self.reference_prefix
'!'
end
......
class Namespace < ActiveRecord::Base
acts_as_paranoid without_default_scope: true
include CacheMarkdownField
include Sortable
include Gitlab::ShellAdapter
......@@ -10,6 +8,9 @@ class Namespace < ActiveRecord::Base
include AfterCommitQueue
include Storage::LegacyNamespace
include Gitlab::SQL::Pattern
include IgnorableColumn
ignore_column :deleted_at
# Prevent users from creating unreasonably deep level of nesting.
# The number 20 was taken based on maximum nesting level of
......@@ -221,12 +222,6 @@ class Namespace < ActiveRecord::Base
has_parent?
end
def soft_delete_without_removing_associations
# We can't use paranoia's `#destroy` since this will hard-delete projects.
# Project uses `pending_delete` instead of the acts_as_paranoia gem.
self.deleted_at = Time.now
end
private
def refresh_access_of_projects_invited_groups
......
......@@ -6,7 +6,6 @@ class IssueEntity < IssuableEntity
expose :updated_by_id
expose :created_at
expose :updated_at
expose :deleted_at
expose :milestone, using: API::Entities::Milestone
expose :labels, using: LabelEntity
expose :lock_version
......
module Groups
class DestroyService < Groups::BaseService
def async_execute
group.soft_delete_without_removing_associations
job_id = GroupDestroyWorker.perform_async(group.id, current_user.id)
Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}")
end
......@@ -23,7 +22,7 @@ module Groups
group.chat_team&.remove_mattermost_team(current_user)
group.really_destroy!
group.destroy
end
end
end
......@@ -53,7 +53,7 @@ module Users
# Destroy the namespace after destroying the user since certain methods may depend on the namespace existing
user_data = user.destroy
namespace.really_destroy!
namespace.destroy
user_data
end
......
......@@ -4,7 +4,7 @@ class GroupDestroyWorker
def perform(group_id, user_id)
begin
group = Group.with_deleted.find(group_id)
group = Group.find(group_id)
rescue ActiveRecord::RecordNotFound
return
end
......
---
title: Remove soft removals related code
merge_request: 15789
author:
type: changed
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveSoftRemovedObjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
module SoftRemoved
extend ActiveSupport::Concern
included do
scope :soft_removed, -> { where('deleted_at IS NOT NULL') }
end
end
class User < ActiveRecord::Base
self.table_name = 'users'
include EachBatch
end
class Issue < ActiveRecord::Base
self.table_name = 'issues'
include EachBatch
include SoftRemoved
end
class MergeRequest < ActiveRecord::Base
self.table_name = 'merge_requests'
include EachBatch
include SoftRemoved
end
class Namespace < ActiveRecord::Base
self.table_name = 'namespaces'
include EachBatch
include SoftRemoved
scope :soft_removed_personal, -> { soft_removed.where(type: nil) }
scope :soft_removed_group, -> { soft_removed.where(type: 'Group') }
end
class Route < ActiveRecord::Base
self.table_name = 'routes'
include EachBatch
include SoftRemoved
end
class Project < ActiveRecord::Base
self.table_name = 'projects'
include EachBatch
include SoftRemoved
end
class CiPipelineSchedule < ActiveRecord::Base
self.table_name = 'ci_pipeline_schedules'
include EachBatch
include SoftRemoved
end
class CiTrigger < ActiveRecord::Base
self.table_name = 'ci_triggers'
include EachBatch
include SoftRemoved
end
MODELS = [Issue, MergeRequest, CiPipelineSchedule, CiTrigger].freeze
def up
disable_statement_timeout
remove_personal_routes
remove_personal_namespaces
remove_group_namespaces
remove_simple_soft_removed_rows
end
def down
# The data removed by this migration can't be restored in an automated way.
end
def remove_simple_soft_removed_rows
create_temporary_indexes
MODELS.each do |model|
say_with_time("Removing soft removed rows from #{model.table_name}") do
model.soft_removed.each_batch do |batch, index|
batch.delete_all
end
end
end
ensure
remove_temporary_indexes
end
def create_temporary_indexes
MODELS.each do |model|
index_name = temporary_index_name_for(model)
# Without this index the removal process can take a very long time. For
# example, getting the next ID of a batch for the `issues` table in
# staging would take between 15 and 20 seconds.
next if temporary_index_exists?(model)
say_with_time("Creating temporary index #{index_name}") do
add_concurrent_index(
model.table_name,
[:deleted_at, :id],
name: index_name,
where: 'deleted_at IS NOT NULL'
)
end
end
end
def remove_temporary_indexes
MODELS.each do |model|
index_name = temporary_index_name_for(model)
next unless temporary_index_exists?(model)
say_with_time("Removing temporary index #{index_name}") do
remove_concurrent_index_by_name(model.table_name, index_name)
end
end
end
def temporary_index_name_for(model)
"index_on_#{model.table_name}_tmp"
end
def temporary_index_exists?(model)
index_name = temporary_index_name_for(model)
index_exists?(model.table_name, [:deleted_at, :id], name: index_name)
end
def remove_personal_namespaces
# Some personal namespaces are left behind in case of GitLab.com. In these
# cases the associated data such as the projects and users has already been
# removed.
Namespace.soft_removed_personal.each_batch do |batch|
batch.delete_all
end
end
def remove_group_namespaces
# Left over groups can't be easily removed because we may also need to
# remove memberships, repositories, and other associated data. As a result
# we'll just schedule a Sidekiq job to remove these.
#
# As of January 5th, 2018 there are 36 groups that will be removed using
# this code.
Namespace.select(:id).soft_removed_group.each_batch(of: 10) do |batch, index|
# We need the ID of an admin user as the owners of the group may no longer
# exist (or might not even be set in `namespaces.owner_id`).
admin_id = id_for_admin_user
batch.each do |ns|
schedule_group_removal(index * 5.minutes, ns.id, admin_id)
end
end
end
def schedule_group_removal(delay, group_id, user_id)
if migrate_inline?
GroupDestroyWorker.new.perform(group_id, user_id)
else
GroupDestroyWorker.perform_in(delay, group_id, user_id)
end
end
def remove_personal_routes
namespaces = Namespace.select(1)
.soft_removed
.where('namespaces.type IS NULL')
.where('routes.source_type = ?', 'Namespace')
.where('routes.source_id = namespaces.id')
Route.where('EXISTS (?)', namespaces).each_batch do |batch|
batch.delete_all
end
end
def id_for_admin_user
return @id_for_admin_user if @id_for_admin_user
if (admin_id = User.where(admin: true).limit(1).pluck(:id).first)
@id_for_admin_user = admin_id
else
raise 'Can not remove soft removed groups as no admin user exists. ' \
'Please make sure at least one user with `admin` set to TRUE exists before proceeding.'
end
end
def migrate_inline?
Rails.env.test? || Rails.env.development?
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveDeletedAtColumns < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
TABLES = %i[issues merge_requests namespaces ci_pipeline_schedules ci_triggers].freeze
COLUMN = :deleted_at
def up
TABLES.each do |table|
remove_column(table, COLUMN) if column_exists?(table, COLUMN)
end
end
def down
TABLES.each do |table|
unless column_exists?(table, COLUMN)
add_column(table, COLUMN, :datetime_with_timezone)
end
unless index_exists?(table, COLUMN)
add_concurrent_index(table, COLUMN)
end
end
end
end
......@@ -356,7 +356,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
t.integer "project_id"
t.integer "owner_id"
t.boolean "active", default: true
t.datetime "deleted_at"
t.datetime "created_at"
t.datetime "updated_at"
end
......@@ -466,7 +465,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
create_table "ci_triggers", force: :cascade do |t|
t.string "token"
t.datetime "deleted_at"
t.datetime "created_at"
t.datetime "updated_at"
t.integer "project_id"
......@@ -860,7 +858,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
t.integer "iid"
t.integer "updated_by_id"
t.boolean "confidential", default: false, null: false
t.datetime "deleted_at"
t.date "due_date"
t.integer "moved_to_id"
t.integer "lock_version"
......@@ -877,7 +874,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree
add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree
add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree
add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree
add_index "issues", ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)", using: :btree
......@@ -1086,7 +1082,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
t.boolean "merge_when_pipeline_succeeds", default: false, null: false
t.integer "merge_user_id"
t.string "merge_commit_sha"
t.datetime "deleted_at"
t.string "in_progress_merge_commit_sha"
t.integer "lock_version"
t.text "title_html"
......@@ -1105,7 +1100,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree
add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree
add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree
add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
add_index "merge_requests", ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id", using: :btree
......@@ -1165,7 +1159,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
t.boolean "share_with_group_lock", default: false
t.integer "visibility_level", default: 20, null: false
t.boolean "request_access_enabled", default: false, null: false
t.datetime "deleted_at"
t.text "description_html"
t.boolean "lfs_enabled"
t.integer "parent_id"
......@@ -1175,7 +1168,6 @@ ActiveRecord::Schema.define(version: 20171230123729) do
end
add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
add_index "namespaces", ["deleted_at"], name: "index_namespaces_on_deleted_at", using: :btree
add_index "namespaces", ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", unique: true, using: :btree
add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
......
......@@ -24,7 +24,6 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/
"id": 10,
"description": "my trigger",
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z",
......@@ -55,7 +54,6 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/
"id": 10,
"description": "my trigger",
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z",
......@@ -85,7 +83,6 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form descri
"id": 10,
"description": "my trigger",
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z",
......@@ -116,7 +113,6 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form descrip
"id": 10,
"description": "my trigger",
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z",
......@@ -146,7 +142,6 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitl
"id": 10,
"description": "my trigger",
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z",
......
......@@ -918,7 +918,7 @@ module API
class Trigger < Grape::Entity
expose :id
expose :token, :description
expose :created_at, :updated_at, :deleted_at, :last_used
expose :created_at, :updated_at, :last_used
expose :owner, using: Entities::UserBasic
end
......
......@@ -207,7 +207,7 @@ module API
end
class Trigger < Grape::Entity
expose :token, :created_at, :updated_at, :deleted_at, :last_used
expose :token, :created_at, :updated_at, :last_used
expose :owner, using: ::API::Entities::UserBasic
end
......
......@@ -15,7 +15,6 @@ module Gitlab
query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id]))
.join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id]))
.where(issue_table[:project_id].eq(@project.id)) # rubocop:disable Gitlab/ModuleWithInstanceVariables
.where(issue_table[:deleted_at].eq(nil))
.where(issue_table[:created_at].gteq(@options[:from])) # rubocop:disable Gitlab/ModuleWithInstanceVariables
# Load merge_requests
......
......@@ -7,7 +7,6 @@ module Gitlab
closed_at
confidential
created_at
deleted_at
description
due_date
id
......
......@@ -5,7 +5,6 @@ module Gitlab
assignee_id
author_id
created_at
deleted_at
description
head_pipeline_id
id
......
......@@ -28,7 +28,6 @@
"confidential": { "type": "boolean" },
"discussion_locked": { "type": ["boolean", "null"] },
"updated_by_id": { "type": ["string", "null"] },
"deleted_at": { "type": ["string", "null"] },
"time_estimate": { "type": "integer" },
"total_time_spent": { "type": "integer" },
"human_time_estimate": { "type": ["integer", "null"] },
......
......@@ -13,7 +13,6 @@
"updated_by_id": { "type": ["string", "null"] },
"created_at": { "type": "string" },
"updated_at": { "type": "string" },
"deleted_at": { "type": ["string", "null"] },
"time_estimate": { "type": "integer" },
"total_time_spent": { "type": "integer" },
"human_time_estimate": { "type": ["integer", "null"] },
......
......@@ -29,7 +29,6 @@ export const noteableDataMock = {
can_create_note: true,
can_update: true,
},
deleted_at: null,
description: '',
due_date: null,
human_time_estimate: null,
......@@ -283,7 +282,6 @@ export const loggedOutnoteableData = {
"updated_by_id": 1,
"created_at": "2017-02-07T10:11:18.395Z",
"updated_at": "2017-08-08T10:22:51.564Z",
"deleted_at": null,
"time_estimate": 0,
"total_time_spent": 0,
"human_time_estimate": null,
......
......@@ -15,7 +15,6 @@ const RESPONSE_MAP = {
updated_by_id: 1,
created_at: '2017-02-02T21: 49: 49.664Z',
updated_at: '2017-05-03T22: 26: 03.760Z',
deleted_at: null,
time_estimate: 0,
total_time_spent: 0,
human_time_estimate: null,
......@@ -153,7 +152,6 @@ const RESPONSE_MAP = {
updated_by_id: 1,
created_at: '2017-06-27T19:54:42.437Z',
updated_at: '2017-08-18T03:39:49.222Z',
deleted_at: null,
time_estimate: 0,
total_time_spent: 0,
human_time_estimate: null,
......
......@@ -14,7 +14,6 @@ export default {
"updated_by_id": null,
"created_at": "2017-04-07T12:27:26.718Z",
"updated_at": "2017-04-07T15:39:25.852Z",
"deleted_at": null,
"time_estimate": 0,
"total_time_spent": 0,
"human_time_estimate": null,
......
......@@ -14,7 +14,6 @@ describe Gitlab::HookData::IssueBuilder do
closed_at
confidential
created_at
deleted_at
description
due_date
id
......
......@@ -12,7 +12,6 @@ describe Gitlab::HookData::MergeRequestBuilder do
assignee_id
author_id
created_at
deleted_at
description
head_pipeline_id
id
......
......@@ -54,7 +54,6 @@
"iid": 1,
"updated_by_id": 1,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"lock_version": null,
......@@ -134,7 +133,6 @@
"iid": 2,
"updated_by_id": 1,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"lock_version": null,
......
......@@ -56,7 +56,6 @@
"iid": 10,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"test_ee_field": "test",
......@@ -350,7 +349,6 @@
"iid": 9,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"milestone": {
......@@ -586,7 +584,6 @@
"iid": 8,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"label_links": [
......@@ -820,7 +817,6 @@
"iid": 7,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"notes": [
......@@ -1033,7 +1029,6 @@
"iid": 6,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"notes": [
......@@ -1246,7 +1241,6 @@
"iid": 5,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"notes": [
......@@ -1459,7 +1453,6 @@
"iid": 4,
"updated_by_id": null,
"confidential": false,
"deleted_at": null,
"due_date": null,
"moved_to_id": null,
"notes": [
......@@ -1672,7 +1665,6 @@
"iid": 3,
"updated_by_id": null,
"confidential": false,