Concurrent load interlock (rm Rack::Lock)#17102
Conversation
d0e4b18 to
4818a20
Compare
|
Are you aiming for a proper fix to #15089? Why this might not work: # foo.rb
class Foo # L1
# L2
def self.foo; end # L3
end # L4Initial state:
Both threads attempt to
Thoughts 😞? Another idea: upon entering /cc @fxn. |
|
@thedarkone thread A can't obtain the exclusive lock (and thus start loading |
|
Alright, then I can't think of a reason why this wouldn't work! I have maybe 2 suggestions:
btw: I'm seriously impressed, I never thought making |
|
+1 on being impressed. If we do not discover any devil in the details, this is going to be a real win. |
|
@matthewd are you thinking in moving this forward? I believe this is the best time to do it. |
|
👍 makes sense for 5.0. |
|
I'm thinking - this will not work if user decides to spin up his own threads (because we will not be able to track whether they have entered "app space"). Also, how would this work with background jobs? |
|
For background jobs: the job-runner would wrap the job-doing code just as ActionDispatch does for requests. (And as it presumably already does for the AR conn pool, for example.) If the user spins up their own thread (and doesn't opt in to the interlock), however... yeah, they're going to be in danger of seeing a half-loaded constant. Avoiding that sounds expensive. 😕 |
We don't need to fully disable concurrent requests: just ensure that loads are performed in isolation.
We can't actually lean on Rack::Lock's implementation, so we'll just copy it instead. It's simple enough that that's not too troubling.
Concurrent load interlock (rm Rack::Lock)
|
This is so awesome. I think we can modify Sidekiq to reload job code in development mode now! |
Instead of forcing
Rack::Lockwheneager_loadandcache_classesaren't on, just prevent actual conflicts.We achieve this by means of a (reentrant) shared-exclusive lock: multiple running concurrent requests are fine, but any load activity must be performed in isolation.