From 38c2e480bfa180241e94e77c049b1f5256d83bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Wed, 6 Jun 2018 16:45:42 -0400 Subject: shave off 30% of the query count --- .../object_storage/migrate_uploads_worker_spec.rb | 50 ++++++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index aed62f97448..18da0c6d39a 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -1,5 +1,7 @@ require 'spec_helper' +MIGRATION_QUERIES = 7 + describe ObjectStorage::MigrateUploadsWorker, :sidekiq do shared_context 'sanity_check! fails' do before do @@ -11,6 +13,13 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do let(:uploads) { Upload.all } let(:to_store) { ObjectStorage::Store::REMOTE } + def perform(uploads) + binding.pry + described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store) + rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures + # swallow + end + shared_examples "uploads migration worker" do describe '.enqueue!' do def enqueue! @@ -69,12 +78,6 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do end describe '#perform' do - def perform - described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store) - rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures - # swallow - end - shared_examples 'outputs correctly' do |success: 0, failures: 0| total = success + failures @@ -82,7 +85,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do it 'outputs the reports' do expect(Rails.logger).to receive(:info).with(%r{Migrated #{success}/#{total} files}) - perform + perform(uploads) end end @@ -90,7 +93,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do it 'outputs upload failures' do expect(Rails.logger).to receive(:warn).with(/Error .* I am a teapot/) - perform + perform(uploads) end end end @@ -98,7 +101,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do it_behaves_like 'outputs correctly', success: 10 it 'migrates files' do - perform + perform(uploads) expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) end @@ -123,6 +126,18 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do end it_behaves_like "uploads migration worker" + + describe "limits N+1 queries" do + let!(:projects) { create_list(:project, 10, :with_avatar) } + + it do + query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } + + more_projects = create_list(:project, 100, :with_avatar) + expected_queries_per_migration = MIGRATION_QUERIES * more_projects.count + expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(expected_queries_per_migration) + end + end end context "for FileUploader" do @@ -140,5 +155,22 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do end it_behaves_like "uploads migration worker" + + describe "limits N+1 queries" do + let!(:projects) { create_list(:project, 10) } + + it do + query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } + + more_projects = create_list(:project, 100) + more_projects.map do |project| + uploader = FileUploader.new(project) + uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end + expected_queries_per_migration = MIGRATION_QUERIES * more_projects.count + + expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(expected_queries_per_migration) + end + end end end -- cgit v1.2.3 From 44975f8a5ad9c40c615f47f683fb46c94aa0e130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Thu, 7 Jun 2018 10:01:47 -0400 Subject: shave off another 20% query --- spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index 18da0c6d39a..ba01cfe53c5 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -MIGRATION_QUERIES = 7 +MIGRATION_QUERIES = 5 describe ObjectStorage::MigrateUploadsWorker, :sidekiq do shared_context 'sanity_check! fails' do @@ -14,7 +14,6 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do let(:to_store) { ObjectStorage::Store::REMOTE } def perform(uploads) - binding.pry described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store) rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures # swallow -- cgit v1.2.3 From 50872bcc242a582c7e3af25df4d32e1c3e0a28f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Thu, 7 Jun 2018 11:06:04 -0400 Subject: fix the failing spec --- spec/uploaders/object_storage_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 4503288e410..03386bf764f 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -321,7 +321,7 @@ describe ObjectStorage do when_file_is_in_use do expect(uploader).not_to receive(:unsafe_migrate!) - expect { uploader.migrate!(described_class::Store::REMOTE) }.to raise_error('exclusive lease already taken') + expect { uploader.migrate!(described_class::Store::REMOTE) }.to raise_error(ObjectStorage::ExclusiveLeaseTaken) end end @@ -329,7 +329,7 @@ describe ObjectStorage do when_file_is_in_use do expect(uploader).not_to receive(:unsafe_use_file) - expect { uploader.use_file }.to raise_error('exclusive lease already taken') + expect { uploader.use_file }.to raise_error(ObjectStorage::ExclusiveLeaseTaken) end end end -- cgit v1.2.3 From e1589a5c584acae83d97d41494616be1f3981da7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Fri, 8 Jun 2018 10:51:59 -0400 Subject: apply feedback --- spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index ba01cfe53c5..31d323626c5 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -129,7 +129,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do describe "limits N+1 queries" do let!(:projects) { create_list(:project, 10, :with_avatar) } - it do + it "to N*#{MIGRATION_QUERIES}" do query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } more_projects = create_list(:project, 100, :with_avatar) @@ -158,7 +158,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do describe "limits N+1 queries" do let!(:projects) { create_list(:project, 10) } - it do + it "to N*#{MIGRATION_QUERIES}" do query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } more_projects = create_list(:project, 100) -- cgit v1.2.3 From 3d42bab71ad293c99d029dfb4f0c9aa0378643d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mica=C3=ABl=20Bergeron?= Date: Tue, 12 Jun 2018 11:22:35 -0400 Subject: apply feedback --- .../object_storage/migrate_uploads_worker_spec.rb | 34 +++++++++------------- 1 file changed, 14 insertions(+), 20 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index 31d323626c5..da490cb02af 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -1,7 +1,5 @@ require 'spec_helper' -MIGRATION_QUERIES = 5 - describe ObjectStorage::MigrateUploadsWorker, :sidekiq do shared_context 'sanity_check! fails' do before do @@ -127,13 +125,12 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do it_behaves_like "uploads migration worker" describe "limits N+1 queries" do - let!(:projects) { create_list(:project, 10, :with_avatar) } - - it "to N*#{MIGRATION_QUERIES}" do + it "to N*5" do query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } - more_projects = create_list(:project, 100, :with_avatar) - expected_queries_per_migration = MIGRATION_QUERIES * more_projects.count + more_projects = create_list(:project, 3, :with_avatar) + + expected_queries_per_migration = 5 * more_projects.count expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(expected_queries_per_migration) end end @@ -144,30 +141,27 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do let(:secret) { SecureRandom.hex } let(:mounted_as) { nil } + def upload_file(project) + uploader = FileUploader.new(project) + uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end + before do stub_uploads_object_storage(FileUploader) - projects.map do |project| - uploader = FileUploader.new(project) - uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end + projects.map(&method(:upload_file)) end it_behaves_like "uploads migration worker" describe "limits N+1 queries" do - let!(:projects) { create_list(:project, 10) } - - it "to N*#{MIGRATION_QUERIES}" do + it "to N*5" do query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } - more_projects = create_list(:project, 100) - more_projects.map do |project| - uploader = FileUploader.new(project) - uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - expected_queries_per_migration = MIGRATION_QUERIES * more_projects.count + more_projects = create_list(:project, 3) + more_projects.map(&method(:upload_file)) + expected_queries_per_migration = 5 * more_projects.count expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(expected_queries_per_migration) end end -- cgit v1.2.3