Run background reports with Sidekiq, not fork

Forking worked in theory but crashed the browser in system specs. It
also came with many other hurdles and isn't well known solution in the
Rails community. Sidekiq can give us better control over execution
limits as well.
This commit is contained in:
Maikel Linke
2023-03-24 16:32:36 +11:00
parent 4c61666fc7
commit 78fea7c7f2
4 changed files with 17 additions and 89 deletions

View File

@@ -55,11 +55,10 @@ module Admin
def render_report_as(format)
if OpenFoodNetwork::FeatureToggle.enabled?(:background_reports, spree_current_user)
job = ReportJob.new
JobProcessor.perform_forked(
job,
job = ReportJob.perform_later(
report_class, spree_current_user, params, format
)
sleep 1 until job.done?
# This result has been rendered by Rails in safe mode already.
job.result.html_safe # rubocop:disable Rails/OutputSafety

View File

@@ -1,29 +0,0 @@
# frozen_string_literal: true
# Forks into a separate process to contain memory usage and timeout errors.
class JobProcessor
def self.perform_forked(job, *args)
# Reports should abort when puma threads are killed to avoid wasting
# resources. Nobody would be collecting the result. We still need to
# implement a way to email or download reports later.
timeout = ENV.fetch("RACK_TIMEOUT_WAIT_TIMEOUT", "30").to_i
child = fork do
Process.setproctitle("Job worker #{job.job_id}")
Timeout.timeout(timeout) do
job.perform(*args)
end
# Exit is not a good idea within a Rails process but Rubocop doesn't know
# that we are in a forked process.
exit # rubocop:disable Rails/Exit
end
# Wait for the forked child process to exit.
Process.waitpid(child)
ensure
# If this Puma thread is interrupted then we need to detach the child
# process to avoid it becoming a zombie.
Process.detach(child)
end
end

View File

@@ -1,57 +0,0 @@
# frozen_string_literal: true
# We need to configure MiniRacer to allow forking.
# Otherwise this spec hangs on CI.
# https://github.com/rubyjs/mini_racer#fork-safety
require "mini_racer"
MiniRacer::Platform.set_flags!(:single_threaded)
require 'spec_helper'
class TestJob < ApplicationJob
def initialize
@file = Tempfile.new("test-job-result")
super
end
def perform(message)
@file.write(message)
end
def result
@file.rewind
@file.read
end
end
describe JobProcessor do
describe ".perform_forked" do
let(:job) { TestJob.new }
it "executes a job" do
JobProcessor.perform_forked(job, "hello")
expect(job.result).to eq "hello"
end
describe "with other unrelated children" do
let(:start_time) { Time.zone.now }
let(:end_time) { Time.zone.now }
# We made a mistake waiting for all forked processes.
# Now starting an unrelated process in a similar way.
around do |example|
start_time
other_process = fork { sleep 10 }
example.run
Process.kill("QUIT", other_process)
end
it "returns as soon as the job is done" do
JobProcessor.perform_forked(job, "hello")
expect(end_time).to be_within(10.seconds).of start_time
end
end
end
end

View File

@@ -31,6 +31,21 @@ describe '
end
end
describe "Background processing" do
before do
Flipper.enable(:background_reports)
ActiveJob::Base.queue_adapter.perform_enqueued_jobs = true
end
it "can run the customers report" do
login_as_admin_and_visit admin_report_path(
report_type: :customers, report_subtype: :mailing_list
)
click_button "Go"
expect(page).to have_content "EMAIL FIRST NAME"
end
end
describe "Can access Customers reports and generate customers report" do
before do
login_as_admin_and_visit admin_reports_path