diff --git a/changelogs/unreleased/207312-remove-optimisticlocking-monkeypatch.yml b/changelogs/unreleased/207312-remove-optimisticlocking-monkeypatch.yml
new file mode 100644
index 0000000000000000000000000000000000000000..e2084b8170f51f1d2307509a89d113436a1154b2
--- /dev/null
+++ b/changelogs/unreleased/207312-remove-optimisticlocking-monkeypatch.yml
@@ -0,0 +1,5 @@
+---
+title: Remove Rails Optimistic Locking monkeypatch
+merge_request: 25566
+author:
+type: fixed
diff --git a/config/initializers/config_initializers_active_record_locking.rb b/config/initializers/config_initializers_active_record_locking.rb
deleted file mode 100644
index 9f9908283c635b6a47b8c923ba30246e40e846c2..0000000000000000000000000000000000000000
--- a/config/initializers/config_initializers_active_record_locking.rb
+++ /dev/null
@@ -1,46 +0,0 @@
-# frozen_string_literal: true
-
-# ensure ActiveRecord's version has been required already
-require 'active_record/locking/optimistic'
-
-# rubocop:disable Lint/RescueException
-module ActiveRecord
- module Locking
- module Optimistic
- private
-
- def _update_row(attribute_names, attempted_action = "update")
- return super unless locking_enabled?
-
- begin
- locking_column = self.class.locking_column
- previous_lock_value = read_attribute_before_type_cast(locking_column)
- attribute_names << locking_column
-
- self[locking_column] += 1
-
- # Patched because when `lock_version` is read as `0`, it may actually be `NULL` in the DB.
- possible_previous_lock_value = previous_lock_value.to_i == 0 ? [nil, 0] : previous_lock_value
-
- affected_rows = self.class.unscoped.where(
- locking_column => possible_previous_lock_value,
- self.class.primary_key => id_in_database
- ).update_all(
- attributes_with_values(attribute_names)
- )
-
- if affected_rows != 1
- raise ActiveRecord::StaleObjectError.new(self, attempted_action)
- end
-
- affected_rows
-
- # If something went wrong, revert the locking_column value.
- rescue Exception
- self[locking_column] = previous_lock_value.to_i
- raise
- end
- end
- end
- end
-end
diff --git a/db/post_migrate/20200608195222_set_lock_version_not_null_constraint.rb b/db/post_migrate/20200608195222_set_lock_version_not_null_constraint.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ec72053b3078a1dcc6da6b29500e592be1504883
--- /dev/null
+++ b/db/post_migrate/20200608195222_set_lock_version_not_null_constraint.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class SetLockVersionNotNullConstraint < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ TABLES = %i(epics merge_requests issues ci_stages ci_builds ci_pipelines).freeze
+
+ def up
+ TABLES.each do |table|
+ add_not_null_constraint table, :lock_version, validate: false
+ end
+ end
+
+ def down
+ TABLES.each do |table|
+ remove_not_null_constraint table, :lock_version
+ end
+ end
+end
diff --git a/db/post_migrate/20200608205813_set_lock_version_to_not_null.rb b/db/post_migrate/20200608205813_set_lock_version_to_not_null.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fc97484ff4d652822f543a90529b18bd4ac24f48
--- /dev/null
+++ b/db/post_migrate/20200608205813_set_lock_version_to_not_null.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class SetLockVersionToNotNull < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ MODELS = [Epic, MergeRequest, Issue, Ci::Stage, Ci::Build, Ci::Pipeline].freeze
+
+ disable_ddl_transaction!
+
+ def up
+ MODELS.each do |model|
+ model.where(lock_version: nil).update_all(lock_version: 0)
+ end
+ end
+
+ def down
+ # Nothing to do...
+ end
+end
diff --git a/db/post_migrate/20200608212030_lock_version_cleanup_for_epics.rb b/db/post_migrate/20200608212030_lock_version_cleanup_for_epics.rb
new file mode 100644
index 0000000000000000000000000000000000000000..70faa1caca0b3b153c2c07893981f203763d32e7
--- /dev/null
+++ b/db/post_migrate/20200608212030_lock_version_cleanup_for_epics.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForEpics < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :epics, :lock_version
+ remove_concurrent_index :epics, :lock_version, where: "lock_version IS NULL"
+ end
+
+ def down
+ add_concurrent_index :epics, :lock_version, where: "lock_version IS NULL"
+ end
+end
diff --git a/db/post_migrate/20200608212435_lock_version_cleanup_for_merge_requests.rb b/db/post_migrate/20200608212435_lock_version_cleanup_for_merge_requests.rb
new file mode 100644
index 0000000000000000000000000000000000000000..bc4ac3bdc3189ac2d460f40c79b3966874c40ca4
--- /dev/null
+++ b/db/post_migrate/20200608212435_lock_version_cleanup_for_merge_requests.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForMergeRequests < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :merge_requests, :lock_version
+ remove_concurrent_index :merge_requests, :lock_version, where: "lock_version IS NULL"
+ end
+
+ def down
+ add_concurrent_index :merge_requests, :lock_version, where: "lock_version IS NULL"
+ end
+end
diff --git a/db/post_migrate/20200608212549_lock_version_cleanup_for_issues.rb b/db/post_migrate/20200608212549_lock_version_cleanup_for_issues.rb
new file mode 100644
index 0000000000000000000000000000000000000000..21936ac4fa6885608ebcde7945069b98b83ab4ae
--- /dev/null
+++ b/db/post_migrate/20200608212549_lock_version_cleanup_for_issues.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForIssues < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :issues, :lock_version
+ remove_concurrent_index :issues, :lock_version, where: "lock_version IS NULL"
+ end
+
+ def down
+ add_concurrent_index :issues, :lock_version, where: "lock_version IS NULL"
+ end
+end
diff --git a/db/post_migrate/20200608212652_lock_version_cleanup_for_ci_stages.rb b/db/post_migrate/20200608212652_lock_version_cleanup_for_ci_stages.rb
new file mode 100644
index 0000000000000000000000000000000000000000..12e2897123e30ab2fc37f8276c2c943a60ee9dd8
--- /dev/null
+++ b/db/post_migrate/20200608212652_lock_version_cleanup_for_ci_stages.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForCiStages < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :ci_stages, :lock_version
+ remove_concurrent_index :ci_stages, :id, where: "lock_version IS NULL", name: "tmp_index_ci_stages_lock_version"
+ end
+
+ def down
+ add_concurrent_index :ci_stages, :id, where: "lock_version IS NULL", name: "tmp_index_ci_stages_lock_version"
+ end
+end
diff --git a/db/post_migrate/20200608212807_lock_version_cleanup_for_ci_builds.rb b/db/post_migrate/20200608212807_lock_version_cleanup_for_ci_builds.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0512869971bfbd6f10e1b9c30f018247bf1d0aab
--- /dev/null
+++ b/db/post_migrate/20200608212807_lock_version_cleanup_for_ci_builds.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForCiBuilds < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :ci_builds, :lock_version
+ remove_concurrent_index :ci_builds, :id, where: "lock_version IS NULL", name: "tmp_index_ci_builds_lock_version"
+ end
+
+ def down
+ add_concurrent_index :ci_builds, :id, where: "lock_version IS NULL", name: "tmp_index_ci_builds_lock_version"
+ end
+end
diff --git a/db/post_migrate/20200608212824_lock_version_cleanup_for_ci_pipelines.rb b/db/post_migrate/20200608212824_lock_version_cleanup_for_ci_pipelines.rb
new file mode 100644
index 0000000000000000000000000000000000000000..228dd72da8de551c1c02e426587890aa960b8d77
--- /dev/null
+++ b/db/post_migrate/20200608212824_lock_version_cleanup_for_ci_pipelines.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class LockVersionCleanupForCiPipelines < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ validate_not_null_constraint :ci_pipelines, :lock_version
+ remove_concurrent_index :ci_pipelines, :id, where: "lock_version IS NULL", name: "tmp_index_ci_pipelines_lock_version"
+ end
+
+ def down
+ add_concurrent_index :ci_pipelines, :id, where: "lock_version IS NULL", name: "tmp_index_ci_pipelines_lock_version"
+ end
+end
diff --git a/db/structure.sql b/db/structure.sql
index d6fce33622dd8568fecd2f640f30804594874980..73957ce9a69fc0723aebf8f3dbab771c2c3f3eb0 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -1052,7 +1052,8 @@ CREATE TABLE public.ci_builds (
resource_group_id bigint,
waiting_for_resource_at timestamp with time zone,
processed boolean,
- scheduling_type smallint
+ scheduling_type smallint,
+ CONSTRAINT check_1e2fbd1b39 CHECK ((lock_version IS NOT NULL))
);
CREATE SEQUENCE public.ci_builds_id_seq
@@ -1362,7 +1363,8 @@ CREATE TABLE public.ci_pipelines (
source_sha bytea,
target_sha bytea,
external_pull_request_id bigint,
- ci_ref_id bigint
+ ci_ref_id bigint,
+ CONSTRAINT check_d7e99a025e CHECK ((lock_version IS NOT NULL))
);
CREATE TABLE public.ci_pipelines_config (
@@ -1547,7 +1549,8 @@ CREATE TABLE public.ci_stages (
name character varying,
status integer,
lock_version integer DEFAULT 0,
- "position" integer
+ "position" integer,
+ CONSTRAINT check_81b431e49b CHECK ((lock_version IS NOT NULL))
);
CREATE SEQUENCE public.ci_stages_id_seq
@@ -2506,7 +2509,8 @@ CREATE TABLE public.epics (
start_date_sourcing_epic_id integer,
due_date_sourcing_epic_id integer,
confidential boolean DEFAULT false NOT NULL,
- external_key character varying(255)
+ external_key character varying(255),
+ CONSTRAINT check_fcfb4a93ff CHECK ((lock_version IS NOT NULL))
);
CREATE SEQUENCE public.epics_id_seq
@@ -3532,7 +3536,8 @@ CREATE TABLE public.issues (
promoted_to_epic_id integer,
health_status smallint,
external_key character varying(255),
- sprint_id bigint
+ sprint_id bigint,
+ CONSTRAINT check_fba63f706d CHECK ((lock_version IS NOT NULL))
);
CREATE SEQUENCE public.issues_id_seq
@@ -4111,7 +4116,8 @@ CREATE TABLE public.merge_requests (
state_id smallint DEFAULT 1 NOT NULL,
rebase_jid character varying,
squash_commit_sha bytea,
- sprint_id bigint
+ sprint_id bigint,
+ CONSTRAINT check_970d272570 CHECK ((lock_version IS NOT NULL))
);
CREATE TABLE public.merge_requests_closing_issues (
@@ -9835,8 +9841,6 @@ CREATE INDEX index_epics_on_iid ON public.epics USING btree (iid);
CREATE INDEX index_epics_on_last_edited_by_id ON public.epics USING btree (last_edited_by_id);
-CREATE INDEX index_epics_on_lock_version ON public.epics USING btree (lock_version) WHERE (lock_version IS NULL);
-
CREATE INDEX index_epics_on_parent_id ON public.epics USING btree (parent_id);
CREATE INDEX index_epics_on_start_date ON public.epics USING btree (start_date);
@@ -10079,8 +10083,6 @@ CREATE INDEX index_issues_on_duplicated_to_id ON public.issues USING btree (dupl
CREATE INDEX index_issues_on_last_edited_by_id ON public.issues USING btree (last_edited_by_id);
-CREATE INDEX index_issues_on_lock_version ON public.issues USING btree (lock_version) WHERE (lock_version IS NULL);
-
CREATE INDEX index_issues_on_milestone_id ON public.issues USING btree (milestone_id);
CREATE INDEX index_issues_on_moved_to_id ON public.issues USING btree (moved_to_id) WHERE (moved_to_id IS NOT NULL);
@@ -10247,8 +10249,6 @@ CREATE INDEX index_merge_requests_on_head_pipeline_id ON public.merge_requests U
CREATE INDEX index_merge_requests_on_latest_merge_request_diff_id ON public.merge_requests USING btree (latest_merge_request_diff_id);
-CREATE INDEX index_merge_requests_on_lock_version ON public.merge_requests USING btree (lock_version) WHERE (lock_version IS NULL);
-
CREATE INDEX index_merge_requests_on_merge_user_id ON public.merge_requests USING btree (merge_user_id) WHERE (merge_user_id IS NOT NULL);
CREATE INDEX index_merge_requests_on_milestone_id ON public.merge_requests USING btree (milestone_id);
@@ -11249,12 +11249,6 @@ CREATE INDEX tmp_build_stage_position_index ON public.ci_builds USING btree (sta
CREATE INDEX tmp_idx_on_user_id_where_bio_is_filled ON public.users USING btree (id) WHERE ((COALESCE(bio, ''::character varying))::text IS DISTINCT FROM ''::text);
-CREATE INDEX tmp_index_ci_builds_lock_version ON public.ci_builds USING btree (id) WHERE (lock_version IS NULL);
-
-CREATE INDEX tmp_index_ci_pipelines_lock_version ON public.ci_pipelines USING btree (id) WHERE (lock_version IS NULL);
-
-CREATE INDEX tmp_index_ci_stages_lock_version ON public.ci_stages USING btree (id) WHERE (lock_version IS NULL);
-
CREATE UNIQUE INDEX users_security_dashboard_projects_unique_index ON public.users_security_dashboard_projects USING btree (project_id, user_id);
CREATE UNIQUE INDEX vulnerability_feedback_unique_idx ON public.vulnerability_feedback USING btree (project_id, category, feedback_type, project_fingerprint);
@@ -13989,6 +13983,14 @@ COPY "schema_migrations" (version) FROM STDIN;
20200605093113
20200608072931
20200608075553
+20200608195222
+20200608205813
+20200608212030
+20200608212435
+20200608212549
+20200608212652
+20200608212807
+20200608212824
20200608214008
20200609002841
20200609142506
diff --git a/spec/migrations/cleanup_optimistic_locking_nulls_pt2_fixed_spec.rb b/spec/migrations/cleanup_optimistic_locking_nulls_pt2_fixed_spec.rb
index 2e5e450afc7004aac5f4632ea2eba0cfd9ff1ea3..d6dd90e1320d0cb2ec6fa1753f6a8cf96178a183 100644
--- a/spec/migrations/cleanup_optimistic_locking_nulls_pt2_fixed_spec.rb
+++ b/spec/migrations/cleanup_optimistic_locking_nulls_pt2_fixed_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20200427064130_cleanup_optimistic_locking_nulls_pt2_fixed.rb')
-describe CleanupOptimisticLockingNullsPt2Fixed, :migration do
+describe CleanupOptimisticLockingNullsPt2Fixed, :migration, schema: 20200219193117 do
test_tables = %w(ci_stages ci_builds ci_pipelines).freeze
test_tables.each do |table|
let(table.to_sym) { table(table.to_sym) }
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 291cccd72db94f49646be29a01b83c6a822e07ee..63f3e4671434f854ed57adc3706f9ae4407ba51f 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -95,29 +95,6 @@
end
end
- describe 'locking' do
- using RSpec::Parameterized::TableSyntax
-
- where(:lock_version) do
- [
- [0],
- ["0"]
- ]
- end
-
- with_them do
- it 'works when an issue has a NULL lock_version' do
- issue = create(:issue)
-
- described_class.where(id: issue.id).update_all('lock_version = NULL')
-
- issue.update!(lock_version: lock_version, title: 'locking test')
-
- expect(issue.reload.title).to eq('locking test')
- end
- end
- end
-
describe '.simple_sorts' do
it 'includes all keys' do
expect(described_class.simple_sorts.keys).to include(
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index c70ddac5da6bbfa97abf0d0e49d2a92f2609e7cc..dc2d760dbeb143808180976343f5f9cc37c799a2 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -55,29 +55,6 @@
end
end
- describe 'locking' do
- using RSpec::Parameterized::TableSyntax
-
- where(:lock_version) do
- [
- [0],
- ["0"]
- ]
- end
-
- with_them do
- it 'works when a merge request has a NULL lock_version' do
- merge_request = create(:merge_request)
-
- described_class.where(id: merge_request.id).update_all('lock_version = NULL')
-
- merge_request.update!(lock_version: lock_version, title: 'locking test')
-
- expect(merge_request.reload.title).to eq('locking test')
- end
- end
- end
-
describe '#squash_in_progress?' do
let(:repo_path) do
Gitlab::GitalyClient::StorageSettings.allow_disk_access do