Skip to content

Commit 7154ff5

Browse files
committed
Runners: Prevent duplicate runner reservations
Previously, a race condition could occur and cause invalid runner records (i.e., two entries for the same contributor and execution environment). With a recent change in dc9d321, we validated these in Rails, but neither removed old records already present in the database nor did we add a unique constraint. This commit adds the missing parts and ensures the Ruby method retries in case of non-unique errors. Amends dc9d321 Fixes CODEOCEAN-19S Fixes CODEOCEAN-19T
1 parent 111d5bb commit 7154ff5

File tree

3 files changed

+38
-4
lines changed

3 files changed

+38
-4
lines changed

app/models/runner.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,18 @@ def self.management_active?
3838
def self.for(contributor, execution_environment)
3939
runner = find_by(contributor:, execution_environment:)
4040
if runner.nil?
41-
runner = Runner.create(contributor:, execution_environment:)
4241
# The `strategy` is added through the before_validation hook `:request_id`.
43-
raise Runner::Error::Unknown.new("Runner could not be saved: #{runner.errors.inspect}") unless runner.persisted?
42+
runner = Runner.create!(contributor:, execution_environment:)
4443
else
4544
# This information is required but not persisted in the runner model.
4645
runner.strategy = strategy_class.new(runner.runner_id, runner.execution_environment)
4746
end
4847

4948
runner
49+
rescue ActiveRecord::RecordNotUnique
50+
retry
51+
rescue ActiveRecord::RecordInvalid => e
52+
raise Runner::Error::Unknown.new("Runner could not be saved: #{e.inspect}")
5053
end
5154

5255
def copy_files(files, exclusive: true)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# frozen_string_literal: true
2+
3+
class DeleteInvalidRunnerReservations < ActiveRecord::Migration[8.0]
4+
def change
5+
# Keep the first occurrence of each unique combination of execution_environment_id, contributor_type, and contributor_id.
6+
# Delete all other duplicates from the `runners` table.
7+
8+
up_only do
9+
execute <<-SQL.squish
10+
WITH numbered_runner_reservations AS (
11+
SELECT id, execution_environment_id, contributor_type, contributor_id,
12+
ROW_NUMBER() OVER (
13+
PARTITION BY execution_environment_id, contributor_type, contributor_id
14+
ORDER BY created_at
15+
) AS row_num
16+
FROM runners
17+
)
18+
DELETE
19+
FROM runners
20+
WHERE id IN (
21+
SELECT id
22+
FROM numbered_runner_reservations
23+
WHERE row_num > 1
24+
);
25+
SQL
26+
end
27+
28+
remove_index :runners, %i[runner_id contributor_id contributor_type], unique: true, if_exists: true
29+
add_index :runners, %i[execution_environment_id contributor_id contributor_type], unique: true
30+
end
31+
end

db/schema.rb

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)