This is awesome 😍

Viewing the diff combined with #26909 makes the overall intent pretty clear: 2b2c096...3c785bd

My only concern with backporting this is that it moves the responsibility for checking out the thread-cached connection from the executor to the point where it's first used. Right now I can be sure that my application code will never block or raise trying to get a connection from the pool, but with this change that might start to happen.

Would it be insane to backport this but add a reference to ActiveRecord::Base.connection to the executor run hook, to preserve the previous behaviour on 5-0-stable?

My only concern with backporting this is that it moves the responsibility for checking out the thread-cached connection from the executor to the point where it's first used. Right now I can be sure that my application code will never block or raise trying to get a connection from the pool, but with this change that might start to happen.

This is exactly what we want to archive. The application should lazily connect to the database to be resilient to database failures.

Fair enough - I wasn't sure if that was intentional or a side-effect, since there are other benefits to toggling the query cache on checkin/checkout. But if the whole point is to make the connection lazily loaded, then obviously what I suggested makes no sense 😬

Yeah, it's a goal particularly because we're encouraging more places to use the executor (sidekiq, Active Job in general, Action Cable channels, thread-using user code in general)... but I think it's really surprising-bordering-on-bug that previous versions have eagerly allocated connections.

Anyone wanting to preserve the previous behaviour could call connection in a before_action (to get the 4.2-era "controllers only" version), or just add it as a to_run block.

Backported in 2b5d139