-
Couldn't load subscription status.
- Fork 17
Support Rails 6 multiple database connections. #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| ActiveRecord::Base.clear_all_connections! | ||
| if ActiveRecord::VERSION::MAJOR >= 6 | ||
| if should_clear_all_connections? | ||
| ActiveRecord::Base.connection_handlers.each_value do |connection_handler| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connection_handlers will raise NotImplementedError if legacy_connection_handling = false:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review.
Probably fixed to work with Rails 6.1.
Do you know a better way to get a list of roles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can refer to the original connection management code:
https://github.com/rails/rails/blob/v6.1.0/activerecord/lib/active_record/query_cache.rb#L42-L56
| private | ||
|
|
||
| def clear_connections | ||
| if @ar_version >= AR_VERSION_6_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can check the condition in definition time rather than runtime, checking it in runtime have a little overhead for the clear_connections method call.
|
|
||
| def legacy_connection_handling? | ||
| begin | ||
| ActiveRecord::Base.legacy_connection_handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No raise NoMethodError since Rails 6.1 have ActiveRecord::Base.legacy_connection_handling.
|
@kamipo Removed the process when |
073114d to
0ece604
Compare
| if should_clear_all_connections? | ||
| ActiveRecord::Base.connection_handler.all_connection_pools.each(&:disconnect!) | ||
| else | ||
| ActiveRecord::Base.connection_handler.all_connection_pools.each do |pool| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct for legacy_connection_handling = false but is not always correct for legacy_connection_handling = true.
|
@kamipo For example, if you set The connection is out of the management of I also need to run |
…onnection_handling is true.
|
@kamipo The problem is that one of the three cases is wrong or ActiveRecord::Base#connection is not returning the connection_pool in connection_handlers. (1): Multiple databases should not be configured when legacy_connection_handling is enabled. |
EN)
Fixed to disconnect all connection_handlers in ActiveRecord 6 and later versions.
JA)
ActiveRecord 6では
ActiveRecord::Base#clear_all_connections!及びActiveRecord::Base#clear_active_connections!はActiveRecord::Base#default_connection_handlerに処理が移譲されるため、単一のデータベース接続を扱う場合は正常に動作しますが、複数のデータベース接続を扱う場合はdefault_connection_handler以外のconnection_handlerの接続が切れません。
(多くの場合default_connection_handlerはPrimaryデータベース, それ以外のconnection_handlerはReplicaデータベースへ接続されています)
ActiveRecord::Base#connection_handlersはActiveRecord 6で追加されたため、バージョン6以降とそれ以前で処理を分岐し、複数のデータベース接続においてもコネクションを切断出来るよう修正しました。