Skip to content

Commit 109dcaf

Browse files
authored
Job warnings have to be deleted explicitly (#4175)
For the reasoning, see comment in PollableJobCleanup.
1 parent 2be3e87 commit 109dcaf

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

app/jobs/runtime/pollable_job_cleanup.rb

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,30 @@ def initialize(cutoff_age_in_days)
99
end
1010

1111
def perform
12-
old_pollable_jobs = PollableJobModel.where(Sequel.lit("created_at < CURRENT_TIMESTAMP - INTERVAL '?' DAY", cutoff_age_in_days))
1312
logger = Steno.logger('cc.background.pollable-job-cleanup')
13+
cutoff_condition = Sequel.lit("created_at < CURRENT_TIMESTAMP - INTERVAL '?' DAY", cutoff_age_in_days)
14+
15+
old_pollable_jobs = PollableJobModel.where(cutoff_condition)
1416
logger.info("Cleaning up #{old_pollable_jobs.count} Jobs rows")
1517
old_pollable_jobs.delete
18+
19+
# Job warnings have to be deleted explicitly, because
20+
# - we don't have a foreign key constraint and thus cannot use DELETE CASCADE, and
21+
# - we don't delete/destroy pollable job model objects one by one and thus cannot use hooks defined by the
22+
# 'destroy' association dependency between PollableJobModel and JobWarningModel.
23+
# Instead, we delete all expired jobs with a single SQL statement.
24+
#
25+
# By using the same cutoff condition based on 'created_at' for pollable jobs and job warnings, we ensure that
26+
# only job warnings are deleted where it is guaranteed that also the associated pollable job already has been
27+
# removed. This is due to the fact that job warnings are created after their associated pollable job, i.e.
28+
# pollable_job.created_at <= job_warning.created_at.
29+
#
30+
# On the other hand, it is not guaranteed that all associated job warnings are deleted for all pollable jobs
31+
# that have been removed during an execution of this cleanup job. But these leftovers will simply be removed
32+
# during the next job execution.
33+
old_job_warnings = JobWarningModel.where(cutoff_condition)
34+
logger.info("Cleaning up #{old_job_warnings.count} Job Warnings rows")
35+
old_job_warnings.delete
1636
end
1737

1838
def job_name_in_configuration

spec/unit/jobs/runtime/pollable_job_cleanup_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ module Jobs::Runtime
66
let(:cutoff_age_in_days) { 30 }
77
subject(:job) { PollableJobCleanup.new(cutoff_age_in_days) }
88
let!(:old_job) { PollableJobModel.create(created_at: (cutoff_age_in_days + 1).days.ago) }
9+
let!(:old_warning) { JobWarningModel.create(job: old_job, created_at: old_job.created_at, detail: 'some warning') }
910
let!(:new_job) { PollableJobModel.create(created_at: (cutoff_age_in_days - 1).day.ago) }
11+
let!(:new_warning) { JobWarningModel.create(job: new_job, created_at: new_job.created_at, detail: 'some warning') }
1012

1113
it { is_expected.to be_a_valid_job }
1214

@@ -23,6 +25,12 @@ module Jobs::Runtime
2325
it 'knows its job name' do
2426
expect(job.job_name_in_configuration).to equal(:pollable_job_cleanup)
2527
end
28+
29+
it 'also deletes associated warnings' do
30+
job.perform
31+
expect(JobWarningModel.find(id: old_warning.id)).to be_nil
32+
expect(JobWarningModel.find(id: new_warning.id)).to eq(new_warning)
33+
end
2634
end
2735
end
2836
end

0 commit comments

Comments
 (0)