ClosureTree / with_advisory_lock

Advisory locking for ActiveRecord
http://closuretree.github.io/with_advisory_lock/
MIT License
629 stars 68 forks source link

pg_advisory_lock vs. pg_try_advisory_lock #80

Open AleksandrLeontev opened 1 year ago

AleksandrLeontev commented 1 year ago

Why do you use pg_try_advisory_lock and not just simple pg_advisory_lock which will avoid polling in SQL?

You can plan with this example:

  def with_advisory_lock(lock_id)
    execute("SELECT pg_advisory_lock(#{lock_id})")
    yield
  ensure
    execute("SELECT pg_advisory_unlock(#{lock_id})")
  end
seuros commented 1 year ago

pg_try_advisory_lock is non blocking.

You don't want to have processes blocked because they are waiting for the lock .

You can close this Issue if this answer your question.

AleksandrLeontev commented 1 year ago

What do you mean non blocking? Both try to obtains an exclusive session-level advisory lock.

pg_advisory_lock ( key bigint ) → void - Obtains an exclusive session-level advisory lock, waiting if necessary. pg_try_advisory_lock ( key bigint ) → boolean - Obtains an exclusive session-level advisory lock if available. This will either obtain the lock immediately and return true, or return false without waiting if the lock cannot be acquired immediately.

I just see that with pg_try_advisory_lock you need to run polling with jitter sleep(rand(0.05..0.15)) which does a lot of SQL queries (that's what I saw in logs, when had a concurrent requests with the duration of the procedure ~ 1 sec)

or do you mean that with pg_try_advisory_lock there will not be a long queue with strict sequence to specific lock?

To be short, what the benefit of pg_try_advisory_lock?

seuros commented 1 year ago

I see your point, we could have a option to select the behaviour.

Do you want to open a PR for it ?

AleksandrLeontev commented 1 year ago

I can try when will have a bit more free time. Currently just busy on the project where I wanted to apply with_advisory_lock gem :)

tmak commented 1 year ago

This is a critical bug with the PostgreSQL implementation, as far as I understand.

Currently, with_advisory_lock doesn't behave as documented at all.

The "What Happens" section in the README states:

  1. The thread will wait indefinitely until the lock is acquired.

But in reality, if another process already holds the lock, it will just skip the block silently without executing it.

tmak commented 1 year ago

...false alarm. My bad.

I didn't realize that lock_and_yield what checking for timeout_seconds == 0 and use yield_with_lock_and_timeout if timeout is nil.

But I agree with @AleksandrLeontev and his notion that the current approach of using sleep is suboptimal in terms of performance. I may be able to put up a PR.