diff options
-rw-r--r-- | app/models/upload.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/tc-index-uploads-file-store.yml | 5 | ||||
-rw-r--r-- | db/migrate/20181005125926_add_index_to_uploads_store.rb | 17 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/models/upload_spec.rb | 17 | ||||
-rw-r--r-- | spec/tasks/gitlab/uploads/migrate_rake_spec.rb | 8 |
6 files changed, 34 insertions, 18 deletions
diff --git a/app/models/upload.rb b/app/models/upload.rb index 23bc9ca42fc..a6af7a8b869 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -11,7 +11,7 @@ class Upload < ActiveRecord::Base validates :model, presence: true validates :uploader, presence: true - scope :with_files_stored_locally, -> { where(store: [nil, ObjectStorage::Store::LOCAL]) } + scope :with_files_stored_locally, -> { where(store: ObjectStorage::Store::LOCAL) } before_save :calculate_checksum!, if: :foreground_checksummable? after_commit :schedule_checksum, if: :checksummable? @@ -57,8 +57,6 @@ class Upload < ActiveRecord::Base end def local? - return true if store.nil? - store == ObjectStorage::Store::LOCAL end diff --git a/changelogs/unreleased/tc-index-uploads-file-store.yml b/changelogs/unreleased/tc-index-uploads-file-store.yml new file mode 100644 index 00000000000..fa3b3164e38 --- /dev/null +++ b/changelogs/unreleased/tc-index-uploads-file-store.yml @@ -0,0 +1,5 @@ +--- +title: Enhance performance of counting local Uploads +merge_request: 22522 +author: +type: performance diff --git a/db/migrate/20181005125926_add_index_to_uploads_store.rb b/db/migrate/20181005125926_add_index_to_uploads_store.rb new file mode 100644 index 00000000000..d32ca05e980 --- /dev/null +++ b/db/migrate/20181005125926_add_index_to_uploads_store.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToUploadsStore < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :uploads, :store + end + + def down + remove_concurrent_index :uploads, :store + end +end diff --git a/db/schema.rb b/db/schema.rb index cfbfd7ad375..7509941325f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2155,6 +2155,7 @@ ActiveRecord::Schema.define(version: 20181107054254) do add_index "uploads", ["checksum"], name: "index_uploads_on_checksum", using: :btree add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree + add_index "uploads", ["store"], name: "index_uploads_on_store", using: :btree add_index "uploads", ["uploader", "path"], name: "index_uploads_on_uploader_and_path", using: :btree create_table "user_agent_details", force: :cascade do |t| diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 3c89e99abf0..e3544b84b50 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -21,7 +21,8 @@ describe Upload do path: __FILE__, size: described_class::CHECKSUM_THRESHOLD + 1.kilobyte, model: build_stubbed(:user), - uploader: double('ExampleUploader') + uploader: double('ExampleUploader'), + store: ObjectStorage::Store::LOCAL ) expect(UploadChecksumWorker) @@ -35,7 +36,8 @@ describe Upload do path: __FILE__, size: described_class::CHECKSUM_THRESHOLD, model: build_stubbed(:user), - uploader: double('ExampleUploader') + uploader: double('ExampleUploader'), + store: ObjectStorage::Store::LOCAL ) expect { upload.save } @@ -60,7 +62,7 @@ describe Upload do describe '#absolute_path' do it 'returns the path directly when already absolute' do path = '/path/to/namespace/project/secret/file.jpg' - upload = described_class.new(path: path) + upload = described_class.new(path: path, store: ObjectStorage::Store::LOCAL) expect(upload).not_to receive(:uploader_class) @@ -69,7 +71,7 @@ describe Upload do it "delegates to the uploader's absolute_path method" do uploader = spy('FakeUploader') - upload = described_class.new(path: 'secret/file.jpg') + upload = described_class.new(path: 'secret/file.jpg', store: ObjectStorage::Store::LOCAL) expect(upload).to receive(:uploader_class).and_return(uploader) upload.absolute_path @@ -81,7 +83,8 @@ describe Upload do describe '#calculate_checksum!' do let(:upload) do described_class.new(path: __FILE__, - size: described_class::CHECKSUM_THRESHOLD - 1.megabyte) + size: described_class::CHECKSUM_THRESHOLD - 1.megabyte, + store: ObjectStorage::Store::LOCAL) end it 'sets `checksum` to SHA256 sum of the file' do @@ -104,13 +107,13 @@ describe Upload do describe '#exist?' do it 'returns true when the file exists' do - upload = described_class.new(path: __FILE__) + upload = described_class.new(path: __FILE__, store: ObjectStorage::Store::LOCAL) expect(upload).to exist end it 'returns false when the file does not exist' do - upload = described_class.new(path: "#{__FILE__}-nope") + upload = described_class.new(path: "#{__FILE__}-nope", store: ObjectStorage::Store::LOCAL) expect(upload).not_to exist end diff --git a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb index 6fcfae358ec..9588e8be5dc 100644 --- a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb +++ b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb @@ -38,14 +38,6 @@ describe 'gitlab:uploads:migrate rake tasks' do let!(:projects) { create_list(:project, 10, :with_avatar) } it_behaves_like 'enqueue jobs in batch', batch: 4 - - context 'Upload has store = nil' do - before do - Upload.where(model: projects).update_all(store: nil) - end - - it_behaves_like 'enqueue jobs in batch', batch: 4 - end end context "for Group" do |