Avoid replica connection checkout/checkin for every read query

[bmak-lightspeed]

We migrated from Makara to this project, but we found that reads were considerably slower than Makara.

After looking at the code we realized that PrimaryReplicaProxy currently checks out and checks in a replica connection for every read query. Each checkout can cause Active Record to verify the connection by issuing a ; query, adding unnecessary database round trips and latency.

Use replica_pool.connection or replica_pool.lease_connection instead, allowing Rails to manage connection cleanup and check-in at the end of the request or Sidekiq job. This follows the same mechanism Rails uses when obtaining the primary connection, leveraging Rails' existing primary-connection check-in behavior to automatically check in the replica connection as well.

This avoids per-query connection verification while preserving existing connection-pooling behavior and significantly improves read-heavy request performance.

• connection (https://api.rubyonrails.org/v7.1.6/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html#method-i-connection)
• lease_connection (https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html#method-i-lease_connection)

[mateuscruz]

Hello @bmak-lightspeed. Thanks for the contribution. Can you share benchmarks that support your claim, for documentation purposes?

[bmak-lightspeed]

Hi @mateuscruz,

Sorry for the delay (I was on vaca).

Disabled

I don't have benchmarks, but here's picture of a flame graph of a request when this patch is NOT enabled. You can see that every read query makes a "PG active? probe" query which translates to a ; query to the db.

Normally queries sent to the PG gem create a span, but since the query is just ; it does not create a span. However, it is still incurring a round trip cost to to go the db and back to make this query.

(I had to redact some info)

Enabled

When this change is enabled you can see that only the first read query incurs the checkout cost:

Fwiw this change does assume the user is using Rails which automatically returns the connection back to the pool after each request / job. This could be a separate adapter if we want to also maintain support for non Rails web frameworks.

[bmak-lightspeed]

fwiw I'd like to backfill this into the v9 stable branch if possible

[mateuscruz]

@bmak-lightspeed what Ruby and Rails versions is your project running on?

[mateuscruz]

Ugh. Github does not sign rebased commits anymore. @bmak-lightspeed can you rebase locally?

[bmak-lightspeed]

Just force pushed with your fix.
We're currently using 7.1.6 (yeah, we know we need to update and we're working on it)
It would be nice for this to work for the version we're using atm though.

[mateuscruz]

@bmak-lightspeed some unit tests need to be fixed there.

It would be nice for this to work for the version we're using atm though.

I can backport it.

[bmak-lightspeed]

As far as I can tell the tests seem to run fine now, but there's some issue with the coverage report. I believe that is unrelated to the change.