Skip to content

Fix sidekiq transaction check to be decomposition aware

Thong Kuah requested to merge forbiq_sidekiq_txn_fixes into master

What does this MR do and why?

  • Fix sidekiq transaction check to be decomposition aware
  • Add missing spec coverage

Related issue: #346697 (closed)

Screenshots or screen recordings

Before vs after

$ GITLAB_USE_MODEL_LOAD_BALANCING=1 bin/rspec spec/initializers/forbid_sidekiq_in_transactions_spec.rb
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Test environment set up in 6.325628 seconds
...F

Failures:

  1) Sidekiq::Worker forbids queue sidekiq worker in a Ci::ApplicationRecord transaction
     Failure/Error: expect { worker_class.perform_async }.to raise_error(Sidekiq::Worker::EnqueueFromTransactionError)
       expected Sidekiq::Worker::EnqueueFromTransactionError but nothing was raised
     # ./spec/initializers/forbid_sidekiq_in_transactions_spec.rb:35:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:126:in `block in write_using_load_balancer'
     # ./lib/gitlab/database/load_balancing/load_balancer.rb:112:in `block in read_write'
     # ./lib/gitlab/database/load_balancing/load_balancer.rb:172:in `retry_with_backoff'
     # ./lib/gitlab/database/load_balancing/load_balancer.rb:110:in `read_write'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:125:in `write_using_load_balancer'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:77:in `transaction'
     # ./lib/gitlab/database.rb:261:in `block in transaction'
     # ./lib/gitlab/database.rb:260:in `transaction'
     # ./spec/initializers/forbid_sidekiq_in_transactions_spec.rb:34:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:412:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:399:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:31:in `with_raw_context'
     # ./spec/spec_helper.rb:399:in `block (2 levels) in <top (required)>'
     # ./spec/support/database/query_analyzer.rb:9:in `block (3 levels) in <main>'
     # ./lib/gitlab/database/query_analyzer.rb:42:in `within'
     # ./spec/support/database/query_analyzer.rb:9:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:102:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:56:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:102:in `block (2 levels) in <main>'

Finished in 7.7 seconds (files took 10.21 seconds to load)
4 examples, 1 failure

Failed examples:

rspec ./spec/initializers/forbid_sidekiq_in_transactions_spec.rb:33 # Sidekiq::Worker forbids queue sidekiq worker in a Ci::ApplicationRecord transaction
$ GITLAB_USE_MODEL_LOAD_BALANCING=1 bin/rspec spec/initializers/forbid_sidekiq_in_transactions_spec.rb
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Test environment set up in 6.248666 seconds
....

Finished in 7.66 seconds (files took 9.98 seconds to load)
4 examples, 0 failures

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Thong Kuah

Merge request reports