Allow constants to load from DRb threads #10038
Conversation
https://bugzilla.redhat.com/show_bug.cgi?id=1354054 use ActiveSupport::Dependencies.interlock.permit_concurrent_loads to allow for Automate Methods/Drb to load symbols
| # This was added because in RAILS 5 PUMA sends multiple requests | ||
| # to the same processa and our DRb server implementation for Automate | ||
| # Methods is not thread safe. The permit_concurrent_loads allows the | ||
| # DRb requests which run in different threads to load symbols. |
There was a problem hiding this comment.
@mkanoor I don't understand the wording of "load symbols", "allow symbols to load". Is this strictly about loading constants via rails autoload? I'm fine with this change, I just don't understand the language of the PR.
There was a problem hiding this comment.
@mkanoor Do you mean to "autoload models"? "load symbols" also doesn't make sense to me.
There was a problem hiding this comment.
@Fryguy @jrafanie
If there are constants missing Rails has to load them when the Drb thread is running. Without the permit_concurrent_loads the Rails thread would get blocked and the process hangs. So these are not really auto-load, if we had loaded them all upfront we wouldn't have seen this issue, these constants come about when some of the constants are missing and Rails tries to load them.
There was a problem hiding this comment.
The term "Rails const_missing" is synonymous with "Rails autoload" ... "load symbols" is confusing because a symbol is like :symbol...those don't "load"
There was a problem hiding this comment.
So these are not really auto-load, if we had loaded them all upfront we wouldn't have seen this issue, these constants come about when some of the constants are missing and Rails tries to load them.
Right, @mkanoor. "Loading constants" is clearer than "loading symbols".
The key point is if you don't eager load all constants to be used in your automate "methods/dialogs/things" and rely on Rails to autoload those missing constants, you could encounter a race condition with multiple threads, where the holders of two locks cause a deadlock:
ActiveSupport::Dependencies.interlock@deliver_mutex
This permit_concurrent_loads allows constant loading by yielding 'share' locks to avoid deadlocks:
+ # Temporarily give up all held Share locks while executing the
+ # supplied block, allowing any +compatible+ exclusive lock request
+ # to proceed.
From here
mkanoor
changed the title
|
Checked commits mkanoor/manageiq@a1656fc~...d0aefaf with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
|
LGTM |
|
👍 |
Allow constants to load from DRb threads (cherry picked from commit 6bad915)
|
Darga backport details: |
Purpose or Intent
Links
Steps for Testing/QA