Skip to content

Fix DB connection check for Geo user routing

Catalin Irimie requested to merge cat-fix-geo-session-route-initialization into master

What does this MR do and why?

Overriding Geo routes should happen if the current node is a secondary.

However, simply checking connected? on the GeoNode model won't work because Rails releases the DB connection before reloading routes in the initializing phase, resulting in the GeoNode#connected? method being an unreliable indicator for the DB connectivity.

More background for this find in #340086 (comment 680585284), showing that commenting the connection release in Rails would've made GeoNode.connected? return true as well:

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 4f774bb7c4..86758d0c0c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -1083,7 +1083,7 @@ def active_connections?(role = ActiveRecord::Base.current_role)
       # and also returns connections to the pool cached by threads that are no
       # longer alive.
       def clear_active_connections!(role = ActiveRecord::Base.current_role)
-        connection_pool_list(role).each(&:release_connection)
+        # connection_pool_list(role).each(&:release_connection)
       end
 
       # Clears the cache which maps classes.

Since that's not possible/doable/feasible (and was just to confirm), checking if an active connection can be established is enough, and we already use this pattern in the AttrEncrypted initializer, and for the ActiveRecord data types as examples.

Screenshots or screen recordings

How to set up and validate locally

  1. Have GDK + Geo set up

  2. On the secondary GDK, run:

    ╰─>$ bin/rails runner "puts Rails.application.routes.url_helpers.new_user_session_path"
    /users/auth/geo/sign_in

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Catalin Irimie

Merge request reports