From 399deab1005b7c3541ad0dacb52bd7961b5167cc Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Mon, 7 Jan 2019 14:19:36 -0500 Subject: [PATCH] database: remove FindLock() --- database/database.go | 17 +++++------------ database/dbutil.go | 22 ---------------------- database/pgsql/lock.go | 25 ++++--------------------- database/pgsql/lock_test.go | 11 +---------- 4 files changed, 10 insertions(+), 65 deletions(-) diff --git a/database/database.go b/database/database.go index f760061f..8485eed2 100644 --- a/database/database.go +++ b/database/database.go @@ -183,23 +183,16 @@ type Session interface { // FindKeyValue retrieves a value from the given key. FindKeyValue(key string) (value string, found bool, err error) - // Lock creates or renew a Lock in the database with the given name, owner - // and duration. + // Lock acquires or renews a lock in the database with the given name, owner + // and duration without blocking. After the specified duration, the lock + // expires if it hasn't already been unlocked in order to prevent a deadlock. // - // After the specified duration, the Lock expires by itself if it hasn't been - // unlocked, and thus, let other users create a Lock with the same name. - // However, the owner can renew its Lock by setting renew to true. - // Lock should not block, it should instead returns whether the Lock has been - // successfully acquired/renewed. If it's the case, the expiration time of - // that Lock is returned as well. + // If the acquisition of a lock is not successful, expiration should be + // the time that existing lock expires. Lock(name string, owner string, duration time.Duration, renew bool) (success bool, expiration time.Time, err error) // Unlock releases an existing Lock. Unlock(name, owner string) error - - // FindLock returns the owner of a Lock specified by the name, and its - // expiration time if it exists. - FindLock(name string) (owner string, expiration time.Time, found bool, err error) } // Datastore represents a persistent data store diff --git a/database/dbutil.go b/database/dbutil.go index d02286a9..31d48503 100644 --- a/database/dbutil.go +++ b/database/dbutil.go @@ -15,7 +15,6 @@ package database import ( - "errors" "time" "github.com/deckarep/golang-set" @@ -351,24 +350,3 @@ func ReleaseLock(datastore Datastore, name, owner string) { return } } - -// ErrLockNotFound is returned when FindLock succeeds, but cannot find a lock. -var ErrLockNotFound = errors.New("no lock was found") - -// FindLock determines if a global lock with the provided name already exists. -func FindLock(datastore Datastore, updaterLockName string) (owner string, expiration time.Time, err error) { - var tx Session - tx, err = datastore.Begin() - if err != nil { - return - } - defer tx.Rollback() - - var found bool - owner, expiration, found, err = tx.FindLock(updaterLockName) - if err != nil && !found { - err = ErrLockNotFound - } - - return -} diff --git a/database/pgsql/lock.go b/database/pgsql/lock.go index b7e859e6..ddb916ce 100644 --- a/database/pgsql/lock.go +++ b/database/pgsql/lock.go @@ -25,7 +25,7 @@ import ( const ( soiLock = `INSERT INTO lock(name, owner, until) VALUES ($1, $2, $3)` - searchLock = `SELECT owner, until FROM Lock WHERE name = $1` + searchLock = `SELECT until FROM Lock WHERE name = $1` updateLock = `UPDATE Lock SET until = $3 WHERE name = $1 AND owner = $2` removeLock = `DELETE FROM Lock WHERE name = $1 AND owner = $2` removeLockExpired = `DELETE FROM LOCK WHERE until < CURRENT_TIMESTAMP` @@ -67,7 +67,9 @@ func (tx *pgSession) Lock(name string, owner string, duration time.Duration, ren _, err := tx.Exec(soiLock, name, owner, until) if err != nil { if isErrUniqueViolation(err) { - return false, until, nil + // Return the existing locks expiration. + err := tx.QueryRow(searchLock, name).Scan(&until) + return false, until, handleError("searchLock", err) } return false, until, handleError("insertLock", err) } @@ -86,25 +88,6 @@ func (tx *pgSession) Unlock(name, owner string) error { return err } -// FindLock returns the owner of a lock specified by its name and its -// expiration time. -func (tx *pgSession) FindLock(name string) (string, time.Time, bool, error) { - if name == "" { - return "", time.Time{}, false, commonerr.NewBadRequestError("could not find an invalid lock") - } - - defer observeQueryTime("FindLock", "all", time.Now()) - - var owner string - var until time.Time - err := tx.QueryRow(searchLock, name).Scan(&owner, &until) - if err != nil { - return owner, until, false, handleError("searchLock", err) - } - - return owner, until, true, nil -} - // pruneLocks removes every expired locks from the database func (tx *pgSession) pruneLocks() error { defer observeQueryTime("pruneLocks", "all", time.Now()) diff --git a/database/pgsql/lock_test.go b/database/pgsql/lock_test.go index 19a5a934..d2a7ebf3 100644 --- a/database/pgsql/lock_test.go +++ b/database/pgsql/lock_test.go @@ -26,7 +26,6 @@ func TestLock(t *testing.T) { defer datastore.Close() var l bool - var et time.Time // Create a first lock. l, _, err := tx.Lock("test1", "owner1", time.Minute, false) @@ -62,19 +61,11 @@ func TestLock(t *testing.T) { assert.Nil(t, err) tx = restartSession(t, datastore, tx, true) - l, et, err = tx.Lock("test1", "owner2", time.Minute, false) + l, _, err = tx.Lock("test1", "owner2", time.Minute, false) assert.Nil(t, err) assert.True(t, l) tx = restartSession(t, datastore, tx, true) - // LockInfo - o, et2, ok, err := tx.FindLock("test1") - assert.True(t, ok) - assert.Nil(t, err) - assert.Equal(t, "owner2", o) - assert.Equal(t, et.Second(), et2.Second()) - tx = restartSession(t, datastore, tx, true) - // Create a second lock which is actually already expired ... l, _, err = tx.Lock("test2", "owner1", -time.Minute, false) assert.Nil(t, err)