diff --git a/errors.go b/errors.go new file mode 100644 index 0000000..3efb2d4 --- /dev/null +++ b/errors.go @@ -0,0 +1,8 @@ +package cacher + +import "errors" + +var ( + // ErrWaitTooLong error wait too long + ErrWaitTooLong = errors.New("wait too long") +) diff --git a/go.mod b/go.mod index 8a34d06..2931f56 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index bf6c723..37d6169 100644 --- a/go.sum +++ b/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= diff --git a/keeper.go b/keeper.go index 4eb6825..4203f4a 100644 --- a/keeper.go +++ b/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: diff --git a/keeper_test.go b/keeper_test.go index a359c97..c40beb8 100644 --- a/keeper_test.go +++ b/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") } }