From ab974ddf1476179e27d1ebad433e1c359c328d4d Mon Sep 17 00:00:00 2001 From: Misi Date: Wed, 6 Nov 2024 09:47:32 +0100 Subject: [PATCH] ServerLock: Fix pg concurrency/locking issue (#95916) Fix pg unique constraint validation in serverlock --- pkg/infra/serverlock/serverlock.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/infra/serverlock/serverlock.go b/pkg/infra/serverlock/serverlock.go index 988a06d46a7..768854f9b06 100644 --- a/pkg/infra/serverlock/serverlock.go +++ b/pkg/infra/serverlock/serverlock.go @@ -352,16 +352,26 @@ func (sl *ServerLockService) executeFunc(ctx context.Context, actionName string, } func (sl *ServerLockService) createLock(ctx context.Context, - lockRow *serverLock, dbSession *sqlstore.DBSession) (*serverLock, error) { + lockRow *serverLock, dbSession *sqlstore.DBSession, +) (*serverLock, error) { affected := int64(1) rawSQL := `INSERT INTO server_lock (operation_uid, last_execution, version) VALUES (?, ?, ?)` if sl.SQLStore.GetDBType() == migrator.Postgres { - rawSQL += ` RETURNING id` + rawSQL += ` ON CONFLICT DO NOTHING RETURNING id` var id int64 _, err := dbSession.SQL(rawSQL, lockRow.OperationUID, lockRow.LastExecution, 0).Get(&id) if err != nil { return nil, err } + if id == 0 { + // Considering the default isolation level (READ COMMITTED), an entry could be added to the table + // between the SELECT and the INSERT. And inserting a row with the same operation_uid would violate the unique + // constraint. In this case, the ON CONFLICT DO NOTHING clause will prevent generating an error. + // And the returning id will be 0 which means that there wasn't any row inserted (another server has the lock), + // therefore we return the ServerLockExistsError. + // https://www.postgresql.org/docs/current/transaction-iso.html#XACT-READ-COMMITTED + return nil, &ServerLockExistsError{actionName: lockRow.OperationUID} + } lockRow.Id = id } else { res, err := dbSession.Exec(