Move registry import API calls out of transactions
The following discussion from !78613 (merged) should be addressed:
-
@10io started a discussion: Looking at this, I have an horrible doubt: the
after_transition
callbacks are executed outside the db transaction right?🤔 With this change
diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index c1fb2726d03..305819ecfca 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -152,6 +152,10 @@ class ContainerRepository < ApplicationRecord container_repository.migration_skipped_at = Time.zone.now end + after_transition any => :import_skipped do + Rails.logger.debug('#' * 90) + end + before_transition any => %i[import_done import_aborted] do # EnqueuerJob.enqueue perform_async or perform_in depending on the speed FF # To be implemented in https://gitlab.com/gitlab-org/gitlab/-/issues/349744
and executing:
ContainerRepository.first.skip_import(reason: :too_many_tags)
Here is the rails log:
ContainerRepository Load (0.6ms) SELECT "container_repositories".* FROM "container_repositories" ORDER BY "container_repositories"."id" ASC LIMIT 1 TRANSACTION (0.4ms) BEGIN ContainerRepository Exists? (0.6ms) SELECT 1 AS one FROM "container_repositories" WHERE "container_repositories"."name" = '' AND "container_repositories"."id" != 1 AND "container_repositories"."project_id" = 22 LIMIT 1 ContainerRepository Update (3.4ms) UPDATE "container_repositories" SET "updated_at" = '2022-02-08 14:07:06.436876', "migration_skipped_at" = '2022-02-08 14:07:06.427261', "migration_skipped_reason" = 2, "migration_state" = 'import_skipped' WHERE "container_repositories"."id" = 1 ########################################################################################## TRANSACTION (7.7ms) COMMIT
😿 (theafter_transition
is within the transaction)We will need to get this sorted but I think that this MR is big enough already. What about opening a issue and we handle it right after this one?
State_machine observers may be able to solve this problem: #after_import
and #after_pre_import
. Otherwise we can disable transactions for the entire state machine, but that is an undesired solution.