Commit d5f290e4 authored by Toon Claes's avatar Toon Claes

Enhance performance of counting local LFS objects

Add an index to the `file_store` column on `lfs_objects`. This makes
counting local objects faster.

Also, there is no longer need to check for objects with `file_store`
being `NULL`. See
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18557

---

### Query plans

#### Before & with `NULL`

```
Aggregate  (cost=113495.96..113495.97 rows=1 width=8) (actual time=1691.394..1691.394 rows=1 loops=1)
  ->  Seq Scan on lfs_objects  (cost=0.00..106415.50 rows=2832186 width=0) (actual time=0.012..1312.488 rows=2852607 loops=1)
        Filter: ((file_store = 1) OR (file_store IS NULL))
        Rows Removed by Filter: 131
Planning time: 0.077 ms
Execution time: 1691.433 ms
```

#### Before, without `NULL`

```
Aggregate  (cost=113495.96..113495.97 rows=1 width=8) (actual time=856.423..856.424 rows=1 loops=1)
  ->  Seq Scan on lfs_objects  (cost=0.00..106415.50 rows=2832186 width=0) (actual time=0.012..672.181 rows=2852607 loops=1)
        Filter: (file_store = 1)
        Rows Removed by Filter: 131
Planning time: 0.128 ms
Execution time: 856.470 ms
```

#### After & with `NULL`

```
Aggregate  (cost=68819.95..68819.96 rows=1 width=8) (actual time=583.355..583.355 rows=1 loops=1)
  ->  Index Only Scan using index_lfs_objects_on_file_store on lfs_objects  (cost=0.43..61688.35 rows=2852643 width=0) (actual time=0.028..399.177 rows=2852607 loops=1)
        Filter: ((file_store = 1) OR (file_store IS NULL))
        Rows Removed by Filter: 131
        Heap Fetches: 867
Planning time: 0.096 ms
Execution time: 583.404 ms
```

#### After, without `NULL`

```
Aggregate  (cost=68817.29..68817.30 rows=1 width=8) (actual time=490.550..490.551 rows=1 loops=1)
  ->  Index Only Scan using index_lfs_objects_on_file_store on lfs_objects  (cost=0.43..61685.68 rows=2852643 width=0) (actual time=0.040..311.760 rows=2852607 loops=1)
        Index Cond: (file_store = 1)
        Heap Fetches: 831
Planning time: 0.294 ms
Execution time: 490.590 ms
```

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/6067
parent d0b746d8
......@@ -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
......
---
title: Enhance performance of counting local LFS objects
merge_request: 22143
author:
type: performance
# 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
......@@ -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|
......
......@@ -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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment