Use advisory locks to guard against concurrent pgslice invocations#16
Use advisory locks to guard against concurrent pgslice invocations#16
pgslice invocations#16Conversation
| override async perform(pgslice: Pgslice): Promise<void> { | ||
| await pgslice.start(async (tx) => { | ||
| await this.context.pgslice.disableMirroring(tx, { table: this.table }); | ||
| await pgslice.disableMirroring(tx, { table: this.table }); |
There was a problem hiding this comment.
Yeah, I should actually clean this up: There's two ways in a command to get access to the Pgslice instance (context or the perform parameter) but we should probably just have one.
I can look at that later.
| ): Promise<boolean> { | ||
| const result = await connection.one( | ||
| sql.type(z.object({ acquired: z.boolean() }))` | ||
| SELECT pg_try_advisory_lock(${key}) AS acquired |
There was a problem hiding this comment.
An alternative would be row based locks on a dedicated table which might have easier debugging, but I can see why session-level advisory locks make more sense given fill's batch processing.
There was a problem hiding this comment.
True, and if we end up having issues with this advisory locking strategy, maybe we even do that. But it's nice to have pgslice not depend on a specific schema in that way.
src/pgslice.ts
Outdated
|
|
||
| async #acquireLock( | ||
| connection: CommonQueryMethods, | ||
| table: string, |
There was a problem hiding this comment.
Nit: Worth taking a Table arg directly for consistency with withLock?
There was a problem hiding this comment.
Good idea! I'll make that change.
Use PostgreSQL session-level advisory locks (pg_try_advisory_lock) to prevent concurrent operations on the same table. Lock keys are generated from the combination of table name and operation name via hashtext(). Locks are non-blocking and fail immediately with AdvisoryLockError if unavailable. Adds advisoryLocks option to PgsliceOptions (default: true) to allow disabling locks for testing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
They were broken previously because we were not explicitly checking out and reusing the same connection for the duration of the operation, leading to a "Attempted to release lock that was never held." error.
mthadley
force-pushed
the
advisory-locks
branch
from
e094fb1 to
79f5c80
Compare
2 participants
Wraps all of the core Pgslice operations (
prep,fill,swap, etc.) in advisory locks to minimize the risks in cases where multiple pgslice operations were accidentally executed against the same table.Note that this required refactoring some of the connection handling, as operations like
fillwere not ensuring they had checked out a single connection for the lifetime of their operation. Now.startpasses a connection instead of a transaction, which means that many operations now need to start the transaction themselves, hence all of the indentation.