From 0472147942162cca8e9d130c3ba5b6fe4cf4556b Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 10:08:55 -0800 Subject: Fix last batch size equals max batch size error --- .../prepare_untracked_uploads.rb | 2 +- .../prepare_untracked_uploads_spec.rb | 24 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 476c46341ae..71d6e2e435d 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -88,7 +88,7 @@ module Gitlab end end - yield(paths) + yield(paths) if paths.any? end def build_find_command(search_dir) diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 8bb9ebe0419..2d0a63dbd62 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -128,6 +128,18 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do expect(untracked_files_for_uploads.count).to eq(5) end end + + context 'when the last batch size exactly matches the max batch size' do + it 'does not raise error' do + stub_const("#{described_class}::FIND_BATCH_SIZE", 5) + + expect do + described_class.new.perform + end.not_to raise_error + + expect(untracked_files_for_uploads.count).to eq(5) + end + end end end @@ -216,6 +228,18 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do expect(untracked_files_for_uploads.count).to eq(5) end end + + context 'when the last batch size exactly matches the max batch size' do + it 'does not raise error' do + stub_const("#{described_class}::FIND_BATCH_SIZE", 5) + + expect do + described_class.new.perform + end.not_to raise_error + + expect(untracked_files_for_uploads.count).to eq(5) + end + end end end -- cgit v1.2.1 From 5199dcd7a15ba8483438f6784191370edcae1ebb Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 10:17:00 -0800 Subject: Fix orphan temp table untracked_files_for_uploads --- .../prepare_untracked_uploads.rb | 11 ++- .../prepare_untracked_uploads_spec.rb | 87 +++++++++------------- 2 files changed, 44 insertions(+), 54 deletions(-) diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 71d6e2e435d..9fc46f57319 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -39,7 +39,11 @@ module Gitlab store_untracked_file_paths - schedule_populate_untracked_uploads_jobs + if UntrackedFile.all.empty? + drop_temp_table + else + schedule_populate_untracked_uploads_jobs + end end private @@ -158,6 +162,11 @@ module Gitlab bulk_queue_background_migration_jobs_by_range( UntrackedFile, FOLLOW_UP_MIGRATION) end + + def drop_temp_table + UntrackedFile.connection.drop_table(:untracked_files_for_uploads, + if_exists: true) + end end end end diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 2d0a63dbd62..b1f413c99c2 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -8,8 +8,6 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do before do DatabaseCleaner.clean - - drop_temp_table_if_exists end after do @@ -23,25 +21,25 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end - it 'ensures the untracked_files_for_uploads table exists' do - expect do - described_class.new.perform - end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true) - end + # E.g. The installation is in use at the time of migration, and someone has + # just uploaded a file + shared_examples 'does not add files in /uploads/tmp' do + let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - it 'has a path field long enough for really long paths' do - described_class.new.perform + before do + FileUtils.mkdir(File.dirname(tmp_file)) + FileUtils.touch(tmp_file) + end - component = 'a' * 255 + after do + FileUtils.rm(tmp_file) + end - long_path = [ - 'uploads', - component, # project.full_path - component # filename - ].flatten.join('/') + it 'does not add files from /uploads/tmp' do + described_class.new.perform - record = untracked_files_for_uploads.create!(path: long_path) - expect(record.reload.path.size).to eq(519) + expect(untracked_files_for_uploads.count).to eq(5) + end end context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE" do @@ -69,6 +67,21 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do UploadService.new(project2, uploaded_file, FileUploader).execute end + it 'has a path field long enough for really long paths' do + described_class.new.perform + + component = 'a' * 255 + + long_path = [ + 'uploads', + component, # project.full_path + component # filename + ].flatten.join('/') + + record = untracked_files_for_uploads.create!(path: long_path) + expect(record.reload.path.size).to eq(519) + end + it 'adds unhashed files to the untracked_files_for_uploads table' do described_class.new.perform @@ -109,24 +122,8 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end - # E.g. The installation is in use at the time of migration, and someone has - # just uploaded a file context 'when there are files in /uploads/tmp' do - let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - - before do - FileUtils.touch(tmp_file) - end - - after do - FileUtils.rm(tmp_file) - end - - it 'does not add files from /uploads/tmp' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end + it_behaves_like 'does not add files in /uploads/tmp' end context 'when the last batch size exactly matches the max batch size' do @@ -209,24 +206,8 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end - # E.g. The installation is in use at the time of migration, and someone has - # just uploaded a file context 'when there are files in /uploads/tmp' do - let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - - before do - FileUtils.touch(tmp_file) - end - - after do - FileUtils.rm(tmp_file) - end - - it 'does not add files from /uploads/tmp' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end + it_behaves_like 'does not add files in /uploads/tmp' end context 'when the last batch size exactly matches the max batch size' do @@ -246,10 +227,10 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do # Very new or lightly-used installations that are running this migration # may not have an upload directory because they have no uploads. context 'when no files were ever uploaded' do - it 'does not add to the untracked_files_for_uploads table (and does not raise error)' do + it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do described_class.new.perform - expect(untracked_files_for_uploads.count).to eq(0) + expect(untracked_files_for_uploads.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey end end end -- cgit v1.2.1 From f44e93e2de5b2b57602aeee21a6cb837d3938905 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 11:10:57 -0800 Subject: Schedule PopulateUntrackedUploads if needed To finish migrating untracked files to uploads for installations that were affected by https://gitlab.com/gitlab-org/gitlab-ce/issues/42881 Or just to delete the temp table if it is empty and left behind. --- ...chedule_populate_untracked_uploads_if_needed.rb | 47 ++++++++++++++++++++++ db/schema.rb | 2 +- 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180208183958_schedule_populate_untracked_uploads_if_needed.rb diff --git a/db/migrate/20180208183958_schedule_populate_untracked_uploads_if_needed.rb b/db/migrate/20180208183958_schedule_populate_untracked_uploads_if_needed.rb new file mode 100644 index 00000000000..e46e793d9d2 --- /dev/null +++ b/db/migrate/20180208183958_schedule_populate_untracked_uploads_if_needed.rb @@ -0,0 +1,47 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class SchedulePopulateUntrackedUploadsIfNeeded < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + FOLLOW_UP_MIGRATION = 'PopulateUntrackedUploads'.freeze + + class UntrackedFile < ActiveRecord::Base + include EachBatch + + self.table_name = 'untracked_files_for_uploads' + end + + def up + if table_exists?(:untracked_files_for_uploads) + process_or_remove_table + end + end + + def down + # nothing + end + + private + + def process_or_remove_table + if UntrackedFile.all.empty? + drop_temp_table + else + schedule_populate_untracked_uploads_jobs + end + end + + def drop_temp_table + drop_table(:untracked_files_for_uploads, if_exists: true) + end + + def schedule_populate_untracked_uploads_jobs + say "Scheduling #{FOLLOW_UP_MIGRATION} background migration jobs since there are rows in untracked_files_for_uploads." + + bulk_queue_background_migration_jobs_by_range( + UntrackedFile, FOLLOW_UP_MIGRATION) + end +end diff --git a/db/schema.rb b/db/schema.rb index f0d94974f41..ac59992fba1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180202111106) do +ActiveRecord::Schema.define(version: 20180208183958) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- cgit v1.2.1 From 0a9bed3d5e3dc2ab7395486f1aa8968ff45e6cb9 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 11:17:29 -0800 Subject: Add changelog entry --- changelogs/unreleased/mk-fix-no-untracked-upload-files-error.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/mk-fix-no-untracked-upload-files-error.yml diff --git a/changelogs/unreleased/mk-fix-no-untracked-upload-files-error.yml b/changelogs/unreleased/mk-fix-no-untracked-upload-files-error.yml new file mode 100644 index 00000000000..fddfba94192 --- /dev/null +++ b/changelogs/unreleased/mk-fix-no-untracked-upload-files-error.yml @@ -0,0 +1,5 @@ +--- +title: Resolve PrepareUntrackedUploads PostgreSQL syntax error +merge_request: 17019 +author: +type: fixed -- cgit v1.2.1 From 0e109940bcbf563cd0bc2084f4a83241e69e2545 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Wed, 14 Feb 2018 13:19:52 -0800 Subject: Fix dir exists spec error --- spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index b1f413c99c2..b21197144a9 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -27,7 +27,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } before do - FileUtils.mkdir(File.dirname(tmp_file)) + FileUtils.mkdir_p(File.dirname(tmp_file)) FileUtils.touch(tmp_file) end -- cgit v1.2.1