You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At GitHub we're trying to switch over from the GitHub
activerecord-trilogy-adapter to the one in Rails. One difference between
the adapters is that the GitHub one did not have the `#select_all`
method that abandons multi results.
This wouldn't be a problem, except we also have more aggressive query
retries. This led to an issue because the `super` call in `#select_all`
leads to a nested `#with_raw_connection` call. If we experienced
transient connection errors during the query, the inner
`#with_raw_connection` could reconnect leaving the outer block with a
closed connection. In that case, calling `#more_results_exist?` results
in a `Trilogy::ConnectionClosed Attempted to use closed connection`.
That's the scenario described in this comment
https://github.com/rails/rails/blob/7e735451a71ca8c9e7509096f13d00a82adba4f9/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L981-L983
Moving the call to `super` out of the `#with_raw_connection` block will
avoid the nesting and fix the issue. We prefer that solution over
detecting nested `#with_raw_connections` and preventing reconnects when
nested, since in our case we actually do want to reconnect and retry.
We're hoping this change is ok even though it's not strictly necessary
for general Rails users. We don't believe it has any downsides.
We did not add a test for this case since it's not possible to occur in
Rails itself. It's specific to custom GitHub code.
Co-authored-by: Daniel Colson <[email protected]>
0 commit comments