From 5f9c84584efd5a7cbe19ada49fdccefbd5f54aea Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 7 Jul 2017 14:49:05 +0200 Subject: Added EachBatch for iterating tables in batches This module provides a class method called `each_batch` that can be used to iterate tables in batches in a more efficient way compared to Rails' `in_batches` method. This commit also includes a RuboCop cop to blacklist the use of `in_batches` in favour of this new method. --- app/models/concerns/each_batch.rb | 63 ++++++++++++++++++++++++++ doc/development/README.md | 1 + doc/development/iterating_tables_in_batches.md | 37 +++++++++++++++ lib/gitlab/otp_key_rotator.rb | 2 +- rubocop/cop/in_batches.rb | 16 +++++++ rubocop/rubocop.rb | 1 + spec/models/concerns/each_batch_spec.rb | 47 +++++++++++++++++++ spec/rubocop/cop/in_batches_spec.rb | 19 ++++++++ 8 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 app/models/concerns/each_batch.rb create mode 100644 doc/development/iterating_tables_in_batches.md create mode 100644 rubocop/cop/in_batches.rb create mode 100644 spec/models/concerns/each_batch_spec.rb create mode 100644 spec/rubocop/cop/in_batches_spec.rb diff --git a/app/models/concerns/each_batch.rb b/app/models/concerns/each_batch.rb new file mode 100644 index 00000000000..6610e7967d1 --- /dev/null +++ b/app/models/concerns/each_batch.rb @@ -0,0 +1,63 @@ +module EachBatch + extend ActiveSupport::Concern + + module ClassMethods + # Iterates over the rows in a relation in batches, similar to Rails' + # `in_batches` but in a more efficient way. + # + # Unlike `in_batches` provided by Rails this method does not support a + # custom start/end range, nor does it provide support for the `load:` + # keyword argument. + # + # This method will yield an ActiveRecord::Relation to the supplied block, or + # return an Enumerator if no block is given. + # + # Example: + # + # User.each_batch do |relation| + # relation.update_all(updated_at: Time.now) + # end + # + # This will produce SQL queries along the lines of: + # + # User Load (0.7ms) SELECT "users"."id" FROM "users" WHERE ("users"."id" >= 41654) ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1000 + # (0.7ms) SELECT COUNT(*) FROM "users" WHERE ("users"."id" >= 41654) AND ("users"."id" < 42687) + # + # of - The number of rows to retrieve per batch. + def each_batch(of: 1000) + start = except(:select) + .select(primary_key) + .reorder(primary_key => :asc) + .take + + return unless start + + start_id = start[primary_key] + arel_table = self.arel_table + + loop do + stop = except(:select) + .select(primary_key) + .where(arel_table[primary_key].gteq(start_id)) + .reorder(primary_key => :asc) + .offset(of) + .limit(1) + .take + + relation = where(arel_table[primary_key].gteq(start_id)) + + if stop + stop_id = stop[primary_key] + start_id = stop_id + relation = relation.where(arel_table[primary_key].lt(stop_id)) + end + + # Any ORDER BYs are useless for this relation and can lead to less + # efficient UPDATE queries, hence we get rid of it. + yield relation.except(:order) + + break unless stop + end + end + end +end diff --git a/doc/development/README.md b/doc/development/README.md index a2a07c37ced..58993c52dcd 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -55,6 +55,7 @@ - [Single Table Inheritance](single_table_inheritance.md) - [Background Migrations](background_migrations.md) - [Storing SHA1 Hashes As Binary](sha1_as_binary.md) +- [Iterating Tables In Batches](iterating_tables_in_batches.md) ## i18n diff --git a/doc/development/iterating_tables_in_batches.md b/doc/development/iterating_tables_in_batches.md new file mode 100644 index 00000000000..590c8cbba2d --- /dev/null +++ b/doc/development/iterating_tables_in_batches.md @@ -0,0 +1,37 @@ +# Iterating Tables In Batches + +Rails provides a method called `in_batches` that can be used to iterate over +rows in batches. For example: + +```ruby +User.in_batches(of: 10) do |relation| + relation.update_all(updated_at: Time.now) +end +``` + +Unfortunately this method is implemented in a way that is not very efficient, +both query and memory usage wise. + +To work around this you can include the `EachBatch` module into your models, +then use the `each_batch` class method. For example: + +```ruby +class User < ActiveRecord::Base + include EachBatch +end + +User.each_batch(of: 10) do |relation| + relation.update_all(updated_at: Time.now) +end +``` + +This will end up producing queries such as: + +``` +User Load (0.7ms) SELECT "users"."id" FROM "users" WHERE ("users"."id" >= 41654) ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1000 + (0.7ms) SELECT COUNT(*) FROM "users" WHERE ("users"."id" >= 41654) AND ("users"."id" < 42687) +``` + +The API of this method is similar to `in_batches`, though it doesn't support +all of the arguments that `in_batches` supports. You should always use +`each_batch` _unless_ you have a specific need for `in_batches`. diff --git a/lib/gitlab/otp_key_rotator.rb b/lib/gitlab/otp_key_rotator.rb index 0d541935bc6..22332474945 100644 --- a/lib/gitlab/otp_key_rotator.rb +++ b/lib/gitlab/otp_key_rotator.rb @@ -34,7 +34,7 @@ module Gitlab write_csv do |csv| ActiveRecord::Base.transaction do - User.with_two_factor.in_batches do |relation| + User.with_two_factor.in_batches do |relation| # rubocop: disable Cop/InBatches rows = relation.pluck(:id, :encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt) rows.each do |row| user = %i[id ciphertext iv salt].zip(row).to_h diff --git a/rubocop/cop/in_batches.rb b/rubocop/cop/in_batches.rb new file mode 100644 index 00000000000..c0240187e66 --- /dev/null +++ b/rubocop/cop/in_batches.rb @@ -0,0 +1,16 @@ +require_relative '../model_helpers' + +module RuboCop + module Cop + # Cop that prevents the use of `in_batches` + class InBatches < RuboCop::Cop::Cop + MSG = 'Do not use `in_batches`, use `each_batch` from the EachBatch module instead'.freeze + + def on_send(node) + return unless node.children[1] == :in_batches + + add_offense(node, :selector) + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1e70314598a..f76144275c9 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,6 +5,7 @@ require_relative 'cop/redirect_with_status' require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' require_relative 'cop/active_record_dependent' +require_relative 'cop/in_batches' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb new file mode 100644 index 00000000000..10d8c59eea4 --- /dev/null +++ b/spec/models/concerns/each_batch_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe EachBatch do + describe '.each_batch' do + let(:model) do + Class.new(ActiveRecord::Base) do + include EachBatch + + self.table_name = 'users' + end + end + + before do + 5.times { create(:user, updated_at: 1.day.ago) } + end + + it 'yields an ActiveRecord::Relation when a block is given' do + model.each_batch do |relation| + expect(relation).to be_a_kind_of(ActiveRecord::Relation) + end + end + + it 'accepts a custom batch size' do + amount = 0 + + model.each_batch(of: 1) { amount += 1 } + + expect(amount).to eq(5) + end + + it 'does not include ORDER BYs in the yielded relations' do + model.each_batch do |relation| + expect(relation.to_sql).not_to include('ORDER BY') + end + end + + it 'allows updating of the yielded relations' do + time = Time.now + + model.each_batch do |relation| + relation.update_all(updated_at: time) + end + + expect(model.where(updated_at: time).count).to eq(5) + end + end +end diff --git a/spec/rubocop/cop/in_batches_spec.rb b/spec/rubocop/cop/in_batches_spec.rb new file mode 100644 index 00000000000..072481984c6 --- /dev/null +++ b/spec/rubocop/cop/in_batches_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../rubocop/cop/in_batches' + +describe RuboCop::Cop::InBatches do + include CopHelper + + subject(:cop) { described_class.new } + + it 'registers an offense when in_batches is used' do + inspect_source(cop, 'foo.in_batches do; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end +end -- cgit v1.2.3 From 16ae7b7a49314b2525f3f32c07dd8a4891fa74e1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Jun 2017 11:29:04 +0200 Subject: Add initial stage_id background migration files --- ...858_migrate_stage_id_reference_in_background.rb | 12 ++++++++++ .../migrate_build_stage_id_reference.rb | 16 +++++++++++++ ...igrate_stage_id_reference_in_background_spec.rb | 26 ++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb create mode 100644 lib/gitlab/background_migration/migrate_build_stage_id_reference.rb create mode 100644 spec/migrations/migrate_stage_id_reference_in_background_spec.rb diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb new file mode 100644 index 00000000000..2eaa798d0aa --- /dev/null +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -0,0 +1,12 @@ +class MigrateStageIdReferenceInBackground < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + end + + def down + # noop + end +end diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb new file mode 100644 index 00000000000..b554c3e079b --- /dev/null +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -0,0 +1,16 @@ +module Gitlab + module BackgroundMigration + class MigrateBuildStageIdReference + class Build < ActiveRecord::Base + self.table_name = 'ci_builds' + end + + class Stage < ActiveRecord::Base + self.table_name = 'ci_stages' + end + + def perform(id) + end + end + end +end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb new file mode 100644 index 00000000000..f86ef834afa --- /dev/null +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') + +describe MigrateStageIdReferenceInBackground, :migration, :redis do + let(:jobs) { table(:ci_builds) } + let(:stages) { table(:ci_stages) } + let(:pipelines) { table(:ci_pipelines) } + let(:projects) { table(:projects) } + + before do + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') + + jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build') + jobs.create!(id: 2, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build') + jobs.create!(id: 3, commit_id: 1, project_id: 123, stage_idx: 1, stage: 'test') + jobs.create!(id: 4, commit_id: 1, project_id: 123, stage_idx: 3, stage: 'deploy') + + stages.create(id: 101, pipeline_id: 1, project_id: 123, name: 'test') + stages.create(id: 102, pipeline_id: 1, project_id: 123, name: 'build') + stages.create(id: 103, pipeline_id: 1, project_id: 123, name: 'deploy') + end + + it 'schedules background migrations' do + end +end -- cgit v1.2.3 From b7d672328db358690d043aae8b5fc24c358a52ab Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Jun 2017 12:01:52 +0200 Subject: Add initial build stage_id ref background migration --- ...8080858_migrate_stage_id_reference_in_background.rb | 10 ++++++++++ .../migrate_build_stage_id_reference.rb | 18 +++++++++++------- .../migrate_stage_id_reference_in_background_spec.rb | 5 +++++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 2eaa798d0aa..44bac4a8cc7 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -3,7 +3,17 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration DOWNTIME = false + disable_ddl_transaction! + + class Build < ActiveRecord::Base + self.table_name = 'ci_builds' + end + def up + Build.find_each do |build| + BackgroundMigrationWorker + .perform_async('MigrateBuildStageIdReference', [build.id]) + end end def down diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index b554c3e079b..87c6c4ed49f 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -1,15 +1,19 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference - class Build < ActiveRecord::Base - self.table_name = 'ci_builds' - end + def perform(id) + raise ArgumentError unless id.is_a?(Integer) - class Stage < ActiveRecord::Base - self.table_name = 'ci_stages' - end + sql = <<-SQL.strip_heredoc + UPDATE "ci_builds" SET "stage_id" = ( + SELECT id FROM ci_stages + WHERE ci_stages.pipeline_id = ci_builds.commit_id + AND ci_stages.name = ci_builds.stage + ) + WHERE "ci_builds"."id" = #{id} AND "ci_builds"."stage_id" IS NULL + SQL - def perform(id) + ActiveRecord::Base.connection.execute(sql) end end end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index f86ef834afa..ea3a18802d9 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -22,5 +22,10 @@ describe MigrateStageIdReferenceInBackground, :migration, :redis do end it 'schedules background migrations' do + expect(jobs.where(stage_id: nil)).to be_present + + migrate! + + expect(jobs.where(stage_id: nil)).to be_empty end end -- cgit v1.2.3 From 9a32ca409757b4b25520ea9957cdd4c8c97c0e95 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Jun 2017 12:21:25 +0200 Subject: Find builds that require a migration in batches --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 9 ++++++--- spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 44bac4a8cc7..6b326bc0b69 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -2,6 +2,8 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers DOWNTIME = false + BATCH_SIZE = 10000 + MIGRATION = 'MigrateBuildStageIdReference'.freeze disable_ddl_transaction! @@ -10,9 +12,10 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration end def up - Build.find_each do |build| - BackgroundMigrationWorker - .perform_async('MigrateBuildStageIdReference', [build.id]) + Build.find_in_batches(batch_size: BATCH_SIZE).with_index do |builds, batch| + migrations = builds.map { |build| [MIGRATION, [build.id]] } + + BackgroundMigrationWorker.perform_bulk(*migrations) end end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index ea3a18802d9..d515eb42b9d 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -8,6 +8,8 @@ describe MigrateStageIdReferenceInBackground, :migration, :redis do let(:projects) { table(:projects) } before do + stub_const('MigrateStageIdReferenceInBackground::BATCH_SIZE', 1) + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') -- cgit v1.2.3 From 1dc20f7ac7dbb68464cb4ea2727fd4e7171cfba8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Jun 2017 12:23:00 +0200 Subject: Schedule background migration only when it is needed --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 6b326bc0b69..bfeb09f6da1 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -12,11 +12,13 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration end def up - Build.find_in_batches(batch_size: BATCH_SIZE).with_index do |builds, batch| - migrations = builds.map { |build| [MIGRATION, [build.id]] } + Build.where(stage_id: nil) + .find_in_batches(batch_size: BATCH_SIZE) + .with_index do |builds, batch| + migrations = builds.map { |build| [MIGRATION, [build.id]] } - BackgroundMigrationWorker.perform_bulk(*migrations) - end + BackgroundMigrationWorker.perform_bulk(*migrations) + end end def down -- cgit v1.2.3 From 07693b43745e31e47466a9f1f5b73de894323a5d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Jun 2017 15:24:53 +0200 Subject: Add specs for delayed stage_id background migrations --- ...858_migrate_stage_id_reference_in_background.rb | 7 +-- ...igrate_stage_id_reference_in_background_spec.rb | 51 ++++++++++++++++++++-- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index bfeb09f6da1..a73456af386 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -15,9 +15,10 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration Build.where(stage_id: nil) .find_in_batches(batch_size: BATCH_SIZE) .with_index do |builds, batch| - migrations = builds.map { |build| [MIGRATION, [build.id]] } - - BackgroundMigrationWorker.perform_bulk(*migrations) + builds.each do |build| + schedule = (batch - 1) * 5.minutes + BackgroundMigrationWorker.perform_at(schedule, MIGRATION, [build.id]) + end end end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index d515eb42b9d..8b497656377 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -1,7 +1,37 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') -describe MigrateStageIdReferenceInBackground, :migration, :redis do +RSpec::Matchers.define :have_migrated do |*expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['enqueued_at'].present? && job['args'] == [migration, expected] + end + end + + failure_message do |migration| + <<-EOS + Background migration `#{migration}` with args `#{expected.inspect}` + not migrated! + EOS + end +end + +RSpec::Matchers.define :have_scheduled_migration do |time, *expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] && job['at'] >= time + end + end + + failure_message do |migration| + <<-EOS + Background migration `#{migration}` with args `#{expected.inspect}` + not scheduled! + EOS + end +end + +describe MigrateStageIdReferenceInBackground, :migration do let(:jobs) { table(:ci_builds) } let(:stages) { table(:ci_stages) } let(:pipelines) { table(:ci_pipelines) } @@ -23,11 +53,24 @@ describe MigrateStageIdReferenceInBackground, :migration, :redis do stages.create(id: 103, pipeline_id: 1, project_id: 123, name: 'deploy') end + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + migrate! + + expect(described_class::MIGRATION).to have_migrated(1) + expect(described_class::MIGRATION).to have_migrated(2) + expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 3) + expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 4) + end + end + it 'schedules background migrations' do - expect(jobs.where(stage_id: nil)).to be_present + Sidekiq::Testing.inline! do + expect(jobs.where(stage_id: nil)).to be_present - migrate! + migrate! - expect(jobs.where(stage_id: nil)).to be_empty + expect(jobs.where(stage_id: nil)).to be_empty + end end end -- cgit v1.2.3 From 945cdf326edcafdcd7a1d2aeaef611d888a4828e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 11:41:19 +0200 Subject: Make it possible to schedule bg migrations in bulk --- app/workers/background_migration_worker.rb | 18 +++++++++++-- doc/development/background_migrations.md | 19 +++++++++++--- spec/support/sidekiq.rb | 8 +++++- spec/workers/background_migration_worker_spec.rb | 33 +++++++++++++++++++++++- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index e85e221d353..751f37a3c39 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -2,18 +2,32 @@ class BackgroundMigrationWorker include Sidekiq::Worker include DedicatedSidekiqQueue - # Schedules a number of jobs in bulk + # Enqueues a number of jobs in bulk. # # The `jobs` argument should be an Array of Arrays, each sub-array must be in # the form: # # [migration-class, [arg1, arg2, ...]] - def self.perform_bulk(*jobs) + def self.perform_bulk(jobs) Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], 'args' => jobs) end + # Schedules a number of jobs in bulk, with a delay. + # + def self.perform_bulk_in(delay, jobs) + now = Time.now.to_f + schedule = now + delay.to_f + + raise ArgumentError if schedule <= now + + Sidekiq::Client.push_bulk('class' => self, + 'queue' => sidekiq_options['queue'], + 'args' => jobs, + 'at' => schedule) + end + # Performs the background migration. # # See Gitlab::BackgroundMigration.perform for more information. diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 0239e6b3163..a58f161fc30 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -50,14 +50,14 @@ your migration: BackgroundMigrationWorker.perform_async('BackgroundMigrationClassName', [arg1, arg2, ...]) ``` -Usually it's better to schedule jobs in bulk, for this you can use +Usually it's better to enqueue jobs in bulk, for this you can use `BackgroundMigrationWorker.perform_bulk`: ```ruby BackgroundMigrationWorker.perform_bulk( - ['BackgroundMigrationClassName', [1]], - ['BackgroundMigrationClassName', [2]], - ... + [['BackgroundMigrationClassName', [1]], + ['BackgroundMigrationClassName', [2]], + ...] ) ``` @@ -68,6 +68,17 @@ consuming migrations it's best to schedule a background job using an updates. Removals in turn can be handled by simply defining foreign keys with cascading deletes. +If you would like to schedule jobs in bulk with a delay, you can use +`BackgroundMigrationWorker.perform_bulk_in`: + +```ruby +jobs = [['BackgroundMigrationClassName', [1]], + ['BackgroundMigrationClassName', [2]], + ...] + +BackgroundMigrationWorker.perform_bulk_in(5.minutes, jobs) +``` + ## Cleaning Up Because background migrations can take a long time you can't immediately clean diff --git a/spec/support/sidekiq.rb b/spec/support/sidekiq.rb index 575d3451150..f3819ed2353 100644 --- a/spec/support/sidekiq.rb +++ b/spec/support/sidekiq.rb @@ -1,5 +1,11 @@ -require 'sidekiq/testing/inline' +require 'sidekiq/testing' Sidekiq::Testing.server_middleware do |chain| chain.add Gitlab::SidekiqStatus::ServerMiddleware end + +RSpec.configure do |config| + config.after(:each, :sidekiq) do + Sidekiq::Worker.clear_all + end +end diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb index 85939429feb..4f6e3474634 100644 --- a/spec/workers/background_migration_worker_spec.rb +++ b/spec/workers/background_migration_worker_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe BackgroundMigrationWorker do +describe BackgroundMigrationWorker, :sidekiq do describe '.perform' do it 'performs a background migration' do expect(Gitlab::BackgroundMigration) @@ -10,4 +10,35 @@ describe BackgroundMigrationWorker do described_class.new.perform('Foo', [10, 20]) end end + + describe '.perform_bulk' do + it 'enqueues background migrations in bulk' do + Sidekiq::Testing.fake! do + described_class.perform_bulk([['Foo', [1]], ['Foo', [2]]]) + + expect(described_class.jobs.count).to eq 2 + expect(described_class.jobs).to all(include('enqueued_at')) + end + end + end + + describe '.perform_bulk_in' do + context 'when delay is valid' do + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + described_class.perform_bulk_in(1.minute, [['Foo', [1]], ['Foo', [2]]]) + + expect(described_class.jobs.count).to eq 2 + expect(described_class.jobs).to all(include('at')) + end + end + end + + context 'when delay is invalid' do + it 'raises an ArgumentError exception' do + expect { described_class.perform_bulk_in(-60, [['Foo']]) } + .to raise_error(ArgumentError) + end + end + end end -- cgit v1.2.3 From c2efb8acc609eaec799b536412d1f66311aa1cf7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 12:10:29 +0200 Subject: Use ActiveRecord 5 batching to schedule bg migration --- app/workers/background_migration_worker.rb | 2 +- config/initializers/ar5_batching.rb | 4 +- ...858_migrate_stage_id_reference_in_background.rb | 7 ++- ...igrate_stage_id_reference_in_background_spec.rb | 55 ++++++++++------------ 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index 751f37a3c39..23c297de8bc 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -14,7 +14,7 @@ class BackgroundMigrationWorker 'args' => jobs) end - # Schedules a number of jobs in bulk, with a delay. + # Schedules multiple jobs in bulk, with a delay. # def self.perform_bulk_in(delay, jobs) now = Time.now.to_f diff --git a/config/initializers/ar5_batching.rb b/config/initializers/ar5_batching.rb index 35e8b3808e2..31efef83a6f 100644 --- a/config/initializers/ar5_batching.rb +++ b/config/initializers/ar5_batching.rb @@ -15,7 +15,7 @@ module ActiveRecord relation = relation.where(arel_table[primary_key].lteq(finish)) if finish batch_relation = relation - loop do + 1.step do |index| if load records = batch_relation.records ids = records.map(&:id) @@ -31,7 +31,7 @@ module ActiveRecord primary_key_offset = ids.last raise ArgumentError.new("Primary key not included in the custom select clause") unless primary_key_offset - yield yielded_relation + yield yielded_relation, index break if ids.length < of batch_relation = relation.where(arel_table[primary_key].gt(primary_key_offset)) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index a73456af386..9e95216b35a 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -13,10 +13,9 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration def up Build.where(stage_id: nil) - .find_in_batches(batch_size: BATCH_SIZE) - .with_index do |builds, batch| - builds.each do |build| - schedule = (batch - 1) * 5.minutes + .in_batches(of: BATCH_SIZE) do |relation, index| + schedule = index * 5.minutes + relation.each do |build| BackgroundMigrationWorker.perform_at(schedule, MIGRATION, [build.id]) end end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 8b497656377..d3645ec0395 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -1,44 +1,39 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') -RSpec::Matchers.define :have_migrated do |*expected| - match do |migration| - BackgroundMigrationWorker.jobs.any? do |job| - job['enqueued_at'].present? && job['args'] == [migration, expected] +describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do + matcher :have_migrated do |*expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['enqueued_at'].present? && job['args'] == [migration, expected] + end end - end - failure_message do |migration| - <<-EOS - Background migration `#{migration}` with args `#{expected.inspect}` - not migrated! - EOS + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not executed!" + end end -end -RSpec::Matchers.define :have_scheduled_migration do |time, *expected| - match do |migration| - BackgroundMigrationWorker.jobs.any? do |job| - job['args'] == [migration, expected] && job['at'] >= time + matcher :have_scheduled_migration do |delay, *expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] && + job['at'].to_f == (delay.to_f + Time.now.to_f) + end end - end - failure_message do |migration| - <<-EOS - Background migration `#{migration}` with args `#{expected.inspect}` - not scheduled! - EOS + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" + end end -end -describe MigrateStageIdReferenceInBackground, :migration do let(:jobs) { table(:ci_builds) } let(:stages) { table(:ci_stages) } let(:pipelines) { table(:ci_pipelines) } let(:projects) { table(:projects) } before do - stub_const('MigrateStageIdReferenceInBackground::BATCH_SIZE', 1) + stub_const('MigrateStageIdReferenceInBackground::BATCH_SIZE', 2) projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') @@ -55,12 +50,14 @@ describe MigrateStageIdReferenceInBackground, :migration do it 'correctly schedules background migrations' do Sidekiq::Testing.fake! do - migrate! + Timecop.freeze do + migrate! - expect(described_class::MIGRATION).to have_migrated(1) - expect(described_class::MIGRATION).to have_migrated(2) - expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 3) - expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 4) + expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 1) + expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 2) + expect(described_class::MIGRATION).to have_scheduled_migration(10.minutes, 3) + expect(described_class::MIGRATION).to have_scheduled_migration(10.minutes, 4) + end end end -- cgit v1.2.3 From 00ca74810814a9f35a14755f1b91169096640dba Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 12:14:41 +0200 Subject: Remove unused background migrations matcher --- .../migrate_stage_id_reference_in_background_spec.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index d3645ec0395..3eeca2e9659 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -2,18 +2,6 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do - matcher :have_migrated do |*expected| - match do |migration| - BackgroundMigrationWorker.jobs.any? do |job| - job['enqueued_at'].present? && job['args'] == [migration, expected] - end - end - - failure_message do |migration| - "Migration `#{migration}` with args `#{expected.inspect}` not executed!" - end - end - matcher :have_scheduled_migration do |delay, *expected| match do |migration| BackgroundMigrationWorker.jobs.any? do |job| -- cgit v1.2.3 From 316d8821cdcf048d08478b255060cf7645e33049 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 12:14:54 +0200 Subject: Perform stage_id ref backgound migration in bulks --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 9e95216b35a..c54e8bde095 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -15,9 +15,9 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration Build.where(stage_id: nil) .in_batches(of: BATCH_SIZE) do |relation, index| schedule = index * 5.minutes - relation.each do |build| - BackgroundMigrationWorker.perform_at(schedule, MIGRATION, [build.id]) - end + jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } + + BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) end end -- cgit v1.2.3 From 825521539d26b6434fa8e15b07051c29bccba1c9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 12:26:37 +0200 Subject: Improve specs for background stage_id ref migration --- ...080858_migrate_stage_id_reference_in_background.rb | 11 +++++------ .../migrate_stage_id_reference_in_background_spec.rb | 19 +++++++++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index c54e8bde095..1d95fc62c87 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -12,13 +12,12 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration end def up - Build.where(stage_id: nil) - .in_batches(of: BATCH_SIZE) do |relation, index| - schedule = index * 5.minutes - jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } + Build.where(stage_id: nil).in_batches(of: BATCH_SIZE) do |relation, index| + schedule = index * 5.minutes + jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } - BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) - end + BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) + end end def down diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 3eeca2e9659..046d9b351a8 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background') describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do - matcher :have_scheduled_migration do |delay, *expected| + matcher :be_scheduled_migration do |delay, *expected| match do |migration| BackgroundMigrationWorker.jobs.any? do |job| job['args'] == [migration, expected] && @@ -24,16 +24,22 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do stub_const('MigrateStageIdReferenceInBackground::BATCH_SIZE', 2) projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 345, name: 'gitlab2', path: 'gitlab2') + pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') + pipelines.create!(id: 2, project_id: 345, ref: 'feature', sha: 'cdf43c3c') jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build') jobs.create!(id: 2, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build') jobs.create!(id: 3, commit_id: 1, project_id: 123, stage_idx: 1, stage: 'test') jobs.create!(id: 4, commit_id: 1, project_id: 123, stage_idx: 3, stage: 'deploy') + jobs.create!(id: 5, commit_id: 2, project_id: 345, stage_idx: 1, stage: 'test') stages.create(id: 101, pipeline_id: 1, project_id: 123, name: 'test') stages.create(id: 102, pipeline_id: 1, project_id: 123, name: 'build') stages.create(id: 103, pipeline_id: 1, project_id: 123, name: 'deploy') + + jobs.create!(id: 6, commit_id: 2, project_id: 345, stage_id: 101, stage_idx: 1, stage: 'test') end it 'correctly schedules background migrations' do @@ -41,10 +47,11 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 1) - expect(described_class::MIGRATION).to have_scheduled_migration(5.minutes, 2) - expect(described_class::MIGRATION).to have_scheduled_migration(10.minutes, 3) - expect(described_class::MIGRATION).to have_scheduled_migration(10.minutes, 4) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 4) + expect(BackgroundMigrationWorker.jobs.size).to eq 5 end end end @@ -55,7 +62,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do migrate! - expect(jobs.where(stage_id: nil)).to be_empty + expect(jobs.where(stage_id: nil)).to be_one end end end -- cgit v1.2.3 From 55f93db876f1bd62bde09d4003a0ef9a02adb04c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 14:31:12 +0200 Subject: Make `inline` a default sidekiq testing processing again --- spec/support/sidekiq.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/sidekiq.rb b/spec/support/sidekiq.rb index f3819ed2353..5478fea4e64 100644 --- a/spec/support/sidekiq.rb +++ b/spec/support/sidekiq.rb @@ -1,4 +1,4 @@ -require 'sidekiq/testing' +require 'sidekiq/testing/inline' Sidekiq::Testing.server_middleware do |chain| chain.add Gitlab::SidekiqStatus::ServerMiddleware -- cgit v1.2.3 From 989785a31def17a1f0b29b84f8920db96ccf80b1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 15:20:26 +0200 Subject: Test if argument passed to a migration is present --- lib/gitlab/background_migration/migrate_build_stage_id_reference.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index 87c6c4ed49f..711126ea0d3 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -2,7 +2,7 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference def perform(id) - raise ArgumentError unless id.is_a?(Integer) + raise ArgumentError unless id.present? sql = <<-SQL.strip_heredoc UPDATE "ci_builds" SET "stage_id" = ( -- cgit v1.2.3 From 01128b130b32fac3481fb3b386b649cb047b4b1f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 15:22:06 +0200 Subject: Use integers to schedule delayed background migrations --- app/workers/background_migration_worker.rb | 4 ++-- spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index 23c297de8bc..e6ca1159b38 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -17,8 +17,8 @@ class BackgroundMigrationWorker # Schedules multiple jobs in bulk, with a delay. # def self.perform_bulk_in(delay, jobs) - now = Time.now.to_f - schedule = now + delay.to_f + now = Time.now.to_i + schedule = now + delay.to_i raise ArgumentError if schedule <= now diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 046d9b351a8..1bd2c14b61c 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -6,7 +6,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do match do |migration| BackgroundMigrationWorker.jobs.any? do |job| job['args'] == [migration, expected] && - job['at'].to_f == (delay.to_f + Time.now.to_f) + job['at'].to_f == (delay.to_i + Time.now.to_i) end end -- cgit v1.2.3 From d953f1762ea9d4be0e53d5280b9f38224b39e67b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 15:26:47 +0200 Subject: Improve readability of build stage id migration query --- .../migrate_build_stage_id_reference.rb | 13 +++++++------ .../migrate_stage_id_reference_in_background_spec.rb | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index 711126ea0d3..c8669ca3272 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -5,12 +5,13 @@ module Gitlab raise ArgumentError unless id.present? sql = <<-SQL.strip_heredoc - UPDATE "ci_builds" SET "stage_id" = ( - SELECT id FROM ci_stages - WHERE ci_stages.pipeline_id = ci_builds.commit_id - AND ci_stages.name = ci_builds.stage - ) - WHERE "ci_builds"."id" = #{id} AND "ci_builds"."stage_id" IS NULL + UPDATE "ci_builds" + SET "stage_id" = + (SELECT id FROM ci_stages + WHERE ci_stages.pipeline_id = ci_builds.commit_id + AND ci_stages.name = ci_builds.stage) + WHERE "ci_builds"."id" = #{id} + AND "ci_builds"."stage_id" IS NULL SQL ActiveRecord::Base.connection.execute(sql) diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 1bd2c14b61c..63787d71233 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -58,11 +58,11 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do it 'schedules background migrations' do Sidekiq::Testing.inline! do - expect(jobs.where(stage_id: nil)).to be_present + expect(jobs.where(stage_id: nil).count).to eq 5 migrate! - expect(jobs.where(stage_id: nil)).to be_one + expect(jobs.where(stage_id: nil).count).to eq 1 end end end -- cgit v1.2.3 From 14e45c424fa6b147cb32e2ef268b89ab08c37f1f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 29 Jun 2017 15:36:39 +0200 Subject: Do not compare float with integer in migrations specs --- spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 63787d71233..2e5504c849d 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -6,7 +6,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do match do |migration| BackgroundMigrationWorker.jobs.any? do |job| job['args'] == [migration, expected] && - job['at'].to_f == (delay.to_i + Time.now.to_i) + job['at'].to_i == (delay.to_i + Time.now.to_i) end end -- cgit v1.2.3 From 939473076a5ce357e978ffa24db9ef3981424342 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 30 Jun 2017 12:05:33 +0200 Subject: Add description to exception in bg migrations worker --- app/workers/background_migration_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index e6ca1159b38..e7ed71a687c 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -20,7 +20,7 @@ class BackgroundMigrationWorker now = Time.now.to_i schedule = now + delay.to_i - raise ArgumentError if schedule <= now + raise ArgumentError, 'Delay time invalid!' if schedule <= now Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], -- cgit v1.2.3 From 4fa822ae9d1a985c7cd9d5a63eae3a623fb16f50 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 30 Jun 2017 12:06:29 +0200 Subject: Improve code examples in background migrations docs --- doc/development/background_migrations.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index a58f161fc30..72a34aa7de9 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -56,8 +56,7 @@ Usually it's better to enqueue jobs in bulk, for this you can use ```ruby BackgroundMigrationWorker.perform_bulk( [['BackgroundMigrationClassName', [1]], - ['BackgroundMigrationClassName', [2]], - ...] + ['BackgroundMigrationClassName', [2]]] ) ``` @@ -73,8 +72,7 @@ If you would like to schedule jobs in bulk with a delay, you can use ```ruby jobs = [['BackgroundMigrationClassName', [1]], - ['BackgroundMigrationClassName', [2]], - ...] + ['BackgroundMigrationClassName', [2]]] BackgroundMigrationWorker.perform_bulk_in(5.minutes, jobs) ``` -- cgit v1.2.3 From 84d9789d57edf1c09b8a63bbd4f455c9ee437aa7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 30 Jun 2017 12:07:31 +0200 Subject: Sanitize id value passed to async background migration --- lib/gitlab/background_migration/migrate_build_stage_id_reference.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index c8669ca3272..d1d0a968588 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -2,15 +2,13 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference def perform(id) - raise ArgumentError unless id.present? - sql = <<-SQL.strip_heredoc UPDATE "ci_builds" SET "stage_id" = (SELECT id FROM ci_stages WHERE ci_stages.pipeline_id = ci_builds.commit_id AND ci_stages.name = ci_builds.stage) - WHERE "ci_builds"."id" = #{id} + WHERE "ci_builds"."id" = #{id.to_i} AND "ci_builds"."stage_id" IS NULL SQL -- cgit v1.2.3 From 3408157155924e7bc817960767f69cc1fce9a5a5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 30 Jun 2017 13:03:47 +0200 Subject: Do not override original AR5 batching interface --- config/initializers/ar5_batching.rb | 4 ++-- .../20170628080858_migrate_stage_id_reference_in_background.rb | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/config/initializers/ar5_batching.rb b/config/initializers/ar5_batching.rb index 31efef83a6f..35e8b3808e2 100644 --- a/config/initializers/ar5_batching.rb +++ b/config/initializers/ar5_batching.rb @@ -15,7 +15,7 @@ module ActiveRecord relation = relation.where(arel_table[primary_key].lteq(finish)) if finish batch_relation = relation - 1.step do |index| + loop do if load records = batch_relation.records ids = records.map(&:id) @@ -31,7 +31,7 @@ module ActiveRecord primary_key_offset = ids.last raise ArgumentError.new("Primary key not included in the custom select clause") unless primary_key_offset - yield yielded_relation, index + yield yielded_relation break if ids.length < of batch_relation = relation.where(arel_table[primary_key].gt(primary_key_offset)) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 1d95fc62c87..30849ea1361 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -12,9 +12,12 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration end def up - Build.where(stage_id: nil).in_batches(of: BATCH_SIZE) do |relation, index| - schedule = index * 5.minutes + index = 1 + + Build.where(stage_id: nil).in_batches(of: BATCH_SIZE) do |relation| jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } + schedule = index * 5.minutes + index += 1 BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) end -- cgit v1.2.3 From 5e2baaf39fc53e154d40b130eddbec756274cfe2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 3 Jul 2017 13:00:38 +0200 Subject: Improve exception description in bg migrations --- app/workers/background_migration_worker.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index e7ed71a687c..45ce49bb5c0 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -20,7 +20,9 @@ class BackgroundMigrationWorker now = Time.now.to_i schedule = now + delay.to_i - raise ArgumentError, 'Delay time invalid!' if schedule <= now + if schedule <= now + raise ArgumentError, 'The schedule time must be in the future!' + end Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], -- cgit v1.2.3 From 24a4199ac5eb517e7a27e458d5d1b4f937480a31 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 3 Jul 2017 13:02:51 +0200 Subject: Reduce a delay between stage_id scheduled migrations --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 2 +- spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 30849ea1361..ebec4cb6bb7 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -16,7 +16,7 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration Build.where(stage_id: nil).in_batches(of: BATCH_SIZE) do |relation| jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } - schedule = index * 5.minutes + schedule = index * 2.minutes index += 1 BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 2e5504c849d..a32a7fceb68 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -47,10 +47,10 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1) - expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2) - expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3) - expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 4) + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4) expect(BackgroundMigrationWorker.jobs.size).to eq 5 end end -- cgit v1.2.3 From e036a3743069b7d398117c6742bd6c21239e879a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 6 Jul 2017 15:31:54 +0200 Subject: Add walk_table_in_batches and refactor migration helpers --- lib/gitlab/database/migration_helpers.rb | 75 ++++++++++++++-------- spec/lib/gitlab/database/migration_helpers_spec.rb | 53 +++++++++++++-- 2 files changed, 97 insertions(+), 31 deletions(-) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 0643c56db9b..29e812c3c78 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -221,17 +221,19 @@ module Gitlab # make things _more_ complex). # # rubocop: disable Metrics/AbcSize - def update_column_in_batches(table, column, value) + def update_column_in_batches(table, column, value, &scope) if transaction_open? - raise 'update_column_in_batches can not be run inside a transaction, ' \ - 'you can disable transactions by calling disable_ddl_transaction! ' \ - 'in the body of your migration class' + raise <<-MSG + update_column_in_batches helper can not be run inside a transaction. + You can disable transactions by calling `disable_ddl_transaction!` + method in the body of your migration class. + MSG end - table = Arel::Table.new(table) + table_arel = Arel::Table.new(table) - count_arel = table.project(Arel.star.count.as('count')) - count_arel = yield table, count_arel if block_given? + count_arel = table_arel.project(Arel.star.count.as('count')) + count_arel = yield table_arel, count_arel if block_given? total = exec_query(count_arel.to_sql).to_hash.first['count'].to_i @@ -246,37 +248,56 @@ module Gitlab # rows for GitLab.com. batch_size = max_size if batch_size > max_size + walk_table_in_batches(table, of: batch_size, scope: scope) do + Arel::UpdateManager.new(ActiveRecord::Base) + .table(table_arel) + .set([[table_arel[column], value]]) + end + end + + def walk_table_in_batches(table, of: 1000, scope: nil) + if transaction_open? + raise <<-MSG + walk_table_in_batches helper can not be run inside a transaction. + You can disable transactions by calling `disable_ddl_transaction!` + method in the body of your migration class. + MSG + end + + table = Arel::Table.new(table) + start_arel = table.project(table[:id]).order(table[:id].asc).take(1) - start_arel = yield table, start_arel if block_given? - start_id = exec_query(start_arel.to_sql).to_hash.first['id'].to_i + start_arel = scope.call(table, start_arel) if scope + start_id = exec_query(start_arel.to_sql).to_hash.first.to_h['id'].to_i - loop do + 1.step do |batch| stop_arel = table.project(table[:id]) .where(table[:id].gteq(start_id)) .order(table[:id].asc) .take(1) - .skip(batch_size) - - stop_arel = yield table, stop_arel if block_given? - stop_row = exec_query(stop_arel.to_sql).to_hash.first + .skip(of) - update_arel = Arel::UpdateManager.new(ActiveRecord::Base) - .table(table) - .set([[table[column], value]]) - .where(table[:id].gteq(start_id)) + stop_arel = scope.call(table, stop_arel) if scope + stop_id = exec_query(stop_arel.to_sql) + .to_hash.first.to_h['id'].to_i - if stop_row - stop_id = stop_row['id'].to_i - start_id = stop_id - update_arel = update_arel.where(table[:id].lt(stop_id)) - end + action = yield(batch, start_id, stop_id) - update_arel = yield table, update_arel if block_given? + if action.is_a?(Arel::TreeManager) + exec_arel = action.where(table[:id].gteq(start_id)) + exec_arel = exec_arel.where(table[:id].lt(stop_id)) if stop_id.nonzero? + exec_arel = scope.call(table, exec_arel) if scope - execute(update_arel.to_sql) + execute(exec_arel.to_sql) + end - # There are no more rows left to update. - break unless stop_row + if stop_id.zero? + # there are no more rows left to update + break + else + # next loop + start_id = stop_id + end end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 4259be3f522..65af2e1cb52 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' describe Gitlab::Database::MigrationHelpers, lib: true do let(:model) do - ActiveRecord::Migration.new.extend( - Gitlab::Database::MigrationHelpers - ) + ActiveRecord::Migration.new.extend(described_class) end before do @@ -264,7 +262,7 @@ describe Gitlab::Database::MigrationHelpers, lib: true do describe '#update_column_in_batches' do context 'when running outside of a transaction' do before do - expect(model).to receive(:transaction_open?).and_return(false) + expect(model).to receive(:transaction_open?).twice.and_return(false) create_list(:empty_project, 5) end @@ -313,6 +311,53 @@ describe Gitlab::Database::MigrationHelpers, lib: true do end end + describe '#walk_table_in_batches' do + context 'when running outside of a transaction' do + before do + expect(model).to receive(:transaction_open?).and_return(false) + + create_list(:empty_project, 6) + end + + it 'yields for each batch' do + expect { |b| model.walk_table_in_batches(:projects, of: 2, &b) } + .to yield_control.exactly(3).times + end + + it 'yields successive ranges' do + expect { |b| model.walk_table_in_batches(:projects, of: 2, &b) } + .to yield_successive_args([1, Integer, Integer], + [2, Integer, Integer], + [3, Integer, 0]) + end + + context 'when a scope is provided' do + it 'limits the scope of the statement provided inside the block' do + first_id = Project.first.id + scope = ->(table, query) { query.where(table[:id].eq(first_id)) } + + model.walk_table_in_batches(:projects, scope: scope) do + Arel::UpdateManager.new(ActiveRecord::Base) + .table(Arel::Table.new(:projects)) + .set([[Arel::Table.new(:projects)[:archived], true]]) + end + + expect(Project.where(archived: true).count).to eq(1) + end + end + end + + context 'when running inside the transaction' do + it 'raises RuntimeError' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect do + model.update_column_in_batches(:projects, :star_count, Arel.sql('1+1')) + end.to raise_error(RuntimeError) + end + end + end + describe '#add_column_with_default' do context 'outside of a transaction' do context 'when a column limit is not set' do -- cgit v1.2.3 From 2917f4868a19ab17a0217703c7b842261f8b9e46 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 6 Jul 2017 22:15:12 +0200 Subject: Extract `execute_in_batches` migrations helper --- lib/gitlab/database/migration_helpers.rb | 39 ++++++++++++-------- spec/lib/gitlab/database/migration_helpers_spec.rb | 43 ++++++++++++++++++---- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 29e812c3c78..5acc96331c0 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -248,7 +248,7 @@ module Gitlab # rows for GitLab.com. batch_size = max_size if batch_size > max_size - walk_table_in_batches(table, of: batch_size, scope: scope) do + execute_in_batches(table, of: batch_size, scope: scope) do Arel::UpdateManager.new(ActiveRecord::Base) .table(table_arel) .set([[table_arel[column], value]]) @@ -281,23 +281,32 @@ module Gitlab stop_id = exec_query(stop_arel.to_sql) .to_hash.first.to_h['id'].to_i - action = yield(batch, start_id, stop_id) + yield batch, start_id, stop_id - if action.is_a?(Arel::TreeManager) - exec_arel = action.where(table[:id].gteq(start_id)) - exec_arel = exec_arel.where(table[:id].lt(stop_id)) if stop_id.nonzero? - exec_arel = scope.call(table, exec_arel) if scope + stop_id.zero? ? break : start_id = stop_id + end + end - execute(exec_arel.to_sql) - end + def execute_in_batches(table, of: 1000, scope: nil) + if transaction_open? + raise <<-MSG + execute_in_batches helper can not be run inside a transaction. + You can disable transactions by calling `disable_ddl_transaction!` + method in the body of your migration class. + MSG + end - if stop_id.zero? - # there are no more rows left to update - break - else - # next loop - start_id = stop_id - end + # raise 'This method requires a block!' unless block_given? + + table_arel = Arel::Table.new(table) + + walk_table_in_batches(table, of: of, scope: scope) do |_batch, start_id, stop_id| + exec_arel = yield table_arel + exec_arel = exec_arel.where(table_arel[:id].gteq(start_id)) + exec_arel = exec_arel.where(table_arel[:id].lt(stop_id)) if stop_id.nonzero? + exec_arel = scope.call(table_arel, exec_arel) if scope + + execute(exec_arel.to_sql) end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 65af2e1cb52..f4a66b7e2a2 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -262,7 +262,8 @@ describe Gitlab::Database::MigrationHelpers, lib: true do describe '#update_column_in_batches' do context 'when running outside of a transaction' do before do - expect(model).to receive(:transaction_open?).twice.and_return(false) + expect(model).to receive(:transaction_open?) + .at_least(:once).and_return(false) create_list(:empty_project, 5) end @@ -336,10 +337,39 @@ describe Gitlab::Database::MigrationHelpers, lib: true do first_id = Project.first.id scope = ->(table, query) { query.where(table[:id].eq(first_id)) } - model.walk_table_in_batches(:projects, scope: scope) do + expect { |b| model.walk_table_in_batches(:projects, of: 1, scope: scope, &b) } + .to yield_control.exactly(:once) + end + end + end + + context 'when running inside the transaction' do + it 'raises RuntimeError' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect { model.walk_table_in_batches(:projects, of: 2) } + .to raise_error(RuntimeError) + end + end + end + + describe '#execute_in_batches' do + context 'when running outside of a transaction' do + before do + expect(model).to receive(:transaction_open?) + .at_least(:once).and_return(false) + + create_list(:empty_project, 6) + end + + context 'when a scope is provided' do + it 'limits the scope of the statement provided inside the block' do + first_id = Project.first.id + scope = ->(table, query) { query.where(table[:id].eq(first_id)) } + + model.execute_in_batches(:projects, scope: scope) do |table| Arel::UpdateManager.new(ActiveRecord::Base) - .table(Arel::Table.new(:projects)) - .set([[Arel::Table.new(:projects)[:archived], true]]) + .table(table).set([[table[:archived], true]]) end expect(Project.where(archived: true).count).to eq(1) @@ -351,9 +381,8 @@ describe Gitlab::Database::MigrationHelpers, lib: true do it 'raises RuntimeError' do expect(model).to receive(:transaction_open?).and_return(true) - expect do - model.update_column_in_batches(:projects, :star_count, Arel.sql('1+1')) - end.to raise_error(RuntimeError) + expect { model.execute_in_batches(:projects)} + .to raise_error(RuntimeError) end end end -- cgit v1.2.3 From fb89ba24825853ca29b804a4a08f7c83210f09db Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 09:35:28 +0200 Subject: Schedule stage_id background migration in ranges --- ...28080858_migrate_stage_id_reference_in_background.rb | 17 ++++++++--------- .../migrate_build_stage_id_reference.rb | 11 ++++++++--- lib/gitlab/database/migration_helpers.rb | 2 +- .../migrate_stage_id_reference_in_background_spec.rb | 11 +++++------ 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index ebec4cb6bb7..0d108d18501 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -7,19 +7,18 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration disable_ddl_transaction! - class Build < ActiveRecord::Base - self.table_name = 'ci_builds' - end - + ## + # It will take around 3 days to process 20M ci_builds. + # def up - index = 1 + opts = { scope: ->(table, query) { query.where(table[:stage_id].eq(nil)) }, + of: BATCH_SIZE } - Build.where(stage_id: nil).in_batches(of: BATCH_SIZE) do |relation| - jobs = relation.pluck(:id).map { |id| [MIGRATION, [id]] } + walk_table_in_batches(:ci_builds, **opts) do |index, start_id, stop_id| schedule = index * 2.minutes - index += 1 - BackgroundMigrationWorker.perform_bulk_in(schedule, jobs) + BackgroundMigrationWorker + .perform_in(schedule, MIGRATION, [start_id, stop_id]) end end diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index d1d0a968588..0705b26056d 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -1,15 +1,20 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference - def perform(id) + def perform(start_id, stop_id) + scope = if stop_id.nonzero? + "ci_builds.id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}" + else + "ci_builds.id >= #{start_id.to_i}" + end + sql = <<-SQL.strip_heredoc UPDATE "ci_builds" SET "stage_id" = (SELECT id FROM ci_stages WHERE ci_stages.pipeline_id = ci_builds.commit_id AND ci_stages.name = ci_builds.stage) - WHERE "ci_builds"."id" = #{id.to_i} - AND "ci_builds"."stage_id" IS NULL + WHERE #{scope} AND "ci_builds"."stage_id" IS NULL SQL ActiveRecord::Base.connection.execute(sql) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 5acc96331c0..b6883704da0 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -296,7 +296,7 @@ module Gitlab MSG end - # raise 'This method requires a block!' unless block_given? + raise ArgumentError, 'This method requires a block!' unless block_given? table_arel = Arel::Table.new(table) diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index a32a7fceb68..f3dde8b59c0 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -21,7 +21,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do let(:projects) { table(:projects) } before do - stub_const('MigrateStageIdReferenceInBackground::BATCH_SIZE', 2) + stub_const("#{described_class.name}::BATCH_SIZE", 2) projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') projects.create!(id: 345, name: 'gitlab2', path: 'gitlab2') @@ -47,11 +47,10 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1) - expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 2) - expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3) - expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4) - expect(BackgroundMigrationWorker.jobs.size).to eq 5 + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 3) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3, 5) + expect(described_class::MIGRATION).to be_scheduled_migration(6.minutes, 5, 0) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end end -- cgit v1.2.3 From 84265775e56e2203199ff4d841d04b2213e97234 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 10:54:13 +0200 Subject: Add some comments on new migrations helpers --- lib/gitlab/database/migration_helpers.rb | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index b6883704da0..ca7e4c8aa7c 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -255,6 +255,25 @@ module Gitlab end end + ## + # Iterates a table and executes a block for given range. + # + # Yields batch index, start and stop ids. + # + # Optional `scope` keyword argument is a closure that is meant to limit + # the scope the statement is going to be applied onto. + # + # Arel statement this helper will execute must be defined inside the + # block. + # + # Example: + # + # scope = ->(table, query) { query.where(table[:id].gt(100) } + # + # walk_table_in_batches(:table, of: 10, scope: scope) do |index, start, stop| + # # do something here + # end + # def walk_table_in_batches(table, of: 1000, scope: nil) if transaction_open? raise <<-MSG @@ -287,6 +306,25 @@ module Gitlab end end + ## + # Executes an SQL statement in batches, created by Arel manager. + # + # Optional `scope` keyword argument is a closure that is meant to limit + # the scope the statement is going to be applied onto. + # + # Arel statement this helper will execute must be defined inside the + # block. + # + # Example: + # + # scope = ->(table, query) { query.where(table[:id].gt(100) } + # + # execute_in_batches(:table, of: 10000, scope: scope) do |table| + # Arel::UpdateManager.new(ActiveRecord::Base) + # .table(table) + # .set([[table[:field], 101]]) + # end + # def execute_in_batches(table, of: 1000, scope: nil) if transaction_open? raise <<-MSG -- cgit v1.2.3 From af0eeefc324e96d79c85b337ae9e441947a9f729 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 15:05:27 +0200 Subject: Revert recent changes in migration helpers --- lib/gitlab/database/migration_helpers.rb | 120 +++++---------------- spec/lib/gitlab/database/migration_helpers_spec.rb | 82 +------------- 2 files changed, 30 insertions(+), 172 deletions(-) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index ca7e4c8aa7c..0643c56db9b 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -221,19 +221,17 @@ module Gitlab # make things _more_ complex). # # rubocop: disable Metrics/AbcSize - def update_column_in_batches(table, column, value, &scope) + def update_column_in_batches(table, column, value) if transaction_open? - raise <<-MSG - update_column_in_batches helper can not be run inside a transaction. - You can disable transactions by calling `disable_ddl_transaction!` - method in the body of your migration class. - MSG + raise 'update_column_in_batches can not be run inside a transaction, ' \ + 'you can disable transactions by calling disable_ddl_transaction! ' \ + 'in the body of your migration class' end - table_arel = Arel::Table.new(table) + table = Arel::Table.new(table) - count_arel = table_arel.project(Arel.star.count.as('count')) - count_arel = yield table_arel, count_arel if block_given? + count_arel = table.project(Arel.star.count.as('count')) + count_arel = yield table, count_arel if block_given? total = exec_query(count_arel.to_sql).to_hash.first['count'].to_i @@ -248,103 +246,37 @@ module Gitlab # rows for GitLab.com. batch_size = max_size if batch_size > max_size - execute_in_batches(table, of: batch_size, scope: scope) do - Arel::UpdateManager.new(ActiveRecord::Base) - .table(table_arel) - .set([[table_arel[column], value]]) - end - end - - ## - # Iterates a table and executes a block for given range. - # - # Yields batch index, start and stop ids. - # - # Optional `scope` keyword argument is a closure that is meant to limit - # the scope the statement is going to be applied onto. - # - # Arel statement this helper will execute must be defined inside the - # block. - # - # Example: - # - # scope = ->(table, query) { query.where(table[:id].gt(100) } - # - # walk_table_in_batches(:table, of: 10, scope: scope) do |index, start, stop| - # # do something here - # end - # - def walk_table_in_batches(table, of: 1000, scope: nil) - if transaction_open? - raise <<-MSG - walk_table_in_batches helper can not be run inside a transaction. - You can disable transactions by calling `disable_ddl_transaction!` - method in the body of your migration class. - MSG - end - - table = Arel::Table.new(table) - start_arel = table.project(table[:id]).order(table[:id].asc).take(1) - start_arel = scope.call(table, start_arel) if scope - start_id = exec_query(start_arel.to_sql).to_hash.first.to_h['id'].to_i + start_arel = yield table, start_arel if block_given? + start_id = exec_query(start_arel.to_sql).to_hash.first['id'].to_i - 1.step do |batch| + loop do stop_arel = table.project(table[:id]) .where(table[:id].gteq(start_id)) .order(table[:id].asc) .take(1) - .skip(of) - - stop_arel = scope.call(table, stop_arel) if scope - stop_id = exec_query(stop_arel.to_sql) - .to_hash.first.to_h['id'].to_i + .skip(batch_size) - yield batch, start_id, stop_id + stop_arel = yield table, stop_arel if block_given? + stop_row = exec_query(stop_arel.to_sql).to_hash.first - stop_id.zero? ? break : start_id = stop_id - end - end - - ## - # Executes an SQL statement in batches, created by Arel manager. - # - # Optional `scope` keyword argument is a closure that is meant to limit - # the scope the statement is going to be applied onto. - # - # Arel statement this helper will execute must be defined inside the - # block. - # - # Example: - # - # scope = ->(table, query) { query.where(table[:id].gt(100) } - # - # execute_in_batches(:table, of: 10000, scope: scope) do |table| - # Arel::UpdateManager.new(ActiveRecord::Base) - # .table(table) - # .set([[table[:field], 101]]) - # end - # - def execute_in_batches(table, of: 1000, scope: nil) - if transaction_open? - raise <<-MSG - execute_in_batches helper can not be run inside a transaction. - You can disable transactions by calling `disable_ddl_transaction!` - method in the body of your migration class. - MSG - end + update_arel = Arel::UpdateManager.new(ActiveRecord::Base) + .table(table) + .set([[table[column], value]]) + .where(table[:id].gteq(start_id)) - raise ArgumentError, 'This method requires a block!' unless block_given? + if stop_row + stop_id = stop_row['id'].to_i + start_id = stop_id + update_arel = update_arel.where(table[:id].lt(stop_id)) + end - table_arel = Arel::Table.new(table) + update_arel = yield table, update_arel if block_given? - walk_table_in_batches(table, of: of, scope: scope) do |_batch, start_id, stop_id| - exec_arel = yield table_arel - exec_arel = exec_arel.where(table_arel[:id].gteq(start_id)) - exec_arel = exec_arel.where(table_arel[:id].lt(stop_id)) if stop_id.nonzero? - exec_arel = scope.call(table_arel, exec_arel) if scope + execute(update_arel.to_sql) - execute(exec_arel.to_sql) + # There are no more rows left to update. + break unless stop_row end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index f4a66b7e2a2..4259be3f522 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' describe Gitlab::Database::MigrationHelpers, lib: true do let(:model) do - ActiveRecord::Migration.new.extend(described_class) + ActiveRecord::Migration.new.extend( + Gitlab::Database::MigrationHelpers + ) end before do @@ -262,8 +264,7 @@ describe Gitlab::Database::MigrationHelpers, lib: true do describe '#update_column_in_batches' do context 'when running outside of a transaction' do before do - expect(model).to receive(:transaction_open?) - .at_least(:once).and_return(false) + expect(model).to receive(:transaction_open?).and_return(false) create_list(:empty_project, 5) end @@ -312,81 +313,6 @@ describe Gitlab::Database::MigrationHelpers, lib: true do end end - describe '#walk_table_in_batches' do - context 'when running outside of a transaction' do - before do - expect(model).to receive(:transaction_open?).and_return(false) - - create_list(:empty_project, 6) - end - - it 'yields for each batch' do - expect { |b| model.walk_table_in_batches(:projects, of: 2, &b) } - .to yield_control.exactly(3).times - end - - it 'yields successive ranges' do - expect { |b| model.walk_table_in_batches(:projects, of: 2, &b) } - .to yield_successive_args([1, Integer, Integer], - [2, Integer, Integer], - [3, Integer, 0]) - end - - context 'when a scope is provided' do - it 'limits the scope of the statement provided inside the block' do - first_id = Project.first.id - scope = ->(table, query) { query.where(table[:id].eq(first_id)) } - - expect { |b| model.walk_table_in_batches(:projects, of: 1, scope: scope, &b) } - .to yield_control.exactly(:once) - end - end - end - - context 'when running inside the transaction' do - it 'raises RuntimeError' do - expect(model).to receive(:transaction_open?).and_return(true) - - expect { model.walk_table_in_batches(:projects, of: 2) } - .to raise_error(RuntimeError) - end - end - end - - describe '#execute_in_batches' do - context 'when running outside of a transaction' do - before do - expect(model).to receive(:transaction_open?) - .at_least(:once).and_return(false) - - create_list(:empty_project, 6) - end - - context 'when a scope is provided' do - it 'limits the scope of the statement provided inside the block' do - first_id = Project.first.id - scope = ->(table, query) { query.where(table[:id].eq(first_id)) } - - model.execute_in_batches(:projects, scope: scope) do |table| - Arel::UpdateManager.new(ActiveRecord::Base) - .table(table).set([[table[:archived], true]]) - end - - expect(Project.where(archived: true).count).to eq(1) - end - end - end - - context 'when running inside the transaction' do - it 'raises RuntimeError' do - expect(model).to receive(:transaction_open?).and_return(true) - - expect { model.execute_in_batches(:projects)} - .to raise_error(RuntimeError) - end - end - end - describe '#add_column_with_default' do context 'outside of a transaction' do context 'when a column limit is not set' do -- cgit v1.2.3 From b41b4d2e6a44a04fc3e6fff09cf45f93033ff0ad Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 15:19:43 +0200 Subject: Use new `each_batch` helper instead of custom one In stage_id backgrond migration. --- app/models/concerns/each_batch.rb | 4 ++-- ...70628080858_migrate_stage_id_reference_in_background.rb | 14 ++++++++------ .../migrate_stage_id_reference_in_background_spec.rb | 6 +++--- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/models/concerns/each_batch.rb b/app/models/concerns/each_batch.rb index 6610e7967d1..53979c4c683 100644 --- a/app/models/concerns/each_batch.rb +++ b/app/models/concerns/each_batch.rb @@ -35,7 +35,7 @@ module EachBatch start_id = start[primary_key] arel_table = self.arel_table - loop do + 1.step do |index| stop = except(:select) .select(primary_key) .where(arel_table[primary_key].gteq(start_id)) @@ -54,7 +54,7 @@ module EachBatch # Any ORDER BYs are useless for this relation and can lead to less # efficient UPDATE queries, hence we get rid of it. - yield relation.except(:order) + yield relation.except(:order), index break unless stop end diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 0d108d18501..107a5661329 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -7,18 +7,20 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration disable_ddl_transaction! + class Build < ActiveRecord::Base + self.table_name = 'ci_builds' + include ::EachBatch + end + ## # It will take around 3 days to process 20M ci_builds. # def up - opts = { scope: ->(table, query) { query.where(table[:stage_id].eq(nil)) }, - of: BATCH_SIZE } - - walk_table_in_batches(:ci_builds, **opts) do |index, start_id, stop_id| + Build.all.each_batch(of: BATCH_SIZE) do |relation, index| + range = relation.pluck('MIN(id)', 'MAX(id)').first schedule = index * 2.minutes - BackgroundMigrationWorker - .perform_in(schedule, MIGRATION, [start_id, stop_id]) + BackgroundMigrationWorker.perform_in(schedule, MIGRATION, range) end end diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index f3dde8b59c0..90ad5c39089 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -47,9 +47,9 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 3) - expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3, 5) - expect(described_class::MIGRATION).to be_scheduled_migration(6.minutes, 5, 0) + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3, 4) + expect(described_class::MIGRATION).to be_scheduled_migration(6.minutes, 5, 6) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end -- cgit v1.2.3 From c467451ea6f39f498b458e11b5f8a74c53d3541d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 15:44:47 +0200 Subject: Schedule stage_id bg migrations in batches of 10 --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 9 ++++++--- .../background_migration/migrate_build_stage_id_reference.rb | 2 +- .../migrate_stage_id_reference_in_background_spec.rb | 10 ++++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 107a5661329..5b1ff9b8849 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -3,6 +3,7 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration DOWNTIME = false BATCH_SIZE = 10000 + RANGE_SIZE = 1000 MIGRATION = 'MigrateBuildStageIdReference'.freeze disable_ddl_transaction! @@ -17,10 +18,12 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration # def up Build.all.each_batch(of: BATCH_SIZE) do |relation, index| - range = relation.pluck('MIN(id)', 'MAX(id)').first - schedule = index * 2.minutes + relation.each_batch(of: RANGE_SIZE) do |relation| + range = relation.pluck('MIN(id)', 'MAX(id)').first - BackgroundMigrationWorker.perform_in(schedule, MIGRATION, range) + BackgroundMigrationWorker + .perform_in(index * 2.minutes, MIGRATION, range) + end end end diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index 0705b26056d..dcd5ecdf322 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -2,7 +2,7 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference def perform(start_id, stop_id) - scope = if stop_id.nonzero? + scope = if stop_id.to_i.nonzero? "ci_builds.id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}" else "ci_builds.id >= #{start_id.to_i}" diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 90ad5c39089..e829a9238f6 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -21,7 +21,8 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do let(:projects) { table(:projects) } before do - stub_const("#{described_class.name}::BATCH_SIZE", 2) + stub_const("#{described_class.name}::BATCH_SIZE", 3) + stub_const("#{described_class.name}::RANGE_SIZE", 2) projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') projects.create!(id: 345, name: 'gitlab2', path: 'gitlab2') @@ -48,9 +49,10 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do migrate! expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 2) - expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 3, 4) - expect(described_class::MIGRATION).to be_scheduled_migration(6.minutes, 5, 6) - expect(BackgroundMigrationWorker.jobs.size).to eq 3 + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 3, 3) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4, 5) + expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 6, 6) + expect(BackgroundMigrationWorker.jobs.size).to eq 4 end end end -- cgit v1.2.3 From 320229e12aea3c5f52bcf0fb413bb35138ef7c25 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 15:50:33 +0200 Subject: Do not schedule bg migration when it is not needed --- .../20170628080858_migrate_stage_id_reference_in_background.rb | 2 +- spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb index 5b1ff9b8849..f31015d77a3 100644 --- a/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb +++ b/db/post_migrate/20170628080858_migrate_stage_id_reference_in_background.rb @@ -17,7 +17,7 @@ class MigrateStageIdReferenceInBackground < ActiveRecord::Migration # It will take around 3 days to process 20M ci_builds. # def up - Build.all.each_batch(of: BATCH_SIZE) do |relation, index| + Build.where(stage_id: nil).each_batch(of: BATCH_SIZE) do |relation, index| relation.each_batch(of: RANGE_SIZE) do |relation| range = relation.pluck('MIN(id)', 'MAX(id)').first diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index e829a9238f6..260378adaa7 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb @@ -51,8 +51,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 2) expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 3, 3) expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4, 5) - expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 6, 6) - expect(BackgroundMigrationWorker.jobs.size).to eq 4 + expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end end -- cgit v1.2.3 From 2719b2f0a1ed03170abeca7e78d99a36afb0b65d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 7 Jul 2017 15:52:45 +0200 Subject: Simplify stage_id migration as we now use relations --- .../migrate_build_stage_id_reference.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb index dcd5ecdf322..91540127ea9 100644 --- a/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb +++ b/lib/gitlab/background_migration/migrate_build_stage_id_reference.rb @@ -2,19 +2,14 @@ module Gitlab module BackgroundMigration class MigrateBuildStageIdReference def perform(start_id, stop_id) - scope = if stop_id.to_i.nonzero? - "ci_builds.id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}" - else - "ci_builds.id >= #{start_id.to_i}" - end - sql = <<-SQL.strip_heredoc - UPDATE "ci_builds" - SET "stage_id" = + UPDATE ci_builds + SET stage_id = (SELECT id FROM ci_stages WHERE ci_stages.pipeline_id = ci_builds.commit_id AND ci_stages.name = ci_builds.stage) - WHERE #{scope} AND "ci_builds"."stage_id" IS NULL + WHERE ci_builds.id BETWEEN #{start_id.to_i} AND #{stop_id.to_i} + AND ci_builds.stage_id IS NULL SQL ActiveRecord::Base.connection.execute(sql) -- cgit v1.2.3