Skip to content

Commit

Permalink
fix(storage): align retry idempotency (part 1) (#4715)
Browse files Browse the repository at this point in the history
Certain non-idempotent operations were being retried by the
library, and certain idempotent operations were not. This
fixes the following operations:

Add retry: notifications.delete, service_account.get
Remove retry: bucket_acl.delete, bucket_acl.update,
  default_object_acl.delete, default_object_acl.update,
  object_acl.delete, object_acl.update, hmacKey.create

Additional PR on conditionally idempotent operations to come.
  • Loading branch information
tritone authored Sep 2, 2021
1 parent 0055466 commit ffa903e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 37 deletions.
44 changes: 15 additions & 29 deletions storage/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,9 @@ func (a *ACLHandle) bucketDefaultList(ctx context.Context) ([]ACLRule, error) {
}

func (a *ACLHandle) bucketDefaultDelete(ctx context.Context, entity ACLEntity) error {
return runWithRetry(ctx, func() error {
req := a.c.raw.DefaultObjectAccessControls.Delete(a.bucket, string(entity))
a.configureCall(ctx, req)
return req.Do()
})
req := a.c.raw.DefaultObjectAccessControls.Delete(a.bucket, string(entity))
a.configureCall(ctx, req)
return req.Do()
}

func (a *ACLHandle) bucketList(ctx context.Context) ([]ACLRule, error) {
Expand All @@ -161,24 +159,16 @@ func (a *ACLHandle) bucketSet(ctx context.Context, entity ACLEntity, role ACLRol
Entity: string(entity),
Role: string(role),
}
err := runWithRetry(ctx, func() error {
req := a.c.raw.BucketAccessControls.Update(a.bucket, string(entity), acl)
a.configureCall(ctx, req)
_, err := req.Do()
return err
})
if err != nil {
return err
}
return nil
req := a.c.raw.BucketAccessControls.Update(a.bucket, string(entity), acl)
a.configureCall(ctx, req)
_, err := req.Do()
return err
}

func (a *ACLHandle) bucketDelete(ctx context.Context, entity ACLEntity) error {
return runWithRetry(ctx, func() error {
req := a.c.raw.BucketAccessControls.Delete(a.bucket, string(entity))
a.configureCall(ctx, req)
return req.Do()
})
req := a.c.raw.BucketAccessControls.Delete(a.bucket, string(entity))
a.configureCall(ctx, req)
return req.Do()
}

func (a *ACLHandle) objectList(ctx context.Context) ([]ACLRule, error) {
Expand Down Expand Up @@ -214,18 +204,14 @@ func (a *ACLHandle) objectSet(ctx context.Context, entity ACLEntity, role ACLRol
req = a.c.raw.ObjectAccessControls.Update(a.bucket, a.object, string(entity), acl)
}
a.configureCall(ctx, req)
return runWithRetry(ctx, func() error {
_, err := req.Do()
return err
})
_, err := req.Do()
return err
}

func (a *ACLHandle) objectDelete(ctx context.Context, entity ACLEntity) error {
return runWithRetry(ctx, func() error {
req := a.c.raw.ObjectAccessControls.Delete(a.bucket, a.object, string(entity))
a.configureCall(ctx, req)
return req.Do()
})
req := a.c.raw.ObjectAccessControls.Delete(a.bucket, a.object, string(entity))
a.configureCall(ctx, req)
return req.Do()
}

func (a *ACLHandle) configureCall(ctx context.Context, call interface{ Header() http.Header }) {
Expand Down
7 changes: 1 addition & 6 deletions storage/hmac.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,7 @@ func (c *Client) CreateHMACKey(ctx context.Context, projectID, serviceAccountEma

setClientHeader(call.Header())

var hkPb *raw.HmacKey
var err error
err = runWithRetry(ctx, func() error {
hkPb, err = call.Context(ctx).Do()
return err
})
hkPb, err := call.Context(ctx).Do()
if err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion storage/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,7 @@ func (b *BucketHandle) DeleteNotification(ctx context.Context, id string) (err e
if b.userProject != "" {
call.UserProject(b.userProject)
}
return call.Context(ctx).Do()
return runWithRetry(ctx, func() error {
return call.Context(ctx).Do()
})
}
7 changes: 6 additions & 1 deletion storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -1769,7 +1769,12 @@ func setEncryptionHeaders(headers http.Header, key []byte, copySource bool) erro
// ServiceAccount fetches the email address of the given project's Google Cloud Storage service account.
func (c *Client) ServiceAccount(ctx context.Context, projectID string) (string, error) {
r := c.raw.Projects.ServiceAccount.Get(projectID)
res, err := r.Context(ctx).Do()
var res *raw.ServiceAccount
var err error
err = runWithRetry(ctx, func() error {
res, err = r.Context(ctx).Do()
return err
})
if err != nil {
return "", err
}
Expand Down

0 comments on commit ffa903e

Please sign in to comment.