leandromoreira / redlock-rb

Redlock is a redis-based distributed lock implementation in Ruby. More than 20M downloads.
BSD 2-Clause "Simplified" License
687 stars 80 forks source link

lock fail frequently #107

Open wikimo opened 2 years ago

wikimo commented 2 years ago

Scence:

Use redlock to lock product record when reduce the stock number. code like that:

product_lock = $redlock_client.lock(product.redlock_key, ENV.fetch('REDLOCK_TIME', 4000))
if product_lock
  product.stock -= buy_num
  product.save!

  $redlock_client.unlock(product_lock)
else
  Rails.logger.info("set_order_paid.redlock.fail product:#{product.id}")
end

When high concurrency level (like more than 2000 - 5000 request/s), product_lock fail frequently. We don't want to use the database lock, because it will make deadlock sometime. And we also use the retry setting, but not go well.

retry_count: 5
retry_delay: 500

What should we do ? Thanks for your help!

leandromoreira commented 2 years ago

Hi,

I don't know what's causing your race condition but I think you can.

# enforce TTL as integer always
product_lock = $redlock_client.lock(product.redlock_key, ENV.fetch('REDLOCK_TIME', 4000).to_i)

# also, it seems that you're using a global instance of $redlock_client, so double check if your
# rails server or your code is not under a race condition.
bguban commented 2 years ago

@wikimo I guess you have too small retry_count value and too big retry_delay. if you have 2000 requests in parallel then most likely 99% of attempts to take the lock are unsuccessful. 5 times is just not enough to be successful. Also, check retry_jitter value. It should be smaller than retry_delay.

@leandromoreira I feel that using retry_count, retry_delay and retry_jitter is inconvenient because you can't estimate the time the process will wait for lock until raising an exception. What do you think about using timeout and period instead/as alternatives?

  def try_lock_instances(resource, ttl, options)
      timeout = options[:timeout]
      period = period
      started_at = Time.now
      while Time.now - started_at < timeout
        sleep(rand(period))
        lock_info = lock_instances(resource, ttl, options)
        return lock_info if lock_info
      end

      false
    end

  redlock_client.lock('key', 4000, timeout: 10.seconds, period: 0.1..0.2)
wikimo commented 2 years ago

Thanks for your help! I will try them. @leandromoreira @bguban

fluke commented 1 year ago
# also, it seems that you're using a global instance of $redlock_client, so double check if your
# rails server or your code is not under a race condition.

@leandromoreira Sorry to bring this up so much later. But what is the recommended way of using the client. Is a constant preferred?