Guardian - better remote locking through reference IDs

2026-02-11 10:06:24 +05:00 · 2019-03-17 20:25:45 +01:00
parent f19ef9a2dc
commit fa75764203
11 changed files with 402 additions and 257 deletions
--- a/guardian/guardian.go
+++ b/guardian/guardian.go
@@ -27,7 +27,9 @@ import (
 )

 // global package variable
-var Guardian = &GuardianLocker{locksMap: make(map[string]*itemLock)}
+var Guardian = &GuardianLocker{
+	locks: make(map[string]*itemLock),
+	refs:  make(map[string][]string)}

 type itemLock struct {
 	lk  chan struct{}
@@ -36,44 +38,94 @@ type itemLock struct {

 // GuardianLocker is an optimized locking system per locking key
 type GuardianLocker struct {
-	locksMap   map[string]*itemLock
-	sync.Mutex // protects the map
+	locks   map[string]*itemLock
+	lkMux   sync.Mutex          // protects the locks
+	refs    map[string][]string // used in case of remote locks
+	refsMux sync.RWMutex        // protects the map
 }

 func (gl *GuardianLocker) lockItem(itmID string) {
-	gl.Lock()
-	itmLock, exists := gl.locksMap[itmID]
+	if itmID == "" {
+		return
+	}
+	gl.lkMux.Lock()
+	itmLock, exists := gl.locks[itmID]
 	if !exists {
 		itmLock = &itemLock{lk: make(chan struct{}, 1)}
-		gl.locksMap[itmID] = itmLock
+		gl.locks[itmID] = itmLock
 		itmLock.lk <- struct{}{}
 	}
 	itmLock.cnt++
 	select {
 	case <-itmLock.lk:
-		gl.Unlock()
+		gl.lkMux.Unlock()
 		return
 	default: // move further so we can unlock
 	}
-	gl.Unlock()
+	gl.lkMux.Unlock()
 	<-itmLock.lk
 }

 func (gl *GuardianLocker) unlockItem(itmID string) {
-	gl.Lock()
-	itmLock, exists := gl.locksMap[itmID]
+	gl.lkMux.Lock()
+	itmLock, exists := gl.locks[itmID]
 	if !exists {
-		gl.Unlock()
+		gl.lkMux.Unlock()
 		return
 	}
 	itmLock.cnt--
 	if itmLock.cnt == 0 {
-		delete(gl.locksMap, itmID)
+		delete(gl.locks, itmID)
 	}
-	gl.Unlock()
+	gl.lkMux.Unlock()
 	itmLock.lk <- struct{}{}
 }

+// lockWithReference will perform locks and also generate a lock reference for it (so it can be used when remotely locking)
+func (gl *GuardianLocker) lockWithReference(refID string, lkIDs []string) string {
+	var refEmpty bool
+	if refID == "" {
+		refEmpty = true
+		refID = utils.GenUUID()
+	}
+	gl.lockItem(refID) // make sure we only process one simultaneous refID at the time, otherwise checking already used refID is not reliable
+	gl.refsMux.Lock()
+	if !refEmpty {
+		if _, has := gl.refs[refID]; has {
+			gl.refsMux.Unlock()
+			gl.unlockItem(refID)
+			return "" // no locking was done
+		}
+	}
+	gl.refs[refID] = lkIDs
+	gl.refsMux.Unlock()
+	// execute the real locks
+	for _, lk := range lkIDs {
+		gl.lockItem(lk)
+	}
+	gl.unlockItem(refID)
+	return refID
+}
+
+// unlockWithReference will unlock based on the reference ID
+func (gl *GuardianLocker) unlockWithReference(refID string) (lkIDs []string) {
+	gl.lockItem(refID)
+	gl.refsMux.Lock()
+	lkIDs, has := gl.refs[refID]
+	if !has {
+		gl.refsMux.Unlock()
+		gl.unlockItem(refID)
+		return
+	}
+	delete(gl.refs, refID)
+	gl.refsMux.Unlock()
+	for _, lk := range lkIDs {
+		gl.unlockItem(lk)
+	}
+	gl.unlockItem(refID)
+	return
+}
+
 // Guard executes the handler between locks
 func (gl *GuardianLocker) Guard(handler func() (interface{}, error), timeout time.Duration, lockIDs ...string) (reply interface{}, err error) {
 	for _, lockID := range lockIDs {
@@ -107,25 +159,26 @@ func (gl *GuardianLocker) Guard(handler func() (interface{}, error), timeout tim
 	return
 }

-// GuardTimed aquires a lock for duration
-func (gl *GuardianLocker) GuardIDs(timeout time.Duration, lockIDs ...string) {
-	for _, lockID := range lockIDs {
-		gl.lockItem(lockID)
-	}
-	if timeout != 0 {
-		go func(timeout time.Duration, lockIDs ...string) {
+// GuardIDs aquires a lock for duration
+// returns the reference ID for the lock group aquired
+func (gl *GuardianLocker) GuardIDs(refID string, timeout time.Duration, lkIDs ...string) (retRefID string) {
+	retRefID = gl.lockWithReference(refID, lkIDs)
+	if timeout != 0 && retRefID != "" {
+		go func() {
 			time.Sleep(timeout)
-			utils.Logger.Warning(fmt.Sprintf("<Guardian> WARNING: force timing-out locks: %+v", lockIDs))
-			gl.UnguardIDs(lockIDs...)
-		}(timeout, lockIDs...)
+			lkIDs := gl.unlockWithReference(retRefID)
+			if len(lkIDs) != 0 {
+				utils.Logger.Warning(fmt.Sprintf("<Guardian> WARNING: force timing-out locks: %+v", lkIDs))
+			}
+		}()
 	}
 	return
 }

-// UnguardTimed attempts to unlock a set of locks based on their locksUUID
-func (gl *GuardianLocker) UnguardIDs(lockIDs ...string) {
-	for _, lockID := range lockIDs {
-		gl.unlockItem(lockID)
+// UnguardIDs attempts to unlock a set of locks based on their reference ID received on lock
+func (gl *GuardianLocker) UnguardIDs(refID string) (lkIDs []string) {
+	if refID == "" {
+		return
 	}
-	return
+	return gl.unlockWithReference(refID)
 }
--- a/guardian/guardian_test.go
+++ b/guardian/guardian_test.go
@@ -18,9 +18,13 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>
 package guardian

 import (
+	"reflect"
+	"strconv"
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/cgrates/cgrates/utils"
 )

 func delayHandler() (interface{}, error) {
@@ -45,16 +49,17 @@ func TestGuardianMultipleKeys(t *testing.T) {
 	}
 	sg.Wait()
 	mustExecDur := time.Duration(maxIter*100) * time.Millisecond
-	if execTime := time.Now().Sub(tStart); execTime < mustExecDur || execTime > mustExecDur+time.Duration(20*time.Millisecond) {
+	if execTime := time.Now().Sub(tStart); execTime < mustExecDur ||
+		execTime > mustExecDur+time.Duration(100*time.Millisecond) {
 		t.Errorf("Execution took: %v", execTime)
 	}
-	Guardian.Lock()
+	Guardian.lkMux.Lock()
 	for _, key := range keys {
-		if _, hasKey := Guardian.locksMap[key]; hasKey {
+		if _, hasKey := Guardian.locks[key]; hasKey {
 			t.Errorf("Possible memleak for key: %s", key)
 		}
 	}
-	Guardian.Unlock()
+	Guardian.lkMux.Unlock()
 }

 func TestGuardianTimeout(t *testing.T) {
@@ -73,16 +78,17 @@ func TestGuardianTimeout(t *testing.T) {
 	}
 	sg.Wait()
 	mustExecDur := time.Duration(maxIter*10) * time.Millisecond
-	if execTime := time.Now().Sub(tStart); execTime < mustExecDur || execTime > mustExecDur+time.Duration(20*time.Millisecond) {
+	if execTime := time.Now().Sub(tStart); execTime < mustExecDur ||
+		execTime > mustExecDur+time.Duration(100*time.Millisecond) {
 		t.Errorf("Execution took: %v", execTime)
 	}
-	Guardian.Lock()
+	Guardian.lkMux.Lock()
 	for _, key := range keys {
-		if _, hasKey := Guardian.locksMap[key]; hasKey {
+		if _, hasKey := Guardian.locks[key]; hasKey {
 			t.Error("Possible memleak")
 		}
 	}
-	Guardian.Unlock()
+	Guardian.lkMux.Unlock()
 }

 func TestGuardianGuardIDs(t *testing.T) {
@@ -90,122 +96,185 @@ func TestGuardianGuardIDs(t *testing.T) {
 	//lock with 3 keys
 	lockIDs := []string{"test1", "test2", "test3"}
 	// make sure the keys are not in guardian before lock
-	Guardian.Lock()
+	Guardian.lkMux.Lock()
 	for _, lockID := range lockIDs {
-		if _, hasKey := Guardian.locksMap[lockID]; hasKey {
+		if _, hasKey := Guardian.locks[lockID]; hasKey {
 			t.Errorf("Unexpected lockID found: %s", lockID)
 		}
 	}
-	Guardian.Unlock()
-
+	Guardian.lkMux.Unlock()
 	// lock 3 items
 	tStart := time.Now()
 	lockDur := 2 * time.Millisecond
-	Guardian.GuardIDs(lockDur, lockIDs...)
-	Guardian.Lock()
+	Guardian.GuardIDs("", lockDur, lockIDs...)
+	Guardian.lkMux.Lock()
 	for _, lockID := range lockIDs {
-		if itmLock, hasKey := Guardian.locksMap[lockID]; !hasKey {
+		if itmLock, hasKey := Guardian.locks[lockID]; !hasKey {
 			t.Errorf("Cannot find lock for lockID: %s", lockID)
 		} else if itmLock.cnt != 1 {
 			t.Errorf("Unexpected itmLock found: %+v", itmLock)
 		}
 	}
-	Guardian.Unlock()
+	Guardian.lkMux.Unlock()
 	secLockDur := time.Duration(1 * time.Millisecond)
-
 	// second lock to test counter
-	go Guardian.GuardIDs(secLockDur, lockIDs[1:]...)
-	time.Sleep(20 * time.Microsecond) // give time for goroutine to lock
-
+	go Guardian.GuardIDs("", secLockDur, lockIDs[1:]...)
+	time.Sleep(30 * time.Microsecond) // give time for goroutine to lock
 	// check if counters were properly increased
-	Guardian.Lock()
+	Guardian.lkMux.Lock()
 	lkID := lockIDs[0]
 	eCnt := int64(1)
-	if itmLock, hasKey := Guardian.locksMap[lkID]; !hasKey {
+	if itmLock, hasKey := Guardian.locks[lkID]; !hasKey {
 		t.Errorf("Cannot find lock for lockID: %s", lkID)
 	} else if itmLock.cnt != eCnt {
 		t.Errorf("Unexpected counter: %d for itmLock with id %s", itmLock.cnt, lkID)
 	}
 	lkID = lockIDs[1]
 	eCnt = int64(2)
-	if itmLock, hasKey := Guardian.locksMap[lkID]; !hasKey {
+	if itmLock, hasKey := Guardian.locks[lkID]; !hasKey {
 		t.Errorf("Cannot find lock for lockID: %s", lkID)
 	} else if itmLock.cnt != eCnt {
 		t.Errorf("Unexpected counter: %d for itmLock with id %s", itmLock.cnt, lkID)
 	}
 	lkID = lockIDs[2]
 	eCnt = int64(1) // we did not manage to increase it yet since it did not pass first lock
-	if itmLock, hasKey := Guardian.locksMap[lkID]; !hasKey {
+	if itmLock, hasKey := Guardian.locks[lkID]; !hasKey {
 		t.Errorf("Cannot find lock for lockID: %s", lkID)
 	} else if itmLock.cnt != eCnt {
 		t.Errorf("Unexpected counter: %d for itmLock with id %s", itmLock.cnt, lkID)
 	}
-	Guardian.Unlock()
-
-	time.Sleep(lockDur + secLockDur + 10*time.Millisecond) // give time to unlock before proceeding
+	Guardian.lkMux.Unlock()
+	time.Sleep(lockDur + secLockDur + 50*time.Millisecond) // give time to unlock before proceeding

 	// make sure all counters were removed
 	for _, lockID := range lockIDs {
-		if _, hasKey := Guardian.locksMap[lockID]; hasKey {
+		if _, hasKey := Guardian.locks[lockID]; hasKey {
 			t.Errorf("Unexpected lockID found: %s", lockID)
 		}
 	}
-
 	// test lock  without timer
-	Guardian.GuardIDs(0, lockIDs...)
+	refID := Guardian.GuardIDs("", 0, lockIDs...)
+
 	if totalLockDur := time.Now().Sub(tStart); totalLockDur < lockDur {
 		t.Errorf("Lock duration too small")
 	}
 	time.Sleep(time.Duration(30) * time.Millisecond)
-
 	// making sure the items stay locked
-	Guardian.Lock()
-	if len(Guardian.locksMap) != 3 {
-		t.Errorf("locksMap should be have 3 elements, have: %+v", Guardian.locksMap)
+	Guardian.lkMux.Lock()
+	if len(Guardian.locks) != 3 {
+		t.Errorf("locks should have 3 elements, have: %+v", Guardian.locks)
 	}
 	for _, lkID := range lockIDs {
-		if itmLock, hasKey := Guardian.locksMap[lkID]; !hasKey {
+		if itmLock, hasKey := Guardian.locks[lkID]; !hasKey {
 			t.Errorf("Cannot find lock for lockID: %s", lkID)
 		} else if itmLock.cnt != 1 {
 			t.Errorf("Unexpected counter: %d for itmLock with id %s", itmLock.cnt, lkID)
 		}
 	}
-	Guardian.Unlock()
-
-	Guardian.UnguardIDs(lockIDs...)
-	time.Sleep(time.Duration(50) * time.Millisecond)
-
+	Guardian.lkMux.Unlock()
+	Guardian.UnguardIDs(refID)
 	// make sure items were unlocked
-	Guardian.Lock()
-	if len(Guardian.locksMap) != 0 {
-		t.Errorf("locksMap should have 0 elements, has: %+v", Guardian.locksMap)
+	Guardian.lkMux.Lock()
+	if len(Guardian.locks) != 0 {
+		t.Errorf("locks should have 0 elements, has: %+v", Guardian.locks)
 	}
-	Guardian.Unlock()
+	Guardian.lkMux.Unlock()
 }

+// TestGuardianGuardIDsConcurrent executes GuardIDs concurrently
+func TestGuardianGuardIDsConcurrent(t *testing.T) {
+	maxIter := 500
+	sg := new(sync.WaitGroup)
+	keys := []string{"test1", "test2", "test3"}
+	refID := utils.GenUUID()
+	for i := 0; i < maxIter; i++ {
+		sg.Add(1)
+		go func() {
+			if retRefID := Guardian.GuardIDs(refID, 0, keys...); retRefID != "" {
+				if lkIDs := Guardian.UnguardIDs(refID); !reflect.DeepEqual(keys, lkIDs) {
+					t.Errorf("expecting: %+v, received: %+v", keys, lkIDs)
+				}
+			}
+			sg.Done()
+		}()
+	}
+	sg.Wait()
+
+	Guardian.lkMux.Lock()
+	if len(Guardian.locks) != 0 {
+		t.Errorf("Possible memleak for locks: %+v", Guardian.locks)
+	}
+	Guardian.lkMux.Unlock()
+	Guardian.refsMux.Lock()
+	if len(Guardian.refs) != 0 {
+		t.Errorf("Possible memleak for refs: %+v", Guardian.refs)
+	}
+	Guardian.refsMux.Unlock()
+}
+
+func TestGuardianGuardIDsTimeoutConcurrent(t *testing.T) {
+	maxIter := 50
+	sg := new(sync.WaitGroup)
+	keys := []string{"test1", "test2", "test3"}
+	refID := utils.GenUUID()
+	for i := 0; i < maxIter; i++ {
+		sg.Add(1)
+		go func() {
+			Guardian.GuardIDs(refID, time.Duration(time.Microsecond), keys...)
+			sg.Done()
+		}()
+	}
+	sg.Wait()
+	time.Sleep(10 * time.Millisecond)
+	Guardian.lkMux.Lock()
+	if len(Guardian.locks) != 0 {
+		t.Errorf("Possible memleak for locks: %+v", Guardian.locks)
+	}
+	Guardian.lkMux.Unlock()
+	Guardian.refsMux.Lock()
+	if len(Guardian.refs) != 0 {
+		t.Errorf("Possible memleak for refs: %+v", Guardian.refs)
+	}
+	Guardian.refsMux.Unlock()
+}
+
+// BenchmarkGuard-8      	  200000	     13759 ns/op
 func BenchmarkGuard(b *testing.B) {
-	for i := 0; i < 100; i++ {
+	for n := 0; n < b.N; n++ {
 		go Guardian.Guard(func() (interface{}, error) {
-			time.Sleep(1 * time.Millisecond)
+			time.Sleep(time.Microsecond)
 			return 0, nil
 		}, 0, "1")
 		go Guardian.Guard(func() (interface{}, error) {
-			time.Sleep(1 * time.Millisecond)
+			time.Sleep(time.Microsecond)
 			return 0, nil
 		}, 0, "2")
 		go Guardian.Guard(func() (interface{}, error) {
-			time.Sleep(1 * time.Millisecond)
+			time.Sleep(time.Microsecond)
 			return 0, nil
 		}, 0, "1")
 	}

 }

+// BenchmarkGuardian-8   	 1000000	      5794 ns/op
 func BenchmarkGuardian(b *testing.B) {
-	for i := 0; i < 100; i++ {
+	for n := 0; n < b.N; n++ {
 		go Guardian.Guard(func() (interface{}, error) {
-			time.Sleep(1 * time.Millisecond)
+			time.Sleep(time.Microsecond)
 			return 0, nil
-		}, 0, "1")
+		}, 0, strconv.Itoa(n))
+	}
+}
+
+// BenchmarkGuardIDs-8   	 1000000	      8732 ns/op
+func BenchmarkGuardIDs(b *testing.B) {
+	for n := 0; n < b.N; n++ {
+		go func() {
+			if refID := Guardian.GuardIDs("", 0, strconv.Itoa(n)); refID != "" {
+				time.Sleep(time.Microsecond)
+				Guardian.UnguardIDs(refID)
+			}
+		}()
 	}
 }