Merge pull request #955 from scientist-softserv/i852-improve-slow-vid…

…eo-processing-speeds

i852 Improve slow video processing speeds

app/jobs/create_derivatives_job_decorator.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# OVERRIDE Hyrax v3.6.0
+# @see CreateLargeDerivativesJob
+module CreateDerivativesJobDecorator
+  # OVERRIDE: Divert audio and video derivative
+  # creation to CreateLargeDerivativesJob.
+  def perform(file_set, file_id, filepath = nil)
+    return super if is_a?(CreateLargeDerivativesJob)
+    return super unless file_set.video? || file_set.audio?
+
+    CreateLargeDerivativesJob.perform_later(*arguments)
+    true
+  end
+end
+
+CreateDerivativesJob.prepend(CreateDerivativesJobDecorator)

app/jobs/create_large_derivatives_job.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+# CreateLargeDerivativesJob is intended to be used for resource-intensive derivative
+# generation (e.g. video processing). It is functionally similar to CreateDerivativesJob,
+# except that it queues jobs in the :resource_intensive queue.
+#
+# The worker responsible for processing jobs in the :resource_intensive queue should be
+# configured to have more resources dedicated to it, especially CPU. Otherwise, the
+# `ffmpeg` commands that this job class eventually triggers could be throttled.
+#
+# @see CreateDerivativesJobDecorator
+# @see Hydra::Derivatives::Processors::Ffmpeg
+# @see https://github.com/scientist-softserv/palni-palci/issues/852
+class CreateLargeDerivativesJob < CreateDerivativesJob
+  queue_as :resource_intensive
+end

config/sidekiq.yml

@@ -3,3 +3,4 @@
   - default
   - import
   - export
+  - resource_intensive

spec/jobs/create_derivatives_job_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+RSpec.describe CreateDerivativesJob do
+  around do |example|
+    ffmpeg_enabled = Hyrax.config.enable_ffmpeg
+    Hyrax.config.enable_ffmpeg = true
+    example.run
+    Hyrax.config.enable_ffmpeg = ffmpeg_enabled
+  end
+
+  describe 'recreating self as a CreateLargeDerivativesJob' do
+    let(:id)       { '123' }
+    let(:file_set) { FileSet.new }
+    let(:file) do
+      Hydra::PCDM::File.new.tap do |f|
+        f.content = 'foo'
+        f.original_name = filename
+        f.save!
+      end
+    end
+
+    before do
+      allow(FileSet).to receive(:find).with(id).and_return(file_set)
+      allow(file_set).to receive(:mime_type).and_return(mime_type)
+      allow(file_set).to receive(:id).and_return(id)
+      # Short-circuit irrelevant logic
+      allow(file_set).to receive(:reload)
+      allow(file_set).to receive(:update_index)
+    end
+
+    context 'with an image file' do
+      let(:mime_type) { 'image/jpeg' }
+      let(:filename) { 'picture.jpg' }
+
+      before do
+        # Short-circuit irrelevant logic
+        allow(Hydra::Derivatives::ImageDerivatives).to receive(:create)
+      end
+
+      it 'does not recreate as a CreateLargeDerivativesJob' do
+        expect(CreateLargeDerivativesJob).not_to receive(:perform_later)
+
+        described_class.perform_now(file_set, file.id)
+      end
+    end
+
+    context 'with an video file' do
+      let(:mime_type) { 'video/mp4' }
+      let(:filename) { 'video.mp4' }
+
+      before do
+        # Short-circuit irrelevant logic
+        allow(Hydra::Derivatives::VideoDerivatives).to receive(:create)
+      end
+
+      it 'recreates as a CreateLargeDerivativesJob' do
+        expect(CreateLargeDerivativesJob).to receive(:perform_later)
+
+        described_class.perform_now(file_set, file.id)
+      end
+    end
+
+    context 'with an audio file' do
+      let(:mime_type) { 'audio/x-wav' }
+      let(:filename) { 'audio.wav' }
+
+      before do
+        # Short-circuit irrelevant logic
+        allow(Hydra::Derivatives::AudioDerivatives).to receive(:create)
+      end
+
+      it 'recreates as a CreateLargeDerivativesJob' do
+        expect(CreateLargeDerivativesJob).to receive(:perform_later)
+
+        described_class.perform_now(file_set, file.id)
+      end
+    end
+  end
+end

spec/jobs/create_large_derivatives_job_spec.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+RSpec.describe CreateLargeDerivativesJob, type: :job do
+  let(:id)       { '123' }
+  let(:file_set) { FileSet.new }
+  let(:file) do
+    Hydra::PCDM::File.new.tap do |f|
+      f.content = 'foo'
+      f.original_name = 'video.mp4'
+      f.save!
+    end
+  end
+
+  before do
+    allow(FileSet).to receive(:find).with(id).and_return(file_set)
+    allow(file_set).to receive(:id).and_return(id)
+    # Short-circuit irrelevant logic
+    allow(file_set).to receive(:reload)
+    allow(file_set).to receive(:update_index)
+  end
+
+  it 'runs in the :resource_intensive queue' do
+    expect { described_class.perform_later(file_set, file.id) }
+      .to have_enqueued_job(described_class)
+      .on_queue('resource_intensive')
+  end
+
+  # @see CreateDerivativesJobDecorator#perform
+  it "doesn't schedule itself infinitly" do
+    expect(described_class).not_to receive(:perform_later)
+
+    described_class.perform_now(file_set, file.id)
+  end
+
+  it 'successfully calls the logic in CreateDerivativesJob' do
+    allow(file_set).to receive(:mime_type).and_return('video/mp4')
+    expect(Hydra::Derivatives::VideoDerivatives).to receive(:create)
+
+    described_class.perform_now(file_set, file.id)
+  end
+end