{"action":"created","issue":{"url":"https://api.github.com/repos/rails/rails/issues/26956","repository_url":"https://api.github.com/repos/rails/rails","labels_url":"https://api.github.com/repos/rails/rails/issues/26956/labels{/name}","comments_url":"https://api.github.com/repos/rails/rails/issues/26956/comments","events_url":"https://api.github.com/repos/rails/rails/issues/26956/events","html_url":"https://github.com/rails/rails/issues/26956","id":186953336,"number":26956,"title":"Rails5 concurrency, with_connection and executor.wrap interaction","user":{"login":"jrochkind","id":149304,"avatar_url":"https://avatars.githubusercontent.com/u/149304?v=3","gravatar_id":"","url":"https://api.github.com/users/jrochkind","html_url":"https://github.com/jrochkind","followers_url":"https://api.github.com/users/jrochkind/followers","following_url":"https://api.github.com/users/jrochkind/following{/other_user}","gists_url":"https://api.github.com/users/jrochkind/gists{/gist_id}","starred_url":"https://api.github.com/users/jrochkind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jrochkind/subscriptions","organizations_url":"https://api.github.com/users/jrochkind/orgs","repos_url":"https://api.github.com/users/jrochkind/repos","events_url":"https://api.github.com/users/jrochkind/events{/privacy}","received_events_url":"https://api.github.com/users/jrochkind/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-11-03T00:12:25Z","updated_at":"2016-11-03T01:10:59Z","closed_at":null,"body":"I am exploring concurrency involving new Rails5 API, and ActiveRecord. \r\n\r\nI am used to, in Rails 4, wrapping all my AR use in 'created' threads in `ActiveRecord::Base.connection_pool.with_connection`, to ensure the connection is checked back in at end of block.  This is also one of the recommended ways to use AR with concurrency in the [docs](http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html). \r\n\r\nIn Rails5, I learned from @matthewd about `Rails.application.executor.wrap`.  \r\n\r\nIf I try to do some concurrency wrapping in _both_ `executor.wrap` _and_ `connection_pool.with_connection`, it seems to deadlock. This is in a rake task depending on `:environment`, in an app where `cache_classes = eager_load = true`, to isolate AR issues from autoload issues. \r\n\r\n~~~ruby\r\n# DEADLOCKS\r\nfutures = 100.times.collect do |i|\r\n    Concurrent::Future.execute do\r\n      Rails.application.executor.wrap do\r\n        ActiveRecord::Base.connection_pool.with_connection do\r\n          SomeModel.find_or_create_by(value: i.to_s)\r\n        end\r\n      end\r\n    end\r\n  end\r\n~~~\r\n\r\nHowever, the reverse does not deadlock, an outer `with_connection` wrapping an `executor.wrap`\r\n\r\n~~~ruby\r\n  # DOES NOT DEADLOCK\r\n  futures = 100.times.collect do |i|\r\n    Concurrent::Future.execute do\r\n      ActiveRecord::Base.connection_pool.with_connection do\r\n        Rails.application.executor.wrap do\r\n          SomeModel.find_or_create_by(value: i.to_s)\r\n        end\r\n      end\r\n    end\r\n  end\r\n~~~\r\n\r\nThe trick is, the former deadlocking version seems quite likely to happen in real code to me. \r\n\r\n* Is the deadlock a bug, or as intended/designed?\r\n* Or is my use of `Rails.executor.wrap` in this context a bug for some reason? My understanding is that it was required/recommended in this case, but I might still not get it. \r\n* Are new instructions explaining the AR concurrency contract needed in [ConnectionPool](http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html), to take account of interactions with new Rails5 executor/interlock/reloader stuff?\r\n* @matthewd mentioned that he thought maybe `Rails.executor.wrap` made `connection_pool.with_connection` unneccesary, but I haven't confirmed it yet (it's difficult to confirm without monkey-patching or reachign into internals of AR, AR lacks good API for verifying if you have leaked connections or not). \r\n\r\nIn Rails4, `connection_pool.with_connection` was _the_ safe way to use ActiveRecord without having to know anything about your context. If you happened to be a in a thread that hadn't set up anything AR-related, it would work great, and was required. But if you were actually in a Rails request loop (action method), I believe it would still work fine. I think it even worked to nest `connection_pool.with_connection` in itself. So code could just do a `with_connection` without having to have non-local knowledge of what things lower in the call stack may have set up (or not). \r\n\r\nBut with the above behavior, if you write such code, and then use it (possibly indirectly) via a call chain, in a block that in an outer scope did `executor.wrap` -- deadlock. So the decision about how to handle ActiveRecord concurrency needs to be made at the same code level as the decision about how to deal with ActiveSupport issues handled by `executor.wrap`, in the same place, coordinated. This is unfortunate, and makes it harder to write re-usable AR code that is concurrency-safe. \r\n\r\nIn fact, I've run into a more complex issue in a real world app that I haven't figured out yet. To figure it out in the context of Rails5, I went back to some very basic demos to validate my understanding of what's going on and build basic building block knowledge. This was the first mystery I encountered. "},"comment":{"url":"https://api.github.com/repos/rails/rails/issues/comments/258045193","html_url":"https://github.com/rails/rails/issues/26956#issuecomment-258045193","issue_url":"https://api.github.com/repos/rails/rails/issues/26956","id":258045193,"user":{"login":"jrochkind","id":149304,"avatar_url":"https://avatars.githubusercontent.com/u/149304?v=3","gravatar_id":"","url":"https://api.github.com/users/jrochkind","html_url":"https://github.com/jrochkind","followers_url":"https://api.github.com/users/jrochkind/followers","following_url":"https://api.github.com/users/jrochkind/following{/other_user}","gists_url":"https://api.github.com/users/jrochkind/gists{/gist_id}","starred_url":"https://api.github.com/users/jrochkind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jrochkind/subscriptions","organizations_url":"https://api.github.com/users/jrochkind/orgs","repos_url":"https://api.github.com/users/jrochkind/repos","events_url":"https://api.github.com/users/jrochkind/events{/privacy}","received_events_url":"https://api.github.com/users/jrochkind/received_events","type":"User","site_admin":false},"created_at":"2016-11-03T01:10:59Z","updated_at":"2016-11-03T01:10:59Z","body":"Drat, and with just `connection_pool.with_connection`, I'm getting the old `Circular dependency detected while autoloading constant FeedUpdater`, which I now know probably means a violation of the autolock concurrency contract. \r\n\r\nThis happens even if I wrap my blocking waits in a `ActiveSupport::Dependencies.interlock.permit_concurrent_loads`. \r\n\r\nSo every setup I've found either blocks immediately, seems to leak AR connections, or results in the \"Circular dependency detected while autoloading\" error. \r\n\r\nI still am not sure which pattern is actually the \"correct\" one to use, so I don't know which exceptions are because I'm doing something wrong, or which are because of a bug, or even because what I'm doing is literally impossible and intentionally not supported in Rails5 somehow. (The latter seems implausible -- there's got to be a supported way to make some threads and do some AR operations in them, like there was in Rails4!)"}}
{"actor":{"display_login":"jrochkind"}}