Skip to content

Commit

Permalink
feat: improve error messages of rueidislock (#616)
Browse files Browse the repository at this point in the history 
Signed-off-by: Rueian <rueiancsie@gmail.com>
  • Loading branch information
@rueian
rueian committed Aug 26, 2024
1 parent 82a4500 commit 372da2f
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 42 deletions.
74 changes: 37 additions & 37 deletions rueidislock/lock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rueidislock
import (
"context"
"errors"
"fmt"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -168,7 +169,7 @@ func (m *locker) acquire(ctx context.Context, key, val string, deadline time.Tim
}
cancel()
if err = resp.Error(); rueidis.IsRedisNil(err) {
return ErrNotLocked
return fmt.Errorf("%w: key %s is held by others", ErrNotLocked, key)
}
return err
}
Expand Down Expand Up @@ -206,11 +207,7 @@ func (m *locker) waitgate(ctx context.Context, name string) (g *gate, err error)
}
select {
case <-ctx.Done():
m.mu.Lock()
if g.w--; g.w == 0 && m.gates[name] == g {
delete(m.gates, name)
}
m.mu.Unlock()
m.removegate(g, name)
return nil, ctx.Err()
case _, ok = <-g.ch:
if ok {
Expand Down Expand Up @@ -246,6 +243,14 @@ func (m *locker) forcegate(name string) (g *gate) {
return g
}

func (m *locker) removegate(g *gate, name string) {
m.mu.Lock()
if g.w--; g.w == 0 && m.gates[name] == g {
delete(m.gates, name)
}
m.mu.Unlock()
}

func (m *locker) onInvalidations(messages []rueidis.RedisMessage) {
if messages == nil {
m.mu.RLock()
Expand Down Expand Up @@ -286,7 +291,7 @@ func (m *locker) onInvalidations(messages []rueidis.RedisMessage) {
}
}

func (m *locker) try(ctx context.Context, cancel context.CancelFunc, name string, g *gate, force bool) context.CancelFunc {
func (m *locker) try(ctx context.Context, cancel context.CancelFunc, name string, g *gate, force bool) (context.CancelFunc, error) {
var err error

val := random()
Expand Down Expand Up @@ -324,7 +329,7 @@ func (m *locker) try(ctx context.Context, cancel context.CancelFunc, name string
}
}
}
if err != ErrNotLocked {
if !errors.Is(err, ErrNotLocked) {
_ = m.script(context.Background(), delkey, key, val, deadline)
}
if released := atomic.AddInt32(&released, 1); released >= m.majority {
Expand Down Expand Up @@ -354,7 +359,7 @@ func (m *locker) try(ctx context.Context, cancel context.CancelFunc, name string
case <-ch:
default:
}
if err != ErrNotLocked {
if !errors.Is(err, ErrNotLocked) {
if err = m.acquire(ctx, key, val, deadline, force); force && err == nil {
m.mu.RLock()
if m.gates != nil {
Expand Down Expand Up @@ -389,62 +394,57 @@ func (m *locker) try(ctx context.Context, cancel context.CancelFunc, name string
return func() {
cancel()
<-done
}
}, nil
}
<-done
return nil
if err == nil {
err = fmt.Errorf("%w: failed to acquire the majority of keys (%d/%d)", ErrNotLocked, atomic.LoadInt32(&acquired), m.totalcnt)
}
return cancel, err
}

func (m *locker) ForceWithContext(ctx context.Context, name string) (context.Context, context.CancelFunc, error) {
var err error
ctx, cancel := context.WithCancel(ctx)
if g := m.forcegate(name); g != nil {
if cancel := m.try(ctx, cancel, name, g, true); cancel != nil {
if cancel, err = m.try(ctx, cancel, name, g, true); err == nil {
return ctx, cancel, nil
}
m.mu.Lock()
if g.w--; g.w == 0 {
if m.gates[name] == g {
delete(m.gates, name)
}
}
m.mu.Unlock()
m.removegate(g, name)
}
cancel()
return ctx, cancel, ErrNotLocked
if err == nil {
err = ErrLockerClosed
}
return ctx, cancel, err
}

func (m *locker) TryWithContext(ctx context.Context, name string) (context.Context, context.CancelFunc, error) {
var err error
ctx, cancel := context.WithCancel(ctx)
if g := m.trygate(name); g != nil {
if cancel := m.try(ctx, cancel, name, g, false); cancel != nil {
if cancel, err = m.try(ctx, cancel, name, g, false); err == nil {
return ctx, cancel, nil
}
m.mu.Lock()
if g.w--; g.w == 0 {
if m.gates[name] == g {
delete(m.gates, name)
}
}
m.mu.Unlock()
m.removegate(g, name)
}
cancel()
return ctx, cancel, ErrNotLocked
if err == nil {
err = fmt.Errorf("%w: the lock is held by others or the locker is closed", ErrNotLocked)
}
return ctx, cancel, err
}

func (m *locker) WithContext(ctx context.Context, name string) (context.Context, context.CancelFunc, error) {
func (m *locker) WithContext(src context.Context, name string) (context.Context, context.CancelFunc, error) {
for {
ctx, cancel := context.WithCancel(ctx)
ctx, cancel := context.WithCancel(src)
g, err := m.waitgate(ctx, name)
if g != nil {
if cancel := m.try(ctx, cancel, name, g, false); cancel != nil {
if cancel, err := m.try(ctx, cancel, name, g, false); err == nil {
return ctx, cancel, nil
}
m.mu.Lock()
if g.w--; g.w == 0 && err != nil { // delete g from m.gates only when exiting with an error.
if m.gates[name] == g {
delete(m.gates, name)
}
}
g.w-- // do not delete g from m.gates here.
m.mu.Unlock()
}
if cancel(); err != nil {
Expand Down
10 changes: 5 additions & 5 deletions rueidislock/lock_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func TestLocker_TryWithContext(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if _, _, err := locker.TryWithContext(ctx, lck); err != ErrNotLocked {
if _, _, err := locker.TryWithContext(ctx, lck); !errors.Is(err, ErrNotLocked) {
t.Fatal(err)
}
cancel()
Expand Down Expand Up @@ -486,7 +486,7 @@ func TestLocker_ForceWithContextThenTryWithContext(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if _, _, err := locker.TryWithContext(ctx, lck); err != ErrNotLocked {
if _, _, err := locker.TryWithContext(ctx, lck); !errors.Is(err, ErrNotLocked) {
t.Fatal(err)
}
cancel()
Expand Down Expand Up @@ -528,7 +528,7 @@ func TestLocker_TryWithContext_MultipleLocker(t *testing.T) {
for j := 0; j < cnt; j++ {
for {
_, cancel, err := l.TryWithContext(ctx, lck)
if err != nil && err != ErrNotLocked {
if err != nil && !errors.Is(err, ErrNotLocked) {
t.Error(err)
return
}
Expand Down Expand Up @@ -663,7 +663,7 @@ func TestLocker_Close(t *testing.T) {
wg.Add(10)
for i := 0; i < 10; i++ {
go func() {
if _, _, err := locker.WithContext(context.Background(), lck); err != ErrLockerClosed {
if _, _, err := locker.WithContext(context.Background(), lck); !errors.Is(err, ErrLockerClosed) {
t.Error(err)
}
wg.Done()
Expand All @@ -676,7 +676,7 @@ func TestLocker_Close(t *testing.T) {
if err := ctx.Err(); !errors.Is(err, context.Canceled) {
t.Fatal(err)
}
if _, _, err := locker.WithContext(context.Background(), lck); err != ErrLockerClosed {
if _, _, err := locker.WithContext(context.Background(), lck); !errors.Is(err, ErrLockerClosed) {
t.Fatal(err)
}
}
Expand Down

0 comments on commit 372da2f

Please sign in to comment.