From b5918f222b603058c0773f067f7925e026932992 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 22 May 2017 10:58:31 +0200 Subject: Fixes broken MySQL migration for retried > Mysql2::Error: Table 'ci_builds' is specified twice, both as a target for 'UPDATE' and as a separate source for data: UPDATE `ci_builds` SET `retried` = ((SELECT MAX(ci_builds2.id) Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/32647 --- .../20170503004427_upate_retried_for_ci_build.rb | 29 +++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/db/post_migrate/20170503004427_upate_retried_for_ci_build.rb b/db/post_migrate/20170503004427_upate_retried_for_ci_build.rb index 9b20edeb4c3..738e46b9207 100644 --- a/db/post_migrate/20170503004427_upate_retried_for_ci_build.rb +++ b/db/post_migrate/20170503004427_upate_retried_for_ci_build.rb @@ -8,6 +8,32 @@ class UpateRetriedForCiBuild < ActiveRecord::Migration def up disable_statement_timeout + if Gitlab::Database.mysql? + up_mysql + else + up_postgres + end + end + + def down + end + + private + + def up_mysql + # This is a trick to overcome MySQL limitation: + # Mysql2::Error: Table 'ci_builds' is specified twice, both as a target for 'UPDATE' and as a separate source for data + # However, this leads to create a temporary table from `max(ci_builds.id)` which is slow and do full database update + execute <<-SQL.strip_heredoc + UPDATE ci_builds SET retried= + (id NOT IN ( + SELECT * FROM (SELECT MAX(ci_builds.id) FROM ci_builds GROUP BY commit_id, name) AS latest_jobs + )) + WHERE retried IS NULL + SQL + end + + def up_postgres with_temporary_partial_index do latest_id = <<-SQL.strip_heredoc SELECT MAX(ci_builds2.id) @@ -26,9 +52,6 @@ class UpateRetriedForCiBuild < ActiveRecord::Migration end end - def down - end - def with_temporary_partial_index if Gitlab::Database.postgresql? execute 'CREATE INDEX CONCURRENTLY IF NOT EXISTS index_for_ci_builds_retried_migration ON ci_builds (id) WHERE retried IS NULL;' -- cgit v1.2.1 From 5d63a3939551116cd277ab4d32743146490a5e68 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 22 May 2017 17:55:58 +0800 Subject: Add a test to ensure this works on MySQL --- spec/migrations/update_retried_for_ci_builds_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 spec/migrations/update_retried_for_ci_builds_spec.rb diff --git a/spec/migrations/update_retried_for_ci_builds_spec.rb b/spec/migrations/update_retried_for_ci_builds_spec.rb new file mode 100644 index 00000000000..5cdb8a3c7da --- /dev/null +++ b/spec/migrations/update_retried_for_ci_builds_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170503004427_upate_retried_for_ci_build.rb') + +describe UpateRetriedForCiBuild, truncate: true do + let(:pipeline) { create(:ci_pipeline) } + let!(:build_old) { create(:ci_build, pipeline: pipeline, name: 'test') } + let!(:build_new) { create(:ci_build, pipeline: pipeline, name: 'test') } + + before do + described_class.new.up + end + + it 'updates ci_builds.is_retried' do + expect(build_old.reload).to be_retried + expect(build_new.reload).not_to be_retried + end +end -- cgit v1.2.1 From cbafe24a1fef2c13925490453cb97c3831d03169 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 22 May 2017 18:42:14 +0800 Subject: Respect the typo as rubocop said --- spec/migrations/upate_retried_for_ci_builds_spec.rb | 17 +++++++++++++++++ spec/migrations/update_retried_for_ci_builds_spec.rb | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 spec/migrations/upate_retried_for_ci_builds_spec.rb delete mode 100644 spec/migrations/update_retried_for_ci_builds_spec.rb diff --git a/spec/migrations/upate_retried_for_ci_builds_spec.rb b/spec/migrations/upate_retried_for_ci_builds_spec.rb new file mode 100644 index 00000000000..5cdb8a3c7da --- /dev/null +++ b/spec/migrations/upate_retried_for_ci_builds_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170503004427_upate_retried_for_ci_build.rb') + +describe UpateRetriedForCiBuild, truncate: true do + let(:pipeline) { create(:ci_pipeline) } + let!(:build_old) { create(:ci_build, pipeline: pipeline, name: 'test') } + let!(:build_new) { create(:ci_build, pipeline: pipeline, name: 'test') } + + before do + described_class.new.up + end + + it 'updates ci_builds.is_retried' do + expect(build_old.reload).to be_retried + expect(build_new.reload).not_to be_retried + end +end diff --git a/spec/migrations/update_retried_for_ci_builds_spec.rb b/spec/migrations/update_retried_for_ci_builds_spec.rb deleted file mode 100644 index 5cdb8a3c7da..00000000000 --- a/spec/migrations/update_retried_for_ci_builds_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20170503004427_upate_retried_for_ci_build.rb') - -describe UpateRetriedForCiBuild, truncate: true do - let(:pipeline) { create(:ci_pipeline) } - let!(:build_old) { create(:ci_build, pipeline: pipeline, name: 'test') } - let!(:build_new) { create(:ci_build, pipeline: pipeline, name: 'test') } - - before do - described_class.new.up - end - - it 'updates ci_builds.is_retried' do - expect(build_old.reload).to be_retried - expect(build_new.reload).not_to be_retried - end -end -- cgit v1.2.1