Skip to content

Commit 29f52ca

Browse files
committed
Stop eager loading dependencies in dependency parsing workers
The includes(manifests: :dependencies) call loads all dependency rows into memory but parse_dependencies never reads them. Keep the manifests eager load since sync_manifest and delete_old_manifests use the loaded collection, but drop the nested :dependencies to avoid loading potentially millions of rows.
1 parent 65d6e10 commit 29f52ca

File tree

4 files changed

+12
-8
lines changed

4 files changed

+12
-8
lines changed

app/sidekiq/parse_dependencies_worker.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ class ParseDependenciesWorker
33
sidekiq_options queue: 'dependencies', lock: :until_executed, lock_expiration: 1.day.to_i
44

55
def perform(repository_id)
6-
Repository.find_by_id(repository_id).try(:parse_dependencies)
6+
Repository.includes(:manifests).find_by_id(repository_id).try(:parse_dependencies)
77
end
88
end

app/sidekiq/parse_tag_dependencies_worker.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ class ParseTagDependenciesWorker
33
sidekiq_options queue: 'dependencies', lock: :until_executed, lock_expiration: 1.day.to_i
44

55
def perform(tag_id)
6-
Tag.find_by_id(tag_id).try(:parse_dependencies)
6+
Tag.includes(:manifests).find_by_id(tag_id).try(:parse_dependencies)
77
end
88
end

test/workers/parse_dependencies_worker_test.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class ParseDependenciesWorkerTest < ActiveSupport::TestCase
2020
end
2121
end
2222

23-
should 'not eager load manifests and dependencies' do
23+
should 'eager load manifests but not dependencies' do
2424
host = create(:host)
2525
repository = create(:repository, host: host)
2626
manifest = create(:manifest, repository: repository)
@@ -37,8 +37,10 @@ class ParseDependenciesWorkerTest < ActiveSupport::TestCase
3737
ParseDependenciesWorker.new.perform(repository.id)
3838
end
3939

40-
eager_load_queries = queries.select { |q| q.include?('manifests') || q.include?('dependencies') }
41-
assert_empty eager_load_queries, "Should not eager load manifests or dependencies"
40+
manifest_queries = queries.select { |q| q.include?('manifests') }
41+
dependency_queries = queries.select { |q| q.include?('dependencies') }
42+
assert manifest_queries.any?, "Should eager load manifests"
43+
assert_empty dependency_queries, "Should not eager load dependencies"
4244
end
4345
end
4446
end

test/workers/parse_tag_dependencies_worker_test.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class ParseTagDependenciesWorkerTest < ActiveSupport::TestCase
2121
end
2222
end
2323

24-
should 'not eager load manifests and dependencies' do
24+
should 'eager load manifests but not dependencies' do
2525
host = create(:host)
2626
repository = create(:repository, host: host)
2727
tag = create(:tag, repository: repository)
@@ -38,8 +38,10 @@ class ParseTagDependenciesWorkerTest < ActiveSupport::TestCase
3838
ParseTagDependenciesWorker.new.perform(tag.id)
3939
end
4040

41-
eager_load_queries = queries.select { |q| q.include?('manifests') || q.include?('dependencies') }
42-
assert_empty eager_load_queries, "Should not eager load manifests or dependencies"
41+
manifest_queries = queries.select { |q| q.include?('manifests') }
42+
dependency_queries = queries.select { |q| q.include?('dependencies') }
43+
assert manifest_queries.any?, "Should eager load manifests"
44+
assert_empty dependency_queries, "Should not eager load dependencies"
4345
end
4446
end
4547
end

0 commit comments

Comments
 (0)