Commit 4d3cda69 authored by Kamil Trzciński's avatar Kamil Trzciński

Fix usage of threads for pages migration

The main thread already is one that is `running`.
The child threads should always wrap only operation
instead of the whole thread. This ensures that wrapping
operation can hold a lock when loading code.

In a worst case scenario it will result that initially till
all threads do load all relevant code it will be run effectively
in sequence. This will resolve once all dependencies get loaded
lazilly.
parent fd7ffb17
......@@ -24,9 +24,7 @@ module Pages
@queue.close
@logger.info("Waiting for threads to finish...")
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
threads.each(&:join)
end
threads.each(&:join)
{ migrated: @migrated, errored: @errored }
end
......@@ -34,8 +32,8 @@ module Pages
def start_migration_threads
Array.new(@migration_threads) do
Thread.new do
Rails.application.executor.wrap do
while batch = @queue.pop
while batch = @queue.pop
Rails.application.executor.wrap do
process_batch(batch)
end
end
......@@ -51,6 +49,11 @@ module Pages
end
@logger.info("#{@migrated} projects are migrated successfully, #{@errored} projects failed to be migrated")
rescue => e
# This method should never raise exception otherwise all threads might be killed
# and this will result in queue starving (and deadlock)
Gitlab::ErrorTracking.track_exception(e)
@logger.error("failed processing a batch: #{e.message}")
end
def migrate_project(project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment