Don't release connection back to pool early in load balancer

Simon Tomlinsonrequested to merge
stomlinson/lb-track-host-vs-conn-seperately into master

What does this MR do and why?

Fix for gitlab-com/gl-infra/production#20996 (closed)

!205885 (merged) added connnection status tracking for the new rails 7.2 with_connection block approach, but there were 3 problems:

  1. The state data was stored in thread-local vars shared between all the per-database load balancers, so the state could get confused.
  2. When releasing a connection back to the pool, !205885 (merged) also released the host, so the next query would use an unrelated replica. This broke sticking.
  3. The state tracking didn't work perfectly, particularly when several with_connection blocks nested without performing any actual queries.

To quickly fix all of these, we revert back to rails 7.1 style behavior and just keep the connection checked out for the whole request or sidekiq job. We were doing this before rails 7.2 in production, so it's safe.

Edited by Simon Tomlinson

Merge request reports