From 6cfd8cf2a7c94f3b259143071080f933ef5dfbc5 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 13 Jul 2026 11:22:57 +0200 Subject: [PATCH] Tech: maintenance task to purge unattached openstack blobs Re-scans the whole blob stock by id range, targeting only openstack blobs, and hard-deletes the orphans: their variants, the parent blobs, and the corresponding files on the bucket via delete_multiple_objects. Ref #13421 --- app/services/blob_service.rb | 68 ++++++++++++++---- ...3_purge_unattached_openstack_blobs_task.rb | 41 +++++++++++ spec/services/blob_service_spec.rb | 13 +++- ...ge_unattached_openstack_blobs_task_spec.rb | 71 +++++++++++++++++++ 4 files changed, 177 insertions(+), 16 deletions(-) create mode 100644 app/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task.rb create mode 100644 spec/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task_spec.rb diff --git a/app/services/blob_service.rb b/app/services/blob_service.rb index 0b1ec3846ef..fa6a1d3cf4c 100644 --- a/app/services/blob_service.rb +++ b/app/services/blob_service.rb @@ -10,35 +10,75 @@ class << self # - add all the related variant blobs # - delete the corresponding files on the openstack bucket # - delete in db attachment / variant / blob + # + # Returns an audit of every purged blob, one hash per blob: + # { id:, key:, file_deleted:, variant:, parent_id: } + # (parent_id is set only for variant blobs). Callers that don't need it + # can ignore the return value. def purge_blobs_with_variants(parent_ids) - parent_ids = ActiveStorage::Blob.where(id: parent_ids, service_name: :openstack).ids + parents = ActiveStorage::Blob.where(id: parent_ids, service_name: :openstack) + .pluck(:id, :key) + .map { |blob_id, key| { blob_id:, key: } } - variant_record_ids = ActiveStorage::VariantRecord.where(blob_id: parent_ids).ids - variant_attachments = ActiveStorage::Attachment - .where(record_type: 'ActiveStorage::VariantRecord', record_id: variant_record_ids) - blob_ids = variant_attachments.pluck(:blob_id) + parent_ids + blob_id_by_variant_id = ActiveStorage::VariantRecord.where(blob_id: parents.map { it[:blob_id] }) + .pluck(:id, :blob_id) + .to_h - delete_files(blob_ids) + variant_id_attachment_id_by_blob_id = ActiveStorage::Attachment + .where(record_type: 'ActiveStorage::VariantRecord', record_id: blob_id_by_variant_id.keys) + .pluck(:blob_id, :record_id, :id) + .to_h { |blob_id, variant_id, attachment_id| [blob_id, { variant_id:, attachment_id: }] } + + variant_blob_id_key = ActiveStorage::Blob.where(id: variant_id_attachment_id_by_blob_id.keys).pluck(:id, :key) + + children = variant_blob_id_key.map do |blob_id, key| + variant_id_attachment_id_by_blob_id[blob_id] => { variant_id:, attachment_id: } + + { + blob_id:, + key:, + parent_blob_id: blob_id_by_variant_id[variant_id], + variant_id:, + attachment_id:, + } + end + + all = parents.concat(children) + + errored_keys = delete_files(all.map { it[:key] }) # delete_all bypasses callbacks, so we drop the rows ourselves, in FK order: # image attachments -> variant records -> blobs (variants + parents). - variant_attachments.delete_all - ActiveStorage::VariantRecord.where(id: variant_record_ids).delete_all - ActiveStorage::Blob.where(id: blob_ids).delete_all + ActiveStorage::Attachment.where(id: all.map { it[:attachment_id] }.compact).delete_all + ActiveStorage::VariantRecord.where(id: all.map { it[:variant_id] }.compact).delete_all + ActiveStorage::Blob.where(id: all.map { it[:blob_id] }).delete_all + + all.map do + { + id: it[:blob_id], + key: it[:key], + file_deleted: errored_keys.exclude?(it[:key]), + variant: it[:variant_id].present?, + parent_id: it[:parent_blob_id], + } + end end private - def delete_files(blob_ids) - keys = ActiveStorage::Blob.where(id: blob_ids).pluck(:key) - + # Bulk-deletes the given keys on the openstack bucket. + # Returns the keys Swift reported an error for (left on the bucket). + def delete_files(keys) service = ActiveStorage::Blob.service client = service.send(:client) - keys.each_slice(BULK_DELETE_LIMIT) do |slice| + keys.each_slice(BULK_DELETE_LIMIT).flat_map do |slice| response = client.delete_multiple_objects(service.container, slice) errors = response.body['Errors'] - Sentry.capture_message("Bulk delete errors", extra: { errors: }) if errors.present? + next [] if errors.blank? + + Sentry.capture_message("Bulk delete errors", extra: { errors: }) + errors.map { it.first.delete_prefix("#{service.container}/") } end end end diff --git a/app/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task.rb b/app/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task.rb new file mode 100644 index 00000000000..c0ec44995f1 --- /dev/null +++ b/app/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Maintenance + class T20260713PurgeUnattachedOpenstackBlobsTask < MaintenanceTasks::Task + # Reparcours de tout le stock de blobs (~120M) pour supprimer les orphelins + # laissés en base : blobs sans attachment, leurs variants, et les fichiers + # correspondants sur le bucket OpenStack. + # + # On itère par plage d'id (in_batches batche par clé primaire) en ne ciblant + # que les blobs stockés sur openstack. Pour chaque lot, on isole les parents + # réellement orphelins puis on délègue la purge à BlobService. + # + # Chaque blob supprimé est tracé (une ligne JSON) dans log/purge_unattached_ + # openstack_blobs.log, l'opération étant irréversible. + # + # Dépend du monkeypatch de delete_multiple_objects (config/initializers/ + # active_storage.rb, PR #13421). + + LOG_PATH = Rails.root.join('log', 'purge_unattached_openstack_blobs.log') + + def collection + ActiveStorage::Blob.where(service_name: :openstack).in_batches(of: BlobService::BULK_DELETE_LIMIT) + end + + def process(batch) + parent_ids = batch + .unattached + .where(soft_deleted_at: nil, created_at: ..1.day.ago).ids + + return if parent_ids.empty? + + BlobService.purge_blobs_with_variants(parent_ids).each { logger.info(it.to_json) } + end + + private + + def logger + @logger ||= Logger.new(LOG_PATH) + end + end +end diff --git a/spec/services/blob_service_spec.rb b/spec/services/blob_service_spec.rb index 5777e82d339..f765efdbff5 100644 --- a/spec/services/blob_service_spec.rb +++ b/spec/services/blob_service_spec.rb @@ -55,13 +55,22 @@ expect(ActiveStorage::Attachment.where(id: image_attachment.id)).not_to exist end - it 'reports per-object bulk delete errors to Sentry' do + it 'returns an audit of the parent and its variant blob' do + expect(purge_blobs_with_variants).to match_array([ + { id: blob.id, key: blob.key, file_deleted: true, variant: false, parent_id: nil }, + { id: variant_blob.id, key: variant_blob.key, file_deleted: true, variant: true, parent_id: blob.id }, + ]) + end + + it 'reports per-object bulk delete errors to Sentry and marks the file as not deleted' do errors = [["bucket/#{variant_blob.key}", "409 Conflict"]] allow(client).to receive(:delete_multiple_objects).and_return(double(body: { 'Errors' => errors })) expect(Sentry).to receive(:capture_message).with("Bulk delete errors", extra: { errors: }) - purge_blobs_with_variants + audit = purge_blobs_with_variants + expect(audit.find { it[:id] == variant_blob.id }).to include(file_deleted: false) + expect(audit.find { it[:id] == blob.id }).to include(file_deleted: true) end end diff --git a/spec/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task_spec.rb b/spec/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task_spec.rb new file mode 100644 index 00000000000..9a082773a37 --- /dev/null +++ b/spec/tasks/maintenance/t20260713_purge_unattached_openstack_blobs_task_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require "rails_helper" + +module Maintenance + RSpec.describe T20260713PurgeUnattachedOpenstackBlobsTask do + describe '#collection' do + it 'returns a batch enumerator scoped to openstack blobs' do + collection = described_class.new.collection + expect(collection).to be_a(ActiveRecord::Batches::BatchEnumerator) + expect(collection.relation.to_sql).to include("service_name").and include("openstack") + end + end + + describe '#process' do + let!(:unattached_blob) do + ActiveStorage::Blob.create_and_upload!(io: StringIO.new("orphan"), filename: "o.txt", content_type: "text/plain") + .tap { it.update_column(:created_at, 2.days.ago) } + end + + let!(:attached_blob) do + blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("kept"), filename: "k.txt", content_type: "text/plain") + ActiveStorage::Attachment.create!(name: "file", record: create(:dossier), blob:) + blob + end + + def process + described_class.new.process(ActiveStorage::Blob.where(id: [unattached_blob.id, attached_blob.id])) + end + + it 'delegates the unattached blob to BlobService, leaving out the attached one' do + expect(BlobService).to receive(:purge_blobs_with_variants).with([unattached_blob.id]).and_return([]) + + process + end + + it 'logs each purged blob to the log file as JSON' do + entry = { id: unattached_blob.id, key: unattached_blob.key, file_deleted: true, variant: false, parent_id: nil } + allow(BlobService).to receive(:purge_blobs_with_variants).and_return([entry]) + + fake_logger = instance_double(Logger) + allow(Logger).to receive(:new).with(described_class::LOG_PATH).and_return(fake_logger) + expect(fake_logger).to receive(:info).with(entry.to_json) + + process + end + + it 'does not call BlobService when the batch has no orphan' do + expect(BlobService).not_to receive(:purge_blobs_with_variants) + + described_class.new.process(ActiveStorage::Blob.where(id: attached_blob.id)) + end + + it 'leaves out a recently created orphan (may still be getting attached)' do + unattached_blob.update_column(:created_at, 1.hour.ago) + + expect(BlobService).not_to receive(:purge_blobs_with_variants) + + process + end + + it 'leaves out a soft-deleted orphan' do + unattached_blob.update_column(:soft_deleted_at, Time.current) + + expect(BlobService).not_to receive(:purge_blobs_with_variants) + + process + end + end + end +end