Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent background workers from holding ActiveRecord connections #1320

Merged
merged 5 commits into from
Mar 9, 2021

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Mar 7, 2021

When the SDK's background workers serializing events for a Rails application, it could implicitly trigger database queries by serializing data that contain ActiveRecord models/records (e.g. when serializing breadcrumbs). And by triggering the queries, those workers will also obtain database connections from the connection pool and hold them forever. This will later cause the application to run out of available connections.

This PR solves the issue by using ActiveRecord::ConnectionPool#with_connection method to force background workers to release any connection they might obtain during the serialization work.

@st0012 st0012 added this to the 4.3.0 milestone Mar 7, 2021
@st0012 st0012 self-assigned this Mar 7, 2021
@st0012 st0012 added this to In progress in 4.x via automation Mar 7, 2021
@jakeonfire
Copy link
Contributor

it appears as though this strategy requires that the background worker get a connection to enter the block. in our app with 1 db connection per unicorn the background worker is erroring out:

exception happened in background worker: could not obtain a connection from the pool within 6.000 seconds (waited 6.000 seconds); all pooled connections were in use
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:202:in `block in wait_poll'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:193:in `loop'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:193:in `wait_poll'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:154:in `internal_poll'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:278:in `internal_poll'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:148:in `block in poll'
/usr/local/lib/ruby/2.6.0/monitor.rb:235:in `mon_synchronize'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:158:in `synchronize'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:148:in `poll'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:717:in `acquire_connection'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:490:in `checkout'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:364:in `connection'
/usr/local/bundle/gems/activerecord-5.0.7.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:395:in `with_connection'
/usr/local/bundle/gems/sentry-rails-5.3.0/lib/sentry/rails/background_worker.rb:7:in `_perform'
/usr/local/bundle/gems/sentry-ruby-core-5.3.0/lib/sentry/background_worker.rb:54:in `block in perform'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:352:in `run_task'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:343:in `block (3 levels) in create_worker'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `loop'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `block (2 levels) in create_worker'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:333:in `catch'
/usr/local/bundle/gems/concurrent-ruby-1.1.10/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:333:in `block in create_worker'

if the breadcrumbs do indeed need to query the DB then we should either bump up our connection limit to 2 or force synchronous behavior. but it seems like the option to have async without a db connection is not possible.

@st0012
Copy link
Collaborator Author

st0012 commented May 3, 2022

I think your question is legit and with the current background worker design, it will need 2 connections per unicorn worker to work smoothly.

But to have a more detailed discussion, can you also share your application's setup (server, worker) and resource limitation (e.g. you're using heroku and your DB plan only allows 20 connections)?

@st0012
Copy link
Collaborator Author

st0012 commented May 3, 2022

Actually, it'd be great if you can open an issue report and we can discuss there. It'll be easier for other people to chime in too.

@jakeonfire
Copy link
Contributor

jakeonfire commented May 4, 2022

done: #1808. if you have edits for generalizing the title please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.x
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants