Merge pull request #19 from kumparan/feature/BAN-5761-get-lock

feature: lock on empty cache

errors.go

@@ -0,0 +1,8 @@
+package cacher
+
+import "errors"
+
+var (
+	// ErrWaitTooLong error wait too long
+	ErrWaitTooLong = errors.New("wait too long")
+)

go.mod

@@ -1,5 +1,7 @@
 module github.com/kumparan/cacher/v2
 
+go 1.12
+
 require (
 	github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 // indirect
 	github.com/alicebob/miniredis v2.5.0+incompatible

go.sum

@@ -1,4 +1,6 @@
+github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 h1:45bxf7AZMwWcqkLzDAQugVEwedisr5nRJ1r+7LYnv0U=
 github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
+github.com/alicebob/miniredis v2.5.0+incompatible h1:yBHoLpsyjupjz3NL3MhKMVkR41j82Yjf3KFv7ApYzUI=
 github.com/alicebob/miniredis v2.5.0+incompatible/go.mod h1:8HZjEj4yU0dwhYHky+DxYx+6BMjkBbe5ONFIF1MXffk=
 github.com/bsm/redislock v0.4.3 h1:TJ0RzHeSujLSuy4b33OWDknxAzKCdLdit0Hs9kOjElg=
 github.com/bsm/redislock v0.4.3/go.mod h1:mcygIsJknQThqWrlOgiPJ97CGmu3aAdQabg1ZIxT1BA=
@@ -11,6 +13,7 @@ github.com/go-redis/redis v6.15.6+incompatible h1:H9evprGPLI8+ci7fxQx6WNZHJSb7be
 github.com/go-redis/redis v6.15.6+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA=
 github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
+github.com/gomodule/redigo v2.0.0+incompatible h1:K/R+8tc58AaqLkqG2Ol3Qk+DR/TlNuhuh457pBFPtt0=
 github.com/gomodule/redigo v2.0.0+incompatible/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4=
 github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=

keeper.go

@@ -1,7 +1,6 @@
 package cacher
 
 import (
-	"errors"
 	"fmt"
 	"time"
 
@@ -203,10 +202,21 @@ func (k *keeper) GetOrLock(key string) (cachedItem interface{}, mutex *redislock
 
 		if !k.isLocked(key) {
 			cachedItem, err = k.getCachedItem(key)
-			if err != nil || cachedItem != nil {
-				return
+			switch {
+			// redis error, giving up
+			case err != nil && err != goredis.Nil:
+				return nil, nil, err
+			// cache not found, try to get another lock
+			case err == goredis.Nil || cachedItem == nil:
+				mutex, err = k.AcquireLock(key)
+				if err == nil {
+					return nil, mutex, nil
+				}
+				// can't acquire lock, let's keep waiting
+			// cache found, return it
+			default:
+				return cachedItem, nil, nil
 			}
-			return nil, nil, nil
 		}
 
 		elapsed := time.Since(start)
@@ -217,7 +227,7 @@ func (k *keeper) GetOrLock(key string) (cachedItem interface{}, mutex *redislock
 		time.Sleep(b.Duration())
 	}
 
-	return nil, nil, errors.New("wait too long")
+	return nil, nil, ErrWaitTooLong
 }
 
 // GetOrSet :nodoc:
@@ -574,10 +584,21 @@ func (k *keeper) GetHashMemberOrLock(identifier string, key string) (cachedItem
 
 		if !k.isLocked(lockKey) {
 			cachedItem, err = k.GetHashMember(identifier, key)
-			if err != nil || cachedItem != nil {
-				return
+			switch {
+			// redis error, giving up
+			case err != nil && err != goredis.Nil:
+				return nil, nil, err
+			// cache not found, try to get another lock
+			case err == goredis.Nil || cachedItem == nil:
+				mutex, err = k.AcquireLock(key)
+				if err == nil {
+					return nil, mutex, nil
+				}
+				// can't acquire lock, let's keep waiting
+			// cache found, return it
+			default:
+				return cachedItem, nil, nil
 			}
-			return nil, nil, nil
 		}
 
 		elapsed := time.Since(start)
@@ -588,7 +609,7 @@ func (k *keeper) GetHashMemberOrLock(identifier string, key string) (cachedItem
 		time.Sleep(b.Duration())
 	}
 
-	return nil, nil, errors.New("wait too long")
+	return nil, nil, ErrWaitTooLong
 }
 
 // StoreHashMember :nodoc:

keeper_test.go

@@ -1,6 +1,7 @@
 package cacher
 
 import (
+	"fmt"
 	"os"
 	"testing"
 	"time"
@@ -108,6 +109,9 @@ func TestKeeper_GetOrLock(t *testing.T) {
 		if res != nil {
 			t.Fatal("result should be nil")
 		}
+		if err := lock.Release(); err != nil {
+			t.Fatal(err)
+		}
 	})
 
 	t.Run("wait to getting lock", func(t *testing.T) {
@@ -153,6 +157,72 @@ func TestKeeper_GetOrLock(t *testing.T) {
 
 		<-doneCh
 	})
+
+	t.Run("locked got nil, then acquire lock", func(t *testing.T) {
+		keeper := newTestKeeper()
+		key := "test-get-or-lock-but-nil"
+		lockKey := "lock:" + key
+		cmd := client.Set(lockKey, []byte("test"), 500*time.Millisecond)
+		if cmd.Err() != nil {
+			t.Fatal(cmd.Err())
+		}
+
+		doneCh := make(chan struct{})
+		go func() {
+			defer close(doneCh)
+			res, lock, err := keeper.GetOrLock(key)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if lock == nil {
+				t.Fatal("should getting the new lock")
+			}
+			if res != nil {
+				t.Fatal("result should be nil")
+			}
+
+			if err := lock.Release(); err != nil {
+				t.Fatal("should not error")
+			}
+		}()
+
+		// delete the lock key, but not set a value to the key
+		delCmd := client.Del(lockKey)
+		if delCmd.Err() != nil {
+			t.Fatal("should not error")
+		}
+
+		<-doneCh
+	})
+
+	t.Run("error wait too long", func(t *testing.T) {
+		keeper := newTestKeeper()
+		keeper.SetWaitTime(100 * time.Millisecond)
+		key := "test-get-or-lock-but-error-wait-too-ling"
+		lockKey := "lock:" + key
+
+		cmd := client.Set(lockKey, []byte("test"), 200*time.Millisecond)
+		if cmd.Err() != nil {
+			t.Fatal(cmd.Err())
+		}
+
+		res, lock, err := keeper.GetOrLock(key)
+		if err == nil {
+			t.Fatal("should error")
+		}
+
+		if err != ErrWaitTooLong {
+			t.Fatal("should error wait too long")
+		}
+
+		if lock != nil {
+			t.Fatal("should be nil")
+		}
+
+		if res != nil {
+			t.Fatal("should be nil")
+		}
+	})
 }
 
 func TestKeeper_GetOrSet(t *testing.T) {
@@ -289,10 +359,10 @@ func TestKeeper_GetHashMemberOrLock(t *testing.T) {
 	t.Run("wait to getting lock", func(t *testing.T) {
 		keeper := newTestKeeper()
 		bucket := "test-bucket"
-		key := "test-get-or-lock"
-		lock := "lock:" + bucket + ":" + key
+		key := "test-get-or-lockKey"
+		lockKey := fmt.Sprintf("lockKey:%s:%s", bucket, key)
 
-		cmd := client.Set(lock, []byte("test"), 500*time.Millisecond)
+		cmd := client.Set(lockKey, []byte("test"), 500*time.Millisecond)
 		if cmd.Err() != nil {
 			t.Fatal(cmd.Err())
 		}
@@ -324,18 +394,87 @@ func TestKeeper_GetHashMemberOrLock(t *testing.T) {
 			t.Fatal(hsetCmd.Err())
 		}
 
-		dumpCmd := client.Del(lock)
+		dumpCmd := client.Del(lockKey)
 		if dumpCmd.Err() != nil {
 			t.Fatal(dumpCmd.Err())
 		}
 
 		<-doneCh
 	})
+
+	t.Run("locked got nil, then acquire lock", func(t *testing.T) {
+		keeper := newTestKeeper()
+		key := "test-get-or-lock-but-nil"
+		bucket := "test-hash-lock-got-nil-bucket"
+		lockKey := fmt.Sprintf("lock:%s:%s", bucket, key)
+
+		cmd := client.Set(lockKey, []byte("test"), 500*time.Millisecond)
+		if cmd.Err() != nil {
+			t.Fatal(cmd.Err())
+		}
+
+		doneCh := make(chan struct{})
+		go func() {
+			defer close(doneCh)
+			res, lock, err := keeper.GetHashMemberOrLock(bucket, key)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if lock == nil {
+				t.Fatal("should getting the new lock")
+			}
+			if res != nil {
+				t.Fatal("result should be nil")
+			}
+
+			if err := lock.Release(); err != nil {
+				t.Fatal("should not error")
+			}
+		}()
+
+		// delete the lock key, but not set a value to the key
+		delCmd := client.Del(lockKey)
+		if delCmd.Err() != nil {
+			t.Fatal("should not error")
+		}
+
+		<-doneCh
+	})
+
+	t.Run("error wait too long", func(t *testing.T) {
+		keeper := newTestKeeper()
+		keeper.SetWaitTime(100 * time.Millisecond)
+		key := "test-get-or-lock-but-error-wait-too-ling"
+		bucket := "test-hash-error-wait-too-long-bucket"
+		lockKey := fmt.Sprintf("lock:%s:%s", bucket, key)
+
+		cmd := client.Set(lockKey, []byte("test"), 200*time.Millisecond)
+		if cmd.Err() != nil {
+			t.Fatal(cmd.Err())
+		}
+
+		res, lock, err := keeper.GetHashMemberOrLock(bucket, key)
+		if err == nil {
+			t.Fatal("should error")
+		}
+
+		if err != ErrWaitTooLong {
+			t.Fatal("should error wait too long")
+		}
+
+		if lock != nil {
+			t.Fatal("should be nil")
+		}
+
+		if res != nil {
+			t.Fatal("should be nil")
+		}
+	})
 }
 
 func TestKeeper_DeleteHashMember(t *testing.T) {
 	keeper := newTestKeeper()
-	bucket := "test-bucket"
+	bucket := "test-bucket-hash-member"
 	key1 := "test-get-or-lock"
 	key2 := "test-get-or-lock-2"
 
@@ -359,13 +498,9 @@ func TestKeeper_DeleteHashMember(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	bCmd := client.Exists(bucket)
-	if bCmd.Err() != nil {
-		t.Fatal(bCmd.Err())
-	}
-
-	if bCmd.Val() != 0 {
-		t.Fatal("should be 0")
+	bCmd := client.HGet(bucket, key2)
+	if bCmd.Err() == nil || bCmd.Err() != redis.Nil {
+		t.Fatal("should be error and nil error")
 	}
 }