Skip to content
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

> 2.0.0 .lock! no longer works #137

Closed
hirowatari opened this issue Jul 13, 2023 · 4 comments
Closed

> 2.0.0 .lock! no longer works #137

hirowatari opened this issue Jul 13, 2023 · 4 comments

Comments

@hirowatari
Copy link

hirowatari commented Jul 13, 2023

Minimal reproduction

lock_manager = Redlock::Client.new(["redis://redis:6379/1"])
retry_count = 50
retry_delay = 100 # 100 milliseconds
lock_time = 5_000 # 5 seconds
(0..9).map do |i|
  Thread.new do
    lock_manager.lock!('lockname', lock_time, retry_count: retry_count, retry_delay: retry_delay) do
      puts "Thread #{i} got lock"
      sleep 0.2
      puts "Thread #{i} released lock"
    end
  end
end.each(&:join)

Output in version 1.3.2

Thread 3 got lock
Thread 3 released lock
Thread 8 got lock
Thread 8 released lock
Thread 7 got lock
Thread 7 released lock
Thread 5 got lock
Thread 5 released lock
Thread 6 got lock
Thread 6 released lock
Thread 9 got lock
Thread 9 released lock
Thread 4 got lock
Thread 4 released lock
Thread 2 got lock
Thread 2 released lock
Thread 0 got lock
Thread 0 released lock
Thread 1 got lock
Thread 1 released lock

This is consistent and I have a test that looks similar to this (with some real work instead of sleeping)
that has been working in production and relying on this behaviour for a long time.

Output in version 2.0.2

Thread 4 got lock
Thread 4 released lock
Thread 8 got lock
Thread 8 released lock
Thread 0 got lock
Thread 0 released lock
Thread 2 got lock
Thread 9 got lock
Thread 3 got lock
Thread 1 got lock
Thread 7 got lock
Thread 2 released lock
Thread 9 released lock
Thread 3 released lock
Thread 6 got lock
Thread 1 released lock
Thread 5 got lock
Thread 7 released lock
Thread 6 released lock
Thread 5 released lock

Have I misunderstood how this is supposed to work, or is this a bug? I had expected only one thread to be able to get the lock at the same time.

@hirowatari
Copy link
Author

From this comment I see that perhaps I should change

lock_manager = Redlock::Client.new(["redis://redis:6379/1"])
<snip>
(0..9).map do |i|
  Thread.new do
    lock_manager.lock!('lockname', lock_time, retry_count: retry_count, retry_delay: retry_delay) do
<snip>

to

<snip>
(0..9).map do |i|
  Thread.new do
    Redlock::Client.new(["redis://redis:6379/1"]).lock!('lockname', lock_time, retry_count: retry_count, retry_delay: retry_delay) do
<snip>

Is that the correct fix?

I'm sorry but I don't quite understand why my usage is wrong but would appreciate any advice on this.

@jhuckabee
Copy link

@hirowatari This appears to be fixed in the latest version of the gem. Try the same thing on 2.0.4.

@taylorthurlow
Copy link

I was chasing this same problem and it was definitely the fact that a RedisClient instance is not thread-safe.

@hirowatari
Copy link
Author

I upgraded to 2.0.6 today and all is well. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants