Lfs pruning improvements and review changes

Adds indices for LFS pruning
LFS pruning removes LfsObjects
LFS pruning only uses recent ref pushes in "rev-list --not"
Duplicate LFS pointer removals don't delete the object
Renamed ReferenceChange to RecentLfsPush
UpdateLfsPointersWorker scans all objects on first run
LFS cleanup is skipped until a project has 5MB of LFS files
LfsCleanupService uses delete_all and updates storage statistics just once
Created LfsObjectsProject.without_pointers scope
Split RecentLfsPushes into UnprocessedLfsPushes and ProcessedLfsRefs
Removed newrev from ProcessedLfsRef
ProcessedLfsRefs sorted by updated_at
Lfs pruning review changes

21 files changed, 261 insertions, 107 deletions

diff --git a/app/models/lfs_objects_project.rb b/app/models/lfs_objects_project.rb
index b0625c52b62..12af8660d24 100644
--- a/app/models/lfs_objects_project.rb
+++ b/app/models/lfs_objects_project.rb
@@ -8,6 +8,12 @@ class LfsObjectsProject < ActiveRecord::Base
 
   after_commit :update_project_statistics, on: [:create, :destroy]
 
+  def self.without_pointers
+    self.joins(:lfs_object)
+        .joins("LEFT JOIN lfs_pointers ON lfs_pointers.lfs_oid = lfs_objects.oid AND lfs_pointers.project_id = lfs_objects_projects.project_id")
+        .where(lfs_pointers: { lfs_oid: nil })
+  end
+
   private
 
   def update_project_statistics
diff --git a/app/models/processed_lfs_ref.rb b/app/models/processed_lfs_ref.rb
new file mode 100644
index 00000000000..dac7126b949
--- /dev/null
+++ b/app/models/processed_lfs_ref.rb
@@ -0,0 +1,6 @@
+class ProcessedLfsRef < ActiveRecord::Base
+  belongs_to :project
+
+  validates :project, presence: true
+  validates :ref, presence: true
+end
diff --git a/app/models/project.rb b/app/models/project.rb
index d71f20244bb..6023cc24d96 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -178,7 +178,8 @@ class Project < ActiveRecord::Base
   has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :lfs_objects, through: :lfs_objects_projects
   has_many :lfs_pointers
-  has_many :reference_changes
+  has_many :unprocessed_lfs_pushes
+  has_many :processed_lfs_refs
   has_many :project_group_links
   has_many :invited_groups, through: :project_group_links, source: :group
   has_many :pages_domains
diff --git a/app/models/reference_change.rb b/app/models/reference_change.rb
deleted file mode 100644
index 341f883a769..00000000000
--- a/app/models/reference_change.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-class ReferenceChange < ActiveRecord::Base
-  belongs_to :project
-
-  validates :project, presence: true
-  validates :newrev, presence: true
-
-  scope :processed, -> { where(processed: true) }
-  scope :unprocessed, -> { where(processed: false) }
-end
diff --git a/app/models/unprocessed_lfs_push.rb b/app/models/unprocessed_lfs_push.rb
new file mode 100644
index 00000000000..09f5568d08a
--- /dev/null
+++ b/app/models/unprocessed_lfs_push.rb
@@ -0,0 +1,15 @@
+class UnprocessedLfsPush < ActiveRecord::Base
+  belongs_to :project
+
+  validates :project, presence: true
+  validates :newrev, presence: true
+  validates :ref, presence: true
+
+  def processed!
+    transaction do
+      project.processed_lfs_refs.find_or_create_by(ref: ref)
+
+      self.destroy!
+    end
+  end
+end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 77da8bb766c..315104fc599 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -123,10 +123,10 @@ class GitPushService < BaseService
     return unless @project.lfs_enabled?
     return unless @project.lfs_objects.exists?
 
-    reference_change = ReferenceChange.create!(project: @project,
-                                               newrev: params[:newrev])
+    unprocessed_lfs_push = @project.unprocessed_lfs_pushes.create!(ref: params[:ref],
+                                                                   newrev: params[:newrev])
 
-    UpdateLfsPointersWorker.perform_async(reference_change.id)
+    UpdateLfsPointersWorker.perform_async(unprocessed_lfs_push.id)
   end
 
   protected
diff --git a/app/services/lfs_cleanup_service.rb b/app/services/lfs_cleanup_service.rb
index a0f7d8a0c0e..1c55551c098 100644
--- a/app/services/lfs_cleanup_service.rb
+++ b/app/services/lfs_cleanup_service.rb
@@ -1,26 +1,36 @@
 class LfsCleanupService
   attr_reader :project
 
+  MIN_LFS_USAGE = 5.megabytes
+
   def initialize(project)
     @project = project
   end
 
   def remove_unreferenced
     return unless project.lfs_enabled?
-    return unless project.lfs_pointers.exists? # LFS pointers were found
-    return unless project.reference_changes.exists? # Has been scanned for lfs pointers
-    return if project.reference_changes.unprocessed.exists? # No scans still in progress
+    return unless project.lfs_pointers.exists?
+    return if project.unprocessed_lfs_pushes.exists?
     return unless project.lfs_objects.exists?
+    return if project.statistics.lfs_objects_size < MIN_LFS_USAGE
+
+    unreferenced_pointers.delete_all
 
-    unreferenced_pointers.destroy_all
+    project.lfs_objects_projects.without_pointers.delete_all
+
+    ProjectCacheWorker.perform_async(project.id, [], [:lfs_objects_size])
   end
 
   def unreferenced_pointers
-    pointer_oids = project.lfs_pointers.pluck(:blob_oid)
-    removed_pointer_oids = Gitlab::Git::Blob.batch_blob_existance(project.repository,
-                                                                  pointer_oids,
-                                                                  existing: false)
-
     project.lfs_pointers.where(blob_oid: removed_pointer_oids)
   end
+
+  private
+
+  def removed_pointer_oids
+    @removed_pointer_oids ||= begin
+      pointer_oids = project.lfs_pointers.pluck(:blob_oid)
+      project.repository.batch_existence(pointer_oids, existing: false)
+    end
+  end
 end
diff --git a/app/workers/lfs_project_cleanup_worker.rb b/app/workers/lfs_project_cleanup_worker.rb
index 982c0cae841..48a7a46ebbb 100644
--- a/app/workers/lfs_project_cleanup_worker.rb
+++ b/app/workers/lfs_project_cleanup_worker.rb
@@ -2,7 +2,9 @@ class LfsProjectCleanupWorker
   include Sidekiq::Worker
 
   def perform(project_id)
-    project = Project.find(project_id)
+    project = Project.find_by(id: project_id)
+
+    return unless project
 
     LfsCleanupService.new(project).remove_unreferenced
   end
diff --git a/app/workers/update_lfs_pointers_worker.rb b/app/workers/update_lfs_pointers_worker.rb
index 812a026f8cd..3af045942aa 100644
--- a/app/workers/update_lfs_pointers_worker.rb
+++ b/app/workers/update_lfs_pointers_worker.rb
@@ -2,17 +2,16 @@ class UpdateLfsPointersWorker
   include Sidekiq::Worker
   include CronjobQueue
 
-  attr_reader :reference_change
+  attr_reader :unprocessed_lfs_push
 
-  def perform(reference_change_id)
-    @reference_change = ReferenceChange.find(reference_change_id)
+  def perform(unprocessed_lfs_push_id)
+    @unprocessed_lfs_push = UnprocessedLfsPush.find(unprocessed_lfs_push_id)
 
     return unless project.lfs_enabled?
-    return if @reference_change.processed?
 
     create_lfs_pointer_records
 
-    reference_change.update!(processed: true)
+    unprocessed_lfs_push.processed!
   end
 
   private
@@ -24,13 +23,21 @@ class UpdateLfsPointersWorker
   end
 
   def new_lfs_pointers
-    processed_references = project.reference_changes.processed.pluck(:newrev)
+    processed_references = project.processed_lfs_refs
+                                  .order(updated_at: :desc)
+                                  .limit(100)
+                                  .pluck(:ref)
 
-    lfs_changes = Gitlab::Git::LfsChanges.new(project.repository, reference_change.newrev)
-    lfs_changes.new_pointers(not_in: processed_references)
+    lfs_changes = Gitlab::Git::LfsChanges.new(project.repository, unprocessed_lfs_push.newrev)
+
+    if processed_references.present?
+      lfs_changes.new_pointers(not_in: processed_references)
+    else
+      lfs_changes.all_pointers
+    end
   end
 
   def project
-    reference_change.project
+    unprocessed_lfs_push.project
   end
 end
diff --git a/db/migrate/20170922164114_create_unprocessed_lfs_pushes.rb b/db/migrate/20170922164114_create_unprocessed_lfs_pushes.rb
new file mode 100644
index 00000000000..479768adff9
--- /dev/null
+++ b/db/migrate/20170922164114_create_unprocessed_lfs_pushes.rb
@@ -0,0 +1,35 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CreateUnprocessedLfsPushes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    create_table :unprocessed_lfs_pushes do |t|
+      t.references :project, null: false, index: true, foreign_key: { on_delete: :cascade }
+      t.string :ref, null: false
+      t.string :newrev, null: false #TODO: We can probably remove this and use the ref instead. Maybe only use this for deletions and force pushes?
+    end
+  end
+end
diff --git a/db/migrate/20170922171356_create_lfs_pointer.rb b/db/migrate/20170922171356_create_lfs_pointer.rb
index 3917df1af1f..95232f0b110 100644
--- a/db/migrate/20170922171356_create_lfs_pointer.rb
+++ b/db/migrate/20170922171356_create_lfs_pointer.rb
@@ -27,10 +27,10 @@ class CreateLfsPointer < ActiveRecord::Migration
 
   def change
     create_table :lfs_pointers do |t|
+      t.references :project, null: false, index: true, foreign_key: { on_delete: :cascade }
       t.string :blob_oid, null: false
       t.string :lfs_oid, null: false
-      t.references :project, index: true, foreign_key: true
-      t.string :timestamps
+      t.index :blob_oid # Used to filter on removed blobs
     end
   end
 end
diff --git a/db/migrate/20170922164114_create_reference_change.rb b/db/migrate/20171013174017_create_processed_lfs_refs.rb
index 447042d49c7..f149ae00d97 100644
--- a/db/migrate/20170922164114_create_reference_change.rb
+++ b/db/migrate/20171013174017_create_processed_lfs_refs.rb
@@ -1,7 +1,7 @@
 # See http://doc.gitlab.com/ce/development/migration_style_guide.html
 # for more information on how to write migrations for GitLab.
 
-class CreateReferenceChange < ActiveRecord::Migration
+class CreateProcessedLfsRefs < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
   # Set this constant to true if this migration requires downtime.
@@ -26,11 +26,10 @@ class CreateReferenceChange < ActiveRecord::Migration
   # disable_ddl_transaction!
 
   def change
-    create_table :reference_changes do |t|
-      t.references :project, index: true, foreign_key: true, null: false
-      t.string :newrev
-      t.boolean :processed, default: false, null: false
-      t.string :timestamps
+    create_table :processed_lfs_refs do |t|
+      t.references :project, null: false, index: true, foreign_key: { on_delete: :cascade }
+      t.datetime_with_timezone :updated_at, null: false
+      t.string :ref, null: false
     end
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index d8268473c04..1500c4ebe62 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -843,12 +843,12 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_index "lfs_objects_projects", ["project_id"], name: "index_lfs_objects_projects_on_project_id", using: :btree
 
   create_table "lfs_pointers", force: :cascade do |t|
-    t.string "blob_oid"
-    t.string "lfs_oid"
-    t.integer "project_id"
-    t.string "timestamps"
+    t.integer "project_id", null: false
+    t.string "blob_oid", null: false
+    t.string "lfs_oid", null: false
   end
 
+  add_index "lfs_pointers", ["blob_oid"], name: "index_lfs_pointers_on_blob_oid", using: :btree
   add_index "lfs_pointers", ["project_id"], name: "index_lfs_pointers_on_project_id", using: :btree
 
   create_table "lists", force: :cascade do |t|
@@ -1204,6 +1204,14 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
   add_index "personal_access_tokens", ["user_id"], name: "index_personal_access_tokens_on_user_id", using: :btree
 
+  create_table "processed_lfs_refs", force: :cascade do |t|
+    t.integer "project_id", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "ref", null: false
+  end
+
+  add_index "processed_lfs_refs", ["project_id"], name: "index_processed_lfs_refs_on_project_id", using: :btree
+
   create_table "project_authorizations", id: false, force: :cascade do |t|
     t.integer "user_id"
     t.integer "project_id"
@@ -1413,15 +1421,6 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"}
   add_index "redirect_routes", ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree
 
-  create_table "reference_changes", force: :cascade do |t|
-    t.integer "project_id", null: false
-    t.string "newrev"
-    t.boolean "processed", default: false, null: false
-    t.string "timestamps"
-  end
-
-  add_index "reference_changes", ["project_id"], name: "index_reference_changes_on_project_id", using: :btree
-
   create_table "releases", force: :cascade do |t|
     t.string "tag"
     t.text "description"
@@ -1624,6 +1623,14 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree
   add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree
 
+  create_table "unprocessed_lfs_pushes", force: :cascade do |t|
+    t.integer "project_id", null: false
+    t.string "ref", null: false
+    t.string "newrev", null: false
+  end
+
+  add_index "unprocessed_lfs_pushes", ["project_id"], name: "index_unprocessed_lfs_pushes_on_project_id", using: :btree
+
   create_table "uploads", force: :cascade do |t|
     t.integer "size", limit: 8, null: false
     t.string "path", null: false
@@ -1856,7 +1863,7 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_foreign_key "label_priorities", "projects", on_delete: :cascade
   add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "labels", "projects", name: "fk_7de4989a69", on_delete: :cascade
-  add_foreign_key "lfs_pointers", "projects"
+  add_foreign_key "lfs_pointers", "projects", on_delete: :cascade
   add_foreign_key "lists", "boards", name: "fk_0d3f677137", on_delete: :cascade
   add_foreign_key "lists", "labels", name: "fk_7a5553d60f", on_delete: :cascade
   add_foreign_key "merge_request_diff_commits", "merge_request_diffs", on_delete: :cascade
@@ -1875,6 +1882,7 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id"
   add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade
   add_foreign_key "personal_access_tokens", "users"
+  add_foreign_key "processed_lfs_refs", "projects", on_delete: :cascade
   add_foreign_key "project_authorizations", "projects", on_delete: :cascade
   add_foreign_key "project_authorizations", "users", on_delete: :cascade
   add_foreign_key "project_auto_devops", "projects", on_delete: :cascade
@@ -1890,7 +1898,6 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_foreign_key "protected_tag_create_access_levels", "users"
   add_foreign_key "protected_tags", "projects", name: "fk_8e4af87648", on_delete: :cascade
   add_foreign_key "push_event_payloads", "events", name: "fk_36c74129da", on_delete: :cascade
-  add_foreign_key "reference_changes", "projects"
   add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade
   add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade
   add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade
@@ -1901,6 +1908,7 @@ ActiveRecord::Schema.define(version: 20171026082505) do
   add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
+  add_foreign_key "unprocessed_lfs_pushes", "projects", on_delete: :cascade
   add_foreign_key "user_custom_attributes", "users", on_delete: :cascade
   add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade
   add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade
diff --git a/spec/factories/processed_lfs_ref.rb b/spec/factories/processed_lfs_ref.rb
new file mode 100644
index 00000000000..160ab047cc7
--- /dev/null
+++ b/spec/factories/processed_lfs_ref.rb
@@ -0,0 +1,6 @@
+FactoryGirl.define do
+  factory :processed_lfs_ref do
+    project
+    ref 'feature_branch'
+  end
+end
diff --git a/spec/factories/reference_change.rb b/spec/factories/unprocessed_lfs_push.rb
index 4f3117662a8..cbb389247e0 100644
--- a/spec/factories/reference_change.rb
+++ b/spec/factories/unprocessed_lfs_push.rb
@@ -1,6 +1,7 @@
 FactoryGirl.define do
-  factory :reference_change do
+  factory :unprocessed_lfs_push do
     project
     newrev '4b67e82213a2ce0f8b4ed1d65bef48ef8b8be1f3'
+    ref 'feature_branch'
   end
 end
diff --git a/spec/models/processed_lfs_ref_spec.rb b/spec/models/processed_lfs_ref_spec.rb
new file mode 100644
index 00000000000..ef60cfa6a73
--- /dev/null
+++ b/spec/models/processed_lfs_ref_spec.rb
@@ -0,0 +1,8 @@
+require 'spec_helper'
+
+describe ProcessedLfsRef do
+  it { is_expected.to belong_to(:project) }
+
+  it { is_expected.to have_db_column(:newrev).of_type(:string).with_options(null: false) }
+  it { is_expected.to have_db_column(:ref).of_type(:string).with_options(null: false) }
+end
diff --git a/spec/models/reference_change_spec.rb b/spec/models/reference_change_spec.rb
deleted file mode 100644
index 338a0eecd53..00000000000
--- a/spec/models/reference_change_spec.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-require 'spec_helper'
-
-describe ReferenceChange do
-  it { is_expected.to belong_to(:project) }
-
-  it { is_expected.to have_db_column(:newrev).of_type(:string) }
-  it { is_expected.to have_db_column(:processed).of_type(:boolean).with_options(default: false, null: false) }
-
-  describe "scopes" do
-    describe ".processed" do
-      it 'excludes reference changes by default' do
-        create(:reference_change)
-
-        expect(described_class.processed.count).to eq 0
-      end
-
-      it 'includes processed reference changes' do
-        create(:reference_change, processed: true)
-
-        expect(described_class.processed.count).to eq 1
-      end
-    end
-  end
-end
diff --git a/spec/models/unprocessed_lfs_push_spec.rb b/spec/models/unprocessed_lfs_push_spec.rb
new file mode 100644
index 00000000000..e315f620dcd
--- /dev/null
+++ b/spec/models/unprocessed_lfs_push_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe UnprocessedLfsPush do
+  it { is_expected.to belong_to(:project) }
+
+  it { is_expected.to have_db_column(:newrev).of_type(:string).with_options(null: false) }
+  it { is_expected.to have_db_column(:ref).of_type(:string).with_options(null: false) }
+
+  describe "#processed!" do
+    subject(:unprocessed_lfs_push) { create(:unprocessed_lfs_push) }
+    let(:project) { subject.project }
+
+    it 'deletes the record' do
+      subject.processed!
+
+      expect(subject).to be_destroyed
+    end
+
+    it 'creates a ProcessedLfsRef' do
+      expect { subject.processed! }.to change(ProcessedLfsRef, :count).by(1)
+    end
+
+    it 'updates an existing ProcessedLfsRef' do
+      create(:processed_lfs_ref, project: project, ref: subject.ref)
+
+      expect { subject.processed! }.to change { processed_lfs_push.reload.newrev }
+    end
+
+    #TODO: Could create ProcessedLfsRef with sha on force-push or deletion
+  end
+end
diff --git a/spec/services/lfs_cleanup_service_spec.rb b/spec/services/lfs_cleanup_service_spec.rb
index 58fdadd1474..561b64713aa 100644
--- a/spec/services/lfs_cleanup_service_spec.rb
+++ b/spec/services/lfs_cleanup_service_spec.rb
@@ -2,14 +2,18 @@ require 'spec_helper'
 
 describe LfsCleanupService do
   let(:project) { create(:project) }
-  let!(:lfs_pointer) { create(:lfs_pointer, project: project) }
-  let!(:lfs_objects_project) { create(:lfs_objects_project, project: project) }
-  let!(:reference_change) { create(:reference_change, project: project, processed: true) }
+  let!(:lfs_object) { create(:lfs_object, size: 10.megabytes) }
+  let!(:lfs_pointer) { create(:lfs_pointer, project: project, lfs_oid: lfs_object.oid) }
+  let!(:lfs_objects_project) { create(:lfs_objects_project, project: project, lfs_object: lfs_object) }
+  let!(:processed_lfs_ref) { create(:processed_lfs_ref, project: project) }
 
   subject(:service) { described_class.new(project) }
 
   before do
     allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
+
+    # ProjectCacheWorker skips this becuase no project.repository.exists?
+    project.statistics.refresh!(only: [:lfs_objects_size])
   end
 
   describe '#remove_unreferenced' do
@@ -29,28 +33,33 @@ describe LfsCleanupService do
       subject.remove_unreferenced
     end
 
-    it "skips project if it hasn't been scanned for LFS pointers" do
-      ReferenceChange.destroy_all
+    it 'skips project if newly pushed changes could contain LFS pointers' do
+      create(:unprocessed_lfs_push, project: project)
 
       expect(subject).not_to receive(:unreferenced_pointers)
 
       subject.remove_unreferenced
     end
 
-    it 'skips project if newly pushed changes could contain LFS pointers' do
-      reference_change.update!(processed: false)
+    it 'skips project if no LfsObjects exist to clean up' do
+      LfsObject.destroy_all
 
       expect(subject).not_to receive(:unreferenced_pointers)
 
       subject.remove_unreferenced
     end
 
-    it 'skips project if no LfsObjects exist to clean up' do
-      LfsObject.destroy_all
+    context 'with less than 5MB of LFS files in the project' do
+      before do
+        lfs_object.update!(size: 5.megabytes - 1)
+        project.statistics.refresh!(only: [:lfs_objects_size])
+      end
 
-      expect(subject).not_to receive(:unreferenced_pointers)
+      it 'skips cleanup' do
+        expect(subject).not_to receive(:unreferenced_pointers)
 
-      subject.remove_unreferenced
+        subject.remove_unreferenced
+      end
     end
 
     it 'removes LfsPointers which no longer exist in the project' do
@@ -59,18 +68,53 @@ describe LfsCleanupService do
       expect { subject.remove_unreferenced }.to change(LfsPointer, :count).by(-1)
     end
 
-    it 'removes LfsObjectProjects/LfsObjects which are no longer referenced by pointers'
+    describe 'LfsObjectProjects' do
+      let!(:project) { create(:project, :repository) }
+
+      it 'are removed when no longer referenced by pointers' do
+        allow(subject).to receive(:removed_pointer_oids).and_return([lfs_pointer.blob_oid])
+
+        expect { subject.remove_unreferenced }.to change(LfsObjectsProject, :count).by(-1)
+      end
+
+      it 'cause LFS storage statistics to be updated' do
+        allow(subject).to receive(:removed_pointer_oids).and_return([lfs_pointer.blob_oid])
+
+        expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:lfs_objects_size])
+
+        subject.remove_unreferenced
+      end
+
+      it "don't cause LfsObjects to be removed" do
+        allow(subject).to receive(:removed_pointer_oids).and_return([lfs_pointer.blob_oid])
+
+        expect { subject.remove_unreferenced }.not_to change(LfsObject, :count)
+      end
+
+      it "are not removed when pointers haven't been removed" do
+        allow(subject).to receive(:removed_pointer_oids).and_return([])
+
+        expect { subject.remove_unreferenced }.not_to change(LfsObjectsProject, :count)
+      end
+
+      it 'are not removed if other pointers still reference them' do
+        create(:lfs_pointer, project: project,
+                             lfs_oid: lfs_object.oid,
+                             blob_oid: 'b7f094b759e6b023278323195812ce1019cb57ff')
 
-    # xit 'skips projects with less than 5MB of LfsFiles'
+        allow(subject).to receive(:removed_pointer_oids).and_return([lfs_pointer.blob_oid])
+
+        expect { subject.remove_unreferenced }.not_to change(LfsObjectsProject, :count)
+      end
+    end
   end
 
   describe '#unreferenced_pointers' do
     let(:removed_pointer_oids) { [lfs_pointer.blob_oid] }
 
     before do
-      allow(Gitlab::Git::Blob).to receive(:batch_blob_existance)
-                                          .with(project.repository,
-                                                [lfs_pointer.blob_oid],
+      allow(project.repository).to receive(:batch_existence)
+                                          .with([lfs_pointer.blob_oid],
                                                 existing: false)
                                           .and_return(removed_pointer_oids)
     end
diff --git a/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb b/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb
index 57f83c1dbe9..0b9830caedb 100644
--- a/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb
+++ b/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb
@@ -51,5 +51,11 @@ describe RemoveUnreferencedLfsObjectsWorker do
       expect(referenced_lfs_object1.reload).to be_present
       expect(LfsObject.where(id: referenced_lfs_object2.id)).to be_empty
     end
+
+    it 'removes LFS files from disk storage' do
+      expect_any_instance_of(ActiveRecord::Relation).to receive(:destroy_all).and_call_original
+
+      worker.perform
+    end
   end
 end
diff --git a/spec/workers/update_lfs_pointers_worker_spec.rb b/spec/workers/update_lfs_pointers_worker_spec.rb
index ebec3caf061..b87cd209356 100644
--- a/spec/workers/update_lfs_pointers_worker_spec.rb
+++ b/spec/workers/update_lfs_pointers_worker_spec.rb
@@ -5,7 +5,7 @@ describe UpdateLfsPointersWorker do
 
   describe '#perform' do
     let(:project) { create(:project, :repository) }
-    let(:reference_change) { create(:reference_change, project: project) }
+    let!(:unprocessed_lfs_push) { create(:unprocessed_lfs_push, project: project) }
     let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') }
 
     before do
@@ -13,7 +13,7 @@ describe UpdateLfsPointersWorker do
     end
 
     def perform
-      subject.perform(reference_change.id)
+      subject.perform(unprocessed_lfs_push.id)
     end
 
     context 'with LFS not enabled' do
@@ -39,8 +39,12 @@ describe UpdateLfsPointersWorker do
         perform
       end
 
-      it 'marks the updated reference as processed' do
-        expect { perform }.to change { reference_change.reload.processed }.from(false).to(true)
+      it 'removes the updated reference from unprocessed_lfs_pushes' do
+        expect { perform }.to change { project.reload.unprocessed_lfs_pushes.count }.by(-1)
+      end
+
+      it 'creates a ProcessedLfsRef for the reference' do
+        expect { perform }.to change { project.processed_lfs_refs.count }.by(1)
       end
 
       it 'creates a LfsPointer record for each new blob' do
@@ -48,24 +52,22 @@ describe UpdateLfsPointersWorker do
       end
 
       it 'looks up LFS pointers for the new ref' do
-        expect(Gitlab::Git::RevList).to receive(:new).with(hash_including(newrev: reference_change.newrev)).and_call_original
+        expect(Gitlab::Git::RevList).to receive(:new).with(hash_including(newrev: unprocessed_lfs_push.newrev)).and_call_original
 
         perform
       end
 
-      it 'scans all objects in the given ref on first run' do
-        expect_any_instance_of(Gitlab::Git::LfsChanges).to receive(:new_pointers).with(not_in: []).and_call_original
+      it 'scans all objects in the project on first run' do
+        expect_any_instance_of(Gitlab::Git::LfsChanges).to receive(:all_pointers).and_call_original
 
         perform
       end
 
       context 'with processed refrences' do
-        let(:processed_reference) { 'fa15ebeefacce55ed4dec0dea5decafbeefba115' }
+        let(:processed_reference) { 'some_feature' }
 
         before do
-          create(:reference_change, project: project,
-                                    newrev: processed_reference,
-                                    processed: true)
+          create(:processed_lfs_ref, project: project, ref: processed_reference)
         end
 
         it 'ignores objects reachable from processed refs' do