Skip to content

Commit

Permalink
Fix dynamodb HA lock race
Browse files Browse the repository at this point in the history
  • Loading branch information
Mahmoud Abdelsalam committed Apr 1, 2019
1 parent 2afc847 commit 32bdc51
Showing 1 changed file with 20 additions and 12 deletions.
32 changes: 20 additions & 12 deletions physical/dynamodb/dynamodb.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ func (l *DynamoDBLock) tryToLock(stop, success chan struct{}, errors chan error)
case <-stop:
ticker.Stop()
case <-ticker.C:
err := l.writeItem()
err := l.updateItem(true)
if err != nil {
if err, ok := err.(awserr.Error); ok {
// Don't report a condition check failure, this means that the lock
Expand All @@ -633,7 +633,8 @@ func (l *DynamoDBLock) periodicallyRenewLock(done chan struct{}) {
for {
select {
case <-ticker.C:
l.writeItem()
// This should not renew the lock if the lock was deleted from under you.
l.updateItem(false)
case <-done:
ticker.Stop()
return
Expand All @@ -643,9 +644,24 @@ func (l *DynamoDBLock) periodicallyRenewLock(done chan struct{}) {

// Attempts to put/update the dynamodb item using condition expressions to
// evaluate the TTL.
func (l *DynamoDBLock) writeItem() error {
func (l *DynamoDBLock) updateItem(createIfMissing bool) error {
now := time.Now()

conditionExpression := ""
if createIfMissing {
conditionExpression += "attribute_not_exists(#path) or " +
"attribute_not_exists(#key) or "
} else {
conditionExpression += "attribute_exists(#path) and " +
"attribute_exists(#key) and "
}

// To work when upgrading from older versions that did not include the
// Identity attribute, we first check if the attr doesn't exist, and if
// it does, then we check if the identity is equal to our own.
// We also write if the lock expired.
conditionExpression += "(attribute_not_exists(#identity) or #identity = :identity or #expires <= :now)"

_, err := l.backend.client.UpdateItem(&dynamodb.UpdateItemInput{
TableName: aws.String(l.backend.table),
Key: map[string]*dynamodb.AttributeValue{
Expand All @@ -657,15 +673,7 @@ func (l *DynamoDBLock) writeItem() error {
// A. identity is equal to our identity (or the identity doesn't exist)
// or
// B. The ttl on the item is <= to the current time
ConditionExpression: aws.String(
"attribute_not_exists(#path) or " +
"attribute_not_exists(#key) or " +
// To work when upgrading from older versions that did not include the
// Identity attribute, we first check if the attr doesn't exist, and if
// it does, then we check if the identity is equal to our own.
"(attribute_not_exists(#identity) or #identity = :identity) or " +
"#expires <= :now",
),
ConditionExpression: aws.String(conditionExpression),
ExpressionAttributeNames: map[string]*string{
"#path": aws.String("Path"),
"#key": aws.String("Key"),
Expand Down

0 comments on commit 32bdc51

Please sign in to comment.