activerecord-session_store icon indicating copy to clipboard operation
activerecord-session_store copied to clipboard

SqlBypass holds onto old connection, resulting in ActiveRecord::ConnectionNotEstablished: connection is closed error

Open oliverguenther opened this issue 3 years ago • 0 comments

We are using the SqlBypass class to get and store sessions and with threaded Puma in production and a limited database connection pool size, the connection that is memoized in https://github.com/rails/activerecord-session_store/blob/master/lib/active_record/session_store/sql_bypass.rb#L54 is eventually no longer active and got replaced, resulting in the following errors:

[32] pry(#<HomescreenController>)> Sessions::SqlBypass.find_by_session_id "some session id"
ActiveRecord::ConnectionNotEstablished: connection is closed
from /home/oliver/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.1.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'
Caused by PG::ConnectionBad: connection is closed
from /home/oliver/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.1.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'
Caused by ActiveRecord::ConnectionNotEstablished: connection is closed
from /home/oliver/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.1.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'
Caused by PG::ConnectionBad: connection is closed
from /home/oliver/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.1.3.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'


[33] pry(#<HomescreenController>)> ActiveRecord::Base.connection.active?
=> true
[34] pry(#<HomescreenController>)> Sessions::SqlBypass.connection.active?
=> false

I'm wondering why the connection gets memoized in the first place? Alternative, I could provide a PR that ensures that we always use ActiveRecord::Base.connection_pool.with_connection.

oliverguenther avatar Apr 12 '21 12:04 oliverguenther