Merge branch '300524-create-worker-to-clean-up-composer-cache-pages' into 'master'

Cleanup composer cache pages

See merge request gitlab-org/gitlab!53005

app/models/packages/composer/cache_file.rb

@@ -16,6 +16,8 @@ module Packages
 
       scope :with_namespace, ->(namespace) { where(namespace: namespace) }
       scope :with_sha, ->(sha) { where(file_sha256: sha) }
+      scope :expired, -> { where("delete_at <= ?", Time.current) }
+      scope :without_namespace, -> { where(namespace_id: nil) }
     end
   end
 end

app/workers/all_queues.yml

@@ -267,6 +267,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: cronjob:packages_composer_cache_cleanup
+  :feature_category: :package_registry
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: cronjob:pages_domain_removal_cron
   :feature_category: :pages
   :has_external_dependencies:

app/workers/packages/composer/cache_cleanup_worker.rb

+# frozen_string_literal: true
+
+module Packages
+  module Composer
+    class CacheCleanupWorker
+      include ApplicationWorker
+      include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+
+      feature_category :package_registry
+
+      idempotent!
+
+      def perform
+        ::Packages::Composer::CacheFile.without_namespace.find_in_batches do |cache_files|
+          cache_files.each(&:destroy)
+        rescue ActiveRecord::RecordNotFound
+          # ignore. likely due to object already being deleted.
+        end
+
+        ::Packages::Composer::CacheFile.expired.find_in_batches do |cache_files|
+          cache_files.each(&:destroy)
+        rescue ActiveRecord::RecordNotFound
+          # ignore. likely due to object already being deleted.
+        end
+      rescue => e
+        Gitlab::ErrorTracking.log_exception(e)
+      end
+    end
+  end
+end

changelogs/unreleased/300524-create-worker-to-clean-up-composer-cache-pages.yml

+---
+title: Cleanup composer cache pages
+merge_request: 53005
+author:
+type: other

config/initializers/1_settings.rb

@@ -454,6 +454,9 @@ Settings.cron_jobs['personal_access_tokens_expired_notification_worker']['job_cl
 Settings.cron_jobs['repository_archive_cache_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['repository_archive_cache_worker']['cron'] ||= '0 * * * *'
 Settings.cron_jobs['repository_archive_cache_worker']['job_class'] = 'RepositoryArchiveCacheWorker'
+Settings.cron_jobs['packages_composer_cache_cleanup_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['packages_composer_cache_cleanup_worker']['cron'] ||= '30 * * * *'
+Settings.cron_jobs['packages_composer_cache_cleanup_worker']['job_class'] = 'Packages::Composer::CacheCleanupWorker'
 Settings.cron_jobs['import_export_project_cleanup_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['import_export_project_cleanup_worker']['cron'] ||= '0 * * * *'
 Settings.cron_jobs['import_export_project_cleanup_worker']['job_class'] = 'ImportExportProjectCleanupWorker'

db/migrate/20210203222620_add_expired_index_to_composer_cache_files.rb

+# frozen_string_literal: true
+
+class AddExpiredIndexToComposerCacheFiles < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'composer_cache_files_index_on_deleted_at'
+
+  def up
+    add_concurrent_index :packages_composer_cache_files, [:delete_at, :id], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :packages_composer_cache_files, INDEX_NAME
+  end
+end

db/migrate/20210203223551_add_orphan_index_to_composer_cache_files.rb

+# frozen_string_literal: true
+
+class AddOrphanIndexToComposerCacheFiles < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_composer_cache_files_where_namespace_id_is_null'
+
+  def up
+    add_concurrent_index :packages_composer_cache_files, :id, name: INDEX_NAME, where: 'namespace_id IS NULL'
+  end
+
+  def down
+    remove_concurrent_index_by_name :packages_composer_cache_files, INDEX_NAME
+  end
+end

db/schema_migrations/20210203222620

+6d3250533b72c6aa307570d386725fa3ebe1ec49c36edc0f7d6dc8a1d5092826
\ No newline at end of file

db/schema_migrations/20210203223551

+4ccf450bbc9061edae81cabcfafd9360f1f57cfd25af3ad016fbbb344f9fe694
\ No newline at end of file

db/structure.sql

@@ -21202,6 +21202,8 @@ CREATE INDEX code_owner_approval_required ON protected_branches USING btree (pro
 
 CREATE INDEX commit_id_and_note_id_index ON commit_user_mentions USING btree (commit_id, note_id);
 
+CREATE INDEX composer_cache_files_index_on_deleted_at ON packages_composer_cache_files USING btree (delete_at, id);
+
 CREATE UNIQUE INDEX design_management_designs_versions_uniqueness ON design_management_designs_versions USING btree (design_id, version_id);
 
 CREATE INDEX design_user_mentions_on_design_id_and_note_id_index ON design_user_mentions USING btree (design_id, note_id);
@@ -21860,6 +21862,8 @@ CREATE INDEX index_clusters_on_user_id ON clusters USING btree (user_id);
 
 CREATE UNIQUE INDEX index_commit_user_mentions_on_note_id ON commit_user_mentions USING btree (note_id);
 
+CREATE INDEX index_composer_cache_files_where_namespace_id_is_null ON packages_composer_cache_files USING btree (id) WHERE (namespace_id IS NULL);
+
 CREATE INDEX index_container_expiration_policies_on_next_run_at_and_enabled ON container_expiration_policies USING btree (next_run_at, enabled);
 
 CREATE INDEX index_container_repositories_on_project_id ON container_repositories USING btree (project_id);

spec/models/packages/composer/cache_file_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe Packages::Composer::CacheFile, type: :model do
     let_it_be(:group1) { create(:group) }
     let_it_be(:group2) { create(:group) }
     let_it_be(:cache_file1) { create(:composer_cache_file, file_sha256: '123456', group: group1) }
-    let_it_be(:cache_file2) { create(:composer_cache_file, file_sha256: '456778', group: group2) }
+    let_it_be(:cache_file2) { create(:composer_cache_file, delete_at: 2.days.from_now, file_sha256: '456778', group: group2) }
 
     describe '.with_namespace' do
       subject { described_class.with_namespace(group1) }

spec/workers/packages/composer/cache_cleanup_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Composer::CacheCleanupWorker, type: :worker do
+  describe '#perform' do
+    let_it_be(:group) { create(:group) }
+
+    let!(:cache_file1) { create(:composer_cache_file, delete_at: nil, group: group, file_sha256: '124') }
+    let!(:cache_file2) { create(:composer_cache_file, delete_at: 2.days.from_now, group: group, file_sha256: '3456') }
+    let!(:cache_file3) { create(:composer_cache_file, delete_at: 1.day.ago, group: group, file_sha256: '5346') }
+    let!(:cache_file4) { create(:composer_cache_file, delete_at: nil, group: group, file_sha256: '56889') }
+
+    subject { described_class.new.perform }
+
+    before do
+      # emulate group deletion
+      cache_file4.update_columns(namespace_id: nil)
+    end
+
+    it 'deletes expired packages' do
+      expect { subject }.to change { Packages::Composer::CacheFile.count }.by(-2)
+      expect { cache_file1.reload }.not_to raise_error ActiveRecord::RecordNotFound
+      expect { cache_file2.reload }.not_to raise_error ActiveRecord::RecordNotFound
+      expect { cache_file3.reload }.to raise_error ActiveRecord::RecordNotFound
+      expect { cache_file4.reload }.to raise_error ActiveRecord::RecordNotFound
+    end
+  end
+end