madelson / DistributedLock

A .NET library for distributed synchronization
MIT License
1.86k stars 192 forks source link

Postgres distributed lock using PgBouncer connection pooler #168

Closed tiago156 closed 6 months ago

tiago156 commented 1 year ago

Hello,

my postgres database is behind a PgBouncer connection pooler with transaction pooling and it does not allow prepared statements, so its failing as the Distributed Lock postgres implementation uses prepared statement:

https://github.com/madelson/DistributedLock/blob/00c0f4e926c586f5a1fabf869b79817a1cefbe46/src/DistributedLock.Postgres/PostgresDatabaseConnection.cs#L26

I was not able to find a way to disable it through configuration/parameters, do you have any recommendation on this case?

Thank you.

madelson commented 1 year ago

@tiago156 this is an interesting issue and not one I was aware of. Suporting this will required a change to the library. A few questions:

madelson commented 12 months ago

@tiago156 have you looked at https://stackoverflow.com/questions/20150503/prepared-statement-doesnt-exist ? Seems like possibly this is fixable with PG configuration?

P4rpleSky commented 10 months ago

@madelson Is there any reason why you support pg_advisory_lock() but not pg_advisory_xact_lock()? In continuation of our discussion in #175 , I read that with pgbouncer in transaction mode this is the only way to correctly set up advisory locks for PostgreSQL.

madelson commented 10 months ago

@P4rpleSky im not opposed to adding support for that and in fact we do support the equivalent capability for SQL server.

IIRC the reason I didn’t do it initially was that in SQL server I had bad experiences with transaction-based locks. At least on that database keeping transactions open for a long time can cause issues like blocking the transaction log from being flushed. I have no idea if the same is true for Postgres (of course, not all applications need to hold distributed locks for a long time).

can you point me to the resource where you were reading about using transaction-based locks with pg_bouncer?

P4rpleSky commented 10 months ago

@madelson Here are some useful links:

The problem with prepared statements is just the beginning. My comment above is related to this type of problem: if our application unexpectedly shuts down (or our database connection is closed) while having a session-based lock, we won't be sure that pgbouncer with transaction pooling won't make us a lock forever. With transaction-based locks, we'll guarantee that even in the situation described above, it will be unlocked because the transaction will end.

Maybe this problem is somehow solved by calling pg_sleep in your code, but I'm not sure about it. Can you tell me something about it, please? I will be very grateful!

droivan commented 8 months ago

Hi, @madelson, @tiago156!

Could you please provide an update on whether the issue with accessing the database via PgBouncer has been resolved? Thanks in advance for your assistance.

madelson commented 8 months ago

@P4rpleSky thanks for the great info. I think we should add transactional pg lock support to the library.

would you or @droivan (or anyone else) be interested in contributing?

madelson commented 6 months ago

@tiago156 @P4rpleSky @droivan I've implemented transaction-based locking in the upcoming release. Can anyone confirm that we do not need to disable prepared statements (per the original ask)?

rafaelkallis commented 5 months ago

@madelson I can confirm the prepared statement exception has disappeared once I used UseTransaction().

OlegUfaev commented 2 months ago

Also, you need to change max_prepared_statements to a non-zero value in pgbouncer configuration to enable Protocol-level prepared plans