From b1e340cf6656e101ce3d5fb0b6e370675471d115 Mon Sep 17 00:00:00 2001 From: Paul Seiffert Date: Tue, 9 May 2017 18:16:24 +0200 Subject: [PATCH 1/2] Check for children more efficiently --- physical/dynamodb.go | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/physical/dynamodb.go b/physical/dynamodb.go index 4c7cefbfe968..aee605b566f8 100644 --- a/physical/dynamodb.go +++ b/physical/dynamodb.go @@ -317,11 +317,11 @@ func (d *DynamoDBBackend) Delete(key string) error { prefixes := prefixes(key) sort.Sort(sort.Reverse(sort.StringSlice(prefixes))) for _, prefix := range prefixes { - items, err := d.List(prefix) + hasChildren, err := d.hasChildren(prefix) if err != nil { return err } - if len(items) == 1 { + if !hasChildren { requests = append(requests, &dynamodb.WriteRequest{ DeleteRequest: &dynamodb.DeleteRequest{ Key: map[string]*dynamodb.AttributeValue{ @@ -378,6 +378,39 @@ func (d *DynamoDBBackend) List(prefix string) ([]string, error) { return keys, nil } +// hasChildren returns true if there exist items below a certain path prefix. +// To do so, the method fetches such items from DynamoDB. If there are more than one item (which is the "directory" +// item), there are children. +func (d *DynamoDBBackend) hasChildren(prefix string) (bool, error) { + prefix = strings.TrimSuffix(prefix, "/") + prefix = escapeEmptyPath(prefix) + + queryInput := &dynamodb.QueryInput{ + TableName: aws.String(d.table), + ConsistentRead: aws.Bool(true), + KeyConditions: map[string]*dynamodb.Condition{ + "Path": { + ComparisonOperator: aws.String("EQ"), + AttributeValueList: []*dynamodb.AttributeValue{{ + S: aws.String(prefix), + }}, + }, + }, + // Avoid fetching too many items from DynamoDB for performance reasons. + // We need at least two because one is the directory item, all others are children. + Limit: aws.Int64(2), + } + + d.permitPool.Acquire() + defer d.permitPool.Release() + + out, err := d.client.Query(queryInput) + if err != nil { + return false, err + } + return len(out.Items) > 1, nil +} + // LockWith is used for mutual exclusion based on the given key. func (d *DynamoDBBackend) LockWith(key, value string) (Lock, error) { identity, err := uuid.GenerateUUID() From cb70a5c5ff4d1cc474e4bbe33b6bcc0fbadc4ce2 Mon Sep 17 00:00:00 2001 From: Paul Seiffert Date: Sun, 14 May 2017 09:18:02 +0200 Subject: [PATCH 2/2] Wrap comments to a width of 80 --- physical/dynamodb.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/physical/dynamodb.go b/physical/dynamodb.go index aee605b566f8..582640273de9 100644 --- a/physical/dynamodb.go +++ b/physical/dynamodb.go @@ -379,8 +379,8 @@ func (d *DynamoDBBackend) List(prefix string) ([]string, error) { } // hasChildren returns true if there exist items below a certain path prefix. -// To do so, the method fetches such items from DynamoDB. If there are more than one item (which is the "directory" -// item), there are children. +// To do so, the method fetches such items from DynamoDB. If there are more +// than one item (which is the "directory" item), there are children. func (d *DynamoDBBackend) hasChildren(prefix string) (bool, error) { prefix = strings.TrimSuffix(prefix, "/") prefix = escapeEmptyPath(prefix) @@ -397,7 +397,8 @@ func (d *DynamoDBBackend) hasChildren(prefix string) (bool, error) { }, }, // Avoid fetching too many items from DynamoDB for performance reasons. - // We need at least two because one is the directory item, all others are children. + // We need at least two because one is the directory item, all others + // are children. Limit: aws.Int64(2), }