Postgres: Add support for transaction-scoped advisory locks with external DB connections

[Tzachi009]

Hi, I noticed that since version 1.1.0 of DistributedLock.Postgres, the library supports transaction-scoped advisory locks, if you choose to use them. I understand that it was added as part of the following issue - https://github.com/madelson/DistributedLock/issues/168, as it turned out that using prepared statements/session-scoped advisory locks with PgBouncer is problematic.

The problem is that we must use PgBouncer, but in our case the app uses the library by passing an already established DB connection with a transaction, in order to have a better integrity of the lock. When I looked into the code of the library, I realized that the transaction locks are not supported for external connections:

I wonder why that is. Is the issue that it will be the user's responsibilty to explicitly release the lock by commiting/rollbacking the transaction? Will it be possible to add support for such scenarios?

Thanks.

[madelson]

@Tzachi009 thanks for your interest in the library. IIRC, the issue is that Postgres transaction-scoped locks can’t be released without ending the transaction. So either we’re killing the users transaction for them or releasing the lock doesn’t work properly. I don’t like either of those options from an API perspective.

are you hoping to run queries on the same transaction that holds the lock or is it more just the convenience of injecting the connection in your setup?

[Tzachi009]

Thank you for the quick response @madelson.

Yes, it is about running queries on the same transaction that holds the lock, and being able to use PgBouncer in such a case. In the critical path of the app, I acquire locks and then invoke several commands in Postgres (in a transaction) under the locks. Performance is very important in the critical path, so using the same transaction that holds the lock, instead of creating a new one seems reasonable. Moreover, this solution improves the integrity of the lock by using the same DB connection and transaction.

I understand your point regarding the API perspective, but if the API already allows the user to pass an external connection that is not under its control, then why not let the user who created the transaction have the responsibilty to end it?

Maybe another option could be to let the user configure whether the library or the user will be the responsible party of the transaction (via the PostgresConnectionOptionsBuilder class or when passing the DB connection to the API).

[Tzachi009]

Hi @madelson, may we continue discussing the suggestion or are you entirely opposed to the idea?

My team and I would really like to see this feature implemented, and I don't mind contributing to the library myself.

[madelson]

@Tzachi009 I think I can get behind the idea that in Postgres if you create a lock with an explicit transaction and then release the lock, it disposes the transaction. It's a little weird, but less weird than having release just noop. The user will be responsible for releasing the lock after committing the transaction.

Given those caveats, I do think it is worth thinking about what you are really getting from the library in this case vs. just issuing the same SQL commands yourself.

I'd be happy to have to you take a crack at this. Two implementation challenges I foresee: 1) We'll need to figure out a plan for test coverage, since the main suite that most locks run against probably won't work under the "one time use" constraint described above 2) PostgresAdvisoryLock.cs today makes use of save points and session variables to control the lock command; not sure how well any of that will behave with the added constraint of an external transation. I do think it is vital that we not pollute the transaction with overrides to session variables.

[Tzachi009]

Thank you, I will start to look into it soon and will see how it goes.

Just a few points regarding what you said:

• The transactional advisory locks are released automatically when the transaction is commited or rollbacked (https://www.postgresql.org/docs/current/explicit-locking.html#ADVISORY-LOCKS), therefore that's the only the user will have to do in order to release the lock, while the library won't be able to do so.
• As a user, I can still see the advatange of using the library in such a case, since it still consolidates everything regarding the management of the lock.
• I need to get more familiar with the tests, but I will check how to test this specific scenario. I will also try to check what we can do regarding save points/session variables.

[madelson]

@Tzachi009 another avenue I was considering is just offering this as a static method that acquires the lock on an existing transaction, returning no result. That would make it clear that it is the users responsibility to release.

[Tzachi009]

It may be a little weird to have a specific method just for this scenario, but that's a viable option, although I still think that when you send an external connection to the library, it is pretty clear that it is the user's responsibility to manage it, including a transaction, if one exists, therefore for a start I think we should still try to make it work with the existing API.

You said that the static method won't return a result, but what would you expect to happen if the lock can't be acquired? Should such API offer both Acquire() and TryAcquire() methods? Only for Postgres?