Merge branch 'tc-index-lfs-objects-file-store' into 'master'

Enhance performance of counting local LFS objects

Closes gitlab-ee#6067

See merge request gitlab-org/gitlab-ce!22143

app/models/lfs_object.rb

@@ -7,7 +7,7 @@ class LfsObject < ActiveRecord::Base
   has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :projects, through: :lfs_objects_projects
 
-  scope :with_files_stored_locally, -> { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
+  scope :with_files_stored_locally, -> { where(file_store: LfsObjectUploader::Store::LOCAL) }
 
   validates :oid, presence: true, uniqueness: true
 
@@ -26,7 +26,7 @@ class LfsObject < ActiveRecord::Base
   end
 
   def local_store?
-    [nil, LfsObjectUploader::Store::LOCAL].include?(self.file_store)
+    file_store == LfsObjectUploader::Store::LOCAL
   end
 
   # rubocop: disable DestroyAll

changelogs/unreleased/tc-index-lfs-objects-file-store.yml

+---
+title: Enhance performance of counting local LFS objects
+merge_request: 22143
+author:
+type: performance

db/migrate/20181005110927_add_index_to_lfs_objects_file_store.rb

+# frozen_string_literal: true
+
+class AddIndexToLfsObjectsFileStore < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :lfs_objects, :file_store
+  end
+
+  def down
+    remove_concurrent_index :lfs_objects, :file_store
+  end
+end

db/schema.rb

@@ -1167,6 +1167,7 @@ ActiveRecord::Schema.define(version: 20181017001059) do
     t.integer "file_store"
   end
 
+  add_index "lfs_objects", ["file_store"], name: "index_lfs_objects_on_file_store", using: :btree
   add_index "lfs_objects", ["oid"], name: "index_lfs_objects_on_oid", unique: true, using: :btree
 
   create_table "lfs_objects_projects", force: :cascade do |t|

spec/models/lfs_object_spec.rb

@@ -2,12 +2,6 @@ require 'spec_helper'
 
 describe LfsObject do
   describe '#local_store?' do
-    it 'returns true when file_store is nil' do
-      subject.file_store = nil
-
-      expect(subject.local_store?).to eq true
-    end
-
     it 'returns true when file_store is equal to LfsObjectUploader::Store::LOCAL' do
       subject.file_store = LfsObjectUploader::Store::LOCAL
 
@@ -83,19 +77,6 @@ describe LfsObject do
     describe 'file is being stored' do
       let(:lfs_object) { create(:lfs_object, :with_file) }
 
-      context 'when object has nil store' do
-        before do
-          lfs_object.update_column(:file_store, nil)
-          lfs_object.reload
-        end
-
-        it 'is stored locally' do
-          expect(lfs_object.file_store).to be(nil)
-          expect(lfs_object.file).to be_file_storage
-          expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::LOCAL)
-        end
-      end
-
       context 'when existing object has local store' do
         it 'is stored locally' do
           expect(lfs_object.file_store).to be(ObjectStorage::Store::LOCAL)