Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pg_advisory_lock_shared #3758

Open
bonsairobo opened this issue Feb 26, 2025 · 4 comments
Open

Support pg_advisory_lock_shared #3758

bonsairobo opened this issue Feb 26, 2025 · 4 comments
Labels
db:postgres Related to PostgreSQL enhancement New feature or request

Comments

@bonsairobo
Copy link

bonsairobo commented Feb 26, 2025

I have found these related issues/pull requests

Relates to #3495

Description

As documented here.

The PgAdvisoryLock documentation has some language suggesting that the API should support shared locking, but it does not currently support it.

Prefered solution

I'm guessing we just need separate acquire_shared and acquire_exclusive methods on PgAdvisoryLock.

Should be compatible with #3495

Is this a breaking change? Why or why not?

Depends. I'd like to have an explicitly named acquire_exclusive method, which is different from the current acquire name. We could just deprecate the old one.

But #3495 is breaking, and I'd prefer to have that merged first.

@bonsairobo bonsairobo added the enhancement New feature or request label Feb 26, 2025
@abonander
Copy link
Collaborator

The PgAdvisoryLock documentation has some language suggesting that the API should support shared locking, but it does not currently support it.

There isn't really any reason we don't besides YAGNI/scope creep. This is the first time anyone has explicitly requested this support.

Depends. I'd like to have an explicitly named acquire_exclusive method, which is different from the current acquire name. We could just deprecate the old one.

I'd prefer it to match the functions in Postgres. They use unsuffixed names for exclusive locks and the _shared suffix for non-exclusive: https://www.postgresql.org/docs/current/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS

@abonander abonander added the db:postgres Related to PostgreSQL label Feb 26, 2025
@abonander
Copy link
Collaborator

It also just occurred to me that we could also support transaction-scoped locks if we wanted to. They would take &mut Transaction<Postgres> and just not return a guard.

@bonsairobo
Copy link
Author

This is the first time anyone has explicitly requested this support.

Yea I see it as an alternative to explicit table locking which would potentially avoid the lifetime that Transaction carries. So you could acquire a shared lock to exclude some writer for the scope of multiple transactions without needing to carry that lifetime.

But I've also written the equivalent code that does carry the lifetime, and it's not terrible. So I mostly saw this as a relatively easy feature to add. But not something urgent.

It also just occurred to me that we could also support transaction-scoped locks if we wanted to. They would take &mut Transaction and just not return a guard.

Interesting. So you mean you would just call txn.acquire(&advisory_lock) and then assume the lock is held for the lifetime of the transaction?

@abonander
Copy link
Collaborator

Interesting. So you mean you would just call txn.acquire(&advisory_lock) and then assume the lock is held for the lifetime of the transaction?

Yes, though I think the methods would be more discoverable if they were on PgAdvisoryLock. Extension traits tend to be hard to discover without guide-level explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:postgres Related to PostgreSQL enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants