Skip to content

Commit

Permalink
Add context to ErrLeaseNotAcquired (#87)
Browse files Browse the repository at this point in the history
* clientlibrary/checkpoint: convert ErrLeaseAcquired to struct

Signed-off-by: Aurélien Rainone <aurelien.rainone@gmail.com>

* clientlibrary/checkpoint: add context to ErrLeaseNotAcquired

Signed-off-by: Aurélien Rainone <aurelien.rainone@gmail.com>

* Use errors.As to check for ErrLeaseNotAcquired error

Signed-off-by: Aurélien Rainone <aurelien.rainone@gmail.com>
  • Loading branch information
arl authored Jan 26, 2021
1 parent ff6f70d commit 022ec8d
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 10 deletions.
13 changes: 10 additions & 3 deletions clientlibrary/checkpoint/checkpointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ package checkpoint

import (
"errors"
"fmt"

par "github.com/vmware/vmware-go-kcl/clientlibrary/partition"
)

Expand All @@ -41,11 +43,16 @@ const (

// We've completely processed all records in this shard.
ShardEnd = "SHARD_END"

// ErrLeaseNotAcquired is returned when we failed to get a lock on the shard
ErrLeaseNotAcquired = "lease is already held by another node"
)

type ErrLeaseNotAcquired struct {
cause string
}

func (e ErrLeaseNotAcquired) Error() string {
return fmt.Sprintf("lease not acquired: %s", e.cause)
}

// Checkpointer handles checkpointing when a record has been processed
type Checkpointer interface {
// Init initialises the Checkpoint
Expand Down
5 changes: 2 additions & 3 deletions clientlibrary/checkpoint/dynamodb-checkpointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
package checkpoint

import (
"errors"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -142,7 +141,7 @@ func (checkpointer *DynamoCheckpoint) GetLease(shard *par.ShardStatus, newAssign
}

if time.Now().UTC().Before(currentLeaseTimeout) && assignedTo != newAssignTo {
return errors.New(ErrLeaseNotAcquired)
return ErrLeaseNotAcquired{"current lease timeout not yet expired"}
}

checkpointer.log.Debugf("Attempting to get a lock for shard: %s, leaseTimeout: %s, assignedTo: %s", shard.ID, currentLeaseTimeout, assignedTo)
Expand Down Expand Up @@ -186,7 +185,7 @@ func (checkpointer *DynamoCheckpoint) GetLease(shard *par.ShardStatus, newAssign
if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == dynamodb.ErrCodeConditionalCheckFailedException {
return errors.New(ErrLeaseNotAcquired)
return ErrLeaseNotAcquired{dynamodb.ErrCodeConditionalCheckFailedException}
}
}
return err
Expand Down
5 changes: 3 additions & 2 deletions clientlibrary/checkpoint/dynamodb-checkpointer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ package checkpoint

import (
"errors"
"github.com/stretchr/testify/assert"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/dynamodb"
Expand Down Expand Up @@ -85,7 +86,7 @@ func TestGetLeaseNotAquired(t *testing.T) {
Checkpoint: "",
Mux: &sync.Mutex{},
}, "ijkl-mnop")
if err == nil || err.Error() != ErrLeaseNotAcquired {
if err == nil || !errors.As(err, &ErrLeaseNotAcquired{}) {
t.Errorf("Got a lease when it was already held by abcd-efgh: %s", err)
}
}
Expand Down
3 changes: 2 additions & 1 deletion clientlibrary/worker/shard-consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
package worker

import (
"errors"
"math"
"sync"
"time"
Expand Down Expand Up @@ -162,7 +163,7 @@ func (sc *ShardConsumer) getRecords(shard *par.ShardStatus) error {
log.Debugf("Refreshing lease on shard: %s for worker: %s", shard.ID, sc.consumerID)
err = sc.checkpointer.GetLease(shard, sc.consumerID)
if err != nil {
if err.Error() == chk.ErrLeaseNotAcquired {
if errors.As(err, &chk.ErrLeaseNotAcquired{}) {
log.Warnf("Failed in acquiring lease on shard: %s for worker: %s", shard.ID, sc.consumerID)
return nil
}
Expand Down
3 changes: 2 additions & 1 deletion clientlibrary/worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
package worker

import (
"errors"
"math/rand"
"sync"
"time"
Expand Down Expand Up @@ -277,7 +278,7 @@ func (w *Worker) eventLoop() {
err = w.checkpointer.GetLease(shard, w.workerID)
if err != nil {
// cannot get lease on the shard
if err.Error() != chk.ErrLeaseNotAcquired {
if !errors.As(err, &chk.ErrLeaseNotAcquired{}) {
log.Errorf("Cannot get lease: %+v", err)
}
continue
Expand Down

0 comments on commit 022ec8d

Please sign in to comment.