Skip to content

Fix Sidekiq crashing when GITLAB_LOG_LEVEL set to debug

Stan Hurequested to merge
sh-backport-sidekiq-fix-17-3 into 17-3-stable-ee

What does this MR do and why?

This picks a single fix in !163593 (merged) for 17-3-stable-ee.

The introduction of GITLAB_LOG_LEVEL setting in Sidekiq for GitLab 17.0 via !149790 (merged) caused an unexpected problem when debug mode were set: it would lead Sidekiq into an infinite recursion, ultimately resulting in a SystemStackError and causing it to crash.

At startup, Sidekiq logs a debug message with a @config Hash that references Sidekiq::Rails::Reloader. Sidekiq::Rails::Reloader has a reference to Gitlab::Application via the @app instance variable.

When #to_json is called, the @app gets called, when eventually causes #to_json to be called on @config again, leading to an circular reference.

Sidekiq 7.2.4 fixed this problem via https://github.com/sidekiq/sidekiq/pull/6198 by implementing a #to_hash method in Sidekiq::Rails::Reloader, which avoids serializing @app.

This commit picks the fix in Sidekiq 7.2.4 for our vendored Sidekiq 7.1.6 because it is straightforward fix. Upgrading to Sidekiq 7.2.4 (!163593 (merged)) is a bit more involved and not necessary here.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  1. git checkout origin/17-3-stable-ee
  2. Run Sidekiq by hand: GITLAB_LOG_LEVEL=debug bin/sidekiq-cluster -c 10 "*".
  3. This should crash:
bundler: failed to load command: sidekiq (/Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/bin/sidekiq)
/Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:107:in `binary': stack level too deep (SystemStackError)
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:109:in `visit_CAT'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:103:in `visit'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:162:in `visit'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:117:in `unary'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:119:in `visit_GROUP'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:103:in `visit'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:162:in `visit'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/actionpack-7.0.8.4/lib/action_dispatch/journey/visitors.rb:107:in `binary'
         ... 10418 levels...
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/bundler-2.5.11/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/lib/ruby/gems/3.2.0/gems/bundler-2.5.11/exe/bundle:20:in `<top (required)>'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/bin/bundle:25:in `load'
        from /Users/stanhu/.asdf/installs/ruby/3.2.4/bin/bundle:25:in `<main>'
  1. Now checkout this branch and repeat the same test. It should boot fine.

MR acceptance checklist

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

  • This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch.
  • The MR that fixed the bug on the default branch has been deployed to GitLab.com (not applicable for documentation or spec changes).
  • This MR has a severity label assigned (if applicable).
  • Set the milestone of the merge request to match the target backport branch version.
  • This MR has been approved by a maintainer (only one approval is required).
  • Ensure the e2e:test-on-omnibus-ee job has either succeeded or been approved by a Software Engineer in Test.

Note to the merge request author and maintainer

If you have questions about the patch release process, please:

Edited by Sylvester Chin

Merge request reports