pg_advisory_unlock_all run too often

[moser]

As already stated in #46, we have a problem with the unlock all being run too often.

Originally, I thought that it was a problem with running it on connections that never were used to acquire a lock. I "solved" that by using a separate engine/connection pool for locking.

I looked in to the issue again because I saw an increase in the number of unlock all calls lately. I found another problem in this investigation.

In our application, we have multiple parts that use locking. For simplicity's sake, let's say we have a Locker instance for each of our work queues. We added a new type of queue lately and therefore roughly doubled the the number of Locker instances. We expected to have double the amount of unlock all calls. We observed a way larger increase though.

Consider the following example:

import random
import pals
import sqlalchemy as sa

engine = sa.create_engine("postgresql://...")

N_LOCKERS = 3
N_LOCKS = 5
lockers = []

for i in range(N_LOCKERS):
    locker = pals.Locker(f"app_{i}", create_engine_callable=lambda: engine)
    lockers.append(locker)

for i in range(N_LOCKS):
    locker = lockers[random.randint(0, N_LOCKERS - 1)]
    lock = locker.lock("somelock")
    print("Acquiring lock", locker.app_name)
    lock.acquire()
    lock.release()
    print("Released lock", locker.app_name)

This generates the following output (with a print statement on the on_conn_checkin function):

Acquiring lock app_2
Connection checkin event fired for Locker app_0
Connection checkin event fired for Locker app_1
Connection checkin event fired for Locker app_2
Released lock app_2
Acquiring lock app_1
Connection checkin event fired for Locker app_0
Connection checkin event fired for Locker app_1
Connection checkin event fired for Locker app_2
Released lock app_1

For any lock we see 3 checkin events (and thus 3 unlock all calls). Now adding more queues to our application roughly doubled N_LOCKERS and N_LOCKS, so we saw a roughly 4 times more unlock all calls.

I see two ways forward:

1. I pick up on Only run pg_advisory_unlock_all if necessary. #46 again, vendor it and add tests
2. I implement another solution that will only register the on_conn_checkin handler once on the same engine object.

Which one of the two do you find more promising?

[rsyring]

Which one of the two do you find more promising?

You seem to be using this project more intensely than I do. I mean, we use it, but not at a scale where any of this matters. Which makes it hard for me to have a sense of which is the right way forward.

Which do you find more promising?

Also, would you like to become a maintainer of the project? Sounds like you could move things forward in a more reasonable time frame than you can with me being in the middle. :)

[moser]

OK, I will do an experiment with both solutions and report back. I can't really decide what is more elegant and failure tolerant without seeing it :-)

As for your question about becoming a maintainer: Sure, I am happy to help.

[rsyring]

What if we addressed this completely differently.

We are basically contemplating two use cases of the library. One where transaction volume is relatively low and running pg_advisory_unlock_all() as the library currently does has a negligible impact on the app. The other is where the connection/usage volume is higher and the impact of pg_advisory_unlock_all() matters.

The thing is, pg_advisory_unlock_all() is solely defensive. When I originally wrote this, one of main main concerns was avoiding the possibility of accidentally keeping a lock. But, if the library is being used correctly, which is easy to do if you stick with the context managers, then you should never accidentally keep a lock.

So, what if we just added a class argument that, when set, would disable the defensive pg_advisory_unlock_all() call completely?

[moser]

@rsyring You are right, it is a lot simpler to disable it completely. See #50