Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): add maxAttempts RetryOption #9215

Merged
merged 26 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7046ee5
adding max-retry-count to stop retries
Tulsishah Dec 29, 2023
92ca12a
Merge branch 'googleapis:main' into main
Tulsishah Jan 4, 2024
303b8e8
adding option to configure max-retry-count
Tulsishah Jan 4, 2024
fbb30ff
Replacing WithMaxRetry to WithMaxAttempts
Tulsishah Jan 5, 2024
c154295
git test failing
Tulsishah Jan 5, 2024
0c94854
git test failing
Tulsishah Jan 5, 2024
03aa6fe
adding unit tests for invoke.go
Tulsishah Jan 5, 2024
e060d3a
adding unit tests for invoke.go
Tulsishah Jan 5, 2024
f3ac62f
adding unit tests for invoke.go
Tulsishah Jan 5, 2024
17890c9
fixing comment
Tulsishah Jan 6, 2024
ca327a0
fixing git test
Tulsishah Jan 7, 2024
8527d68
Merge branch 'main' into configure_max_retry_count
Tulsishah Jan 7, 2024
a4912a8
fixing comments
Tulsishah Jan 11, 2024
ffa763d
Merge remote-tracking branch 'origin/configure_max_retry_count' into …
Tulsishah Jan 11, 2024
f604a31
adding more unit tests
Tulsishah Jan 11, 2024
af39147
small fix
Tulsishah Jan 11, 2024
c4e0c1f
Merge branch 'main' into configure_max_retry_count
Tulsishah Jan 11, 2024
1877bb4
converting int to pointer
Tulsishah Jan 11, 2024
ca59001
removing logic of settiing retrynever when maxAttempts equals to 0
Tulsishah Jan 15, 2024
de87569
removing unnecessay tests
Tulsishah Jan 15, 2024
57e1ed7
Merge branch 'main' into configure_max_retry_count
Tulsishah Jan 15, 2024
63eca5f
Merge branch 'main' into configure_max_retry_count
tritone Jan 17, 2024
253d65e
doc update
Tulsishah Jan 17, 2024
6b3ed1a
Merge remote-tracking branch 'origin/configure_max_retry_count' into …
Tulsishah Jan 17, 2024
889808f
doc update
Tulsishah Jan 17, 2024
c155efe
Merge branch 'main' into configure_max_retry_count
tritone Jan 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions storage/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,7 @@ func TestBucketRetryer(t *testing.T) {
Multiplier: 3,
}),
WithPolicy(RetryAlways),
WithMaxRetry(5),
WithErrorFunc(func(err error) bool { return false }))
},
want: &retryConfig{
Expand All @@ -1079,8 +1080,9 @@ func TestBucketRetryer(t *testing.T) {
Max: 30 * time.Second,
Multiplier: 3,
},
policy: RetryAlways,
shouldRetry: func(err error) bool { return false },
policy: RetryAlways,
maxRetryCount: 5,
shouldRetry: func(err error) bool { return false },
},
},
{
Expand All @@ -1105,6 +1107,15 @@ func TestBucketRetryer(t *testing.T) {
policy: RetryNever,
},
},
{
name: "set max retry count only",
call: func(b *BucketHandle) *BucketHandle {
return b.Retryer(WithMaxRetry(5))
},
want: &retryConfig{
maxRetryCount: 5,
},
},
{
name: "set ErrorFunc only",
call: func(b *BucketHandle) *BucketHandle {
Expand Down
3 changes: 3 additions & 0 deletions storage/invoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ func run(ctx context.Context, call func(ctx context.Context) error, retry *retry
ctxWithHeaders := setInvocationHeaders(ctx, invocationID, attempts)
err = call(ctxWithHeaders)
attempts++
if attempts == retry.maxRetryCount {
Tulsishah marked this conversation as resolved.
Show resolved Hide resolved
return true, err
}
return !errorFunc(err), err
})
}
Expand Down
29 changes: 23 additions & 6 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -2076,6 +2076,21 @@ func (wb *withBackoff) apply(config *retryConfig) {
config.backoff = &wb.backoff
}

// WithMaxRetry sets the maximum number of retries for operations that may fail.
func WithMaxRetry(maxRetryCount int) RetryOption {
Tulsishah marked this conversation as resolved.
Show resolved Hide resolved
return &withMaxRetry{
maxRetryCount: maxRetryCount,
}
}

type withMaxRetry struct {
Tulsishah marked this conversation as resolved.
Show resolved Hide resolved
maxRetryCount int
}

func (wb *withMaxRetry) apply(config *retryConfig) {
config.maxRetryCount = wb.maxRetryCount
}
Tulsishah marked this conversation as resolved.
Show resolved Hide resolved

// RetryPolicy describes the available policies for which operations should be
// retried. The default is `RetryIdempotent`.
type RetryPolicy int
Expand Down Expand Up @@ -2145,9 +2160,10 @@ func (wef *withErrorFunc) apply(config *retryConfig) {
}

type retryConfig struct {
backoff *gax.Backoff
policy RetryPolicy
shouldRetry func(err error) bool
backoff *gax.Backoff
policy RetryPolicy
shouldRetry func(err error) bool
maxRetryCount int
}

func (r *retryConfig) clone() *retryConfig {
Expand All @@ -2165,9 +2181,10 @@ func (r *retryConfig) clone() *retryConfig {
}

return &retryConfig{
backoff: bo,
policy: r.policy,
shouldRetry: r.shouldRetry,
backoff: bo,
policy: r.policy,
shouldRetry: r.shouldRetry,
maxRetryCount: r.maxRetryCount,
}
}

Expand Down
72 changes: 54 additions & 18 deletions storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@ func TestObjectRetryer(t *testing.T) {
Max: 30 * time.Second,
Multiplier: 3,
}),
WithMaxRetry(5),
WithPolicy(RetryAlways),
WithErrorFunc(func(err error) bool { return false }))
},
Expand All @@ -986,8 +987,9 @@ func TestObjectRetryer(t *testing.T) {
Max: 30 * time.Second,
Multiplier: 3,
},
policy: RetryAlways,
shouldRetry: func(err error) bool { return false },
maxRetryCount: 5,
policy: RetryAlways,
shouldRetry: func(err error) bool { return false },
},
},
{
Expand All @@ -1012,6 +1014,15 @@ func TestObjectRetryer(t *testing.T) {
policy: RetryNever,
},
},
{
name: "set max retry count only",
call: func(o *ObjectHandle) *ObjectHandle {
return o.Retryer(WithMaxRetry(11))
},
want: &retryConfig{
maxRetryCount: 11,
},
},
{
name: "set ErrorFunc only",
call: func(o *ObjectHandle) *ObjectHandle {
Expand Down Expand Up @@ -1063,6 +1074,7 @@ func TestClientSetRetry(t *testing.T) {
Max: 30 * time.Second,
Multiplier: 3,
}),
WithMaxRetry(5),
WithPolicy(RetryAlways),
WithErrorFunc(func(err error) bool { return false }),
},
Expand All @@ -1072,8 +1084,9 @@ func TestClientSetRetry(t *testing.T) {
Max: 30 * time.Second,
Multiplier: 3,
},
policy: RetryAlways,
shouldRetry: func(err error) bool { return false },
maxRetryCount: 5,
policy: RetryAlways,
shouldRetry: func(err error) bool { return false },
},
},
{
Expand All @@ -1097,6 +1110,15 @@ func TestClientSetRetry(t *testing.T) {
policy: RetryNever,
},
},
{
name: "set max retry count only",
clientOptions: []RetryOption{
WithMaxRetry(7),
},
want: &retryConfig{
maxRetryCount: 7,
},
},
{
name: "set ErrorFunc only",
clientOptions: []RetryOption{
Expand Down Expand Up @@ -1150,11 +1172,13 @@ func TestRetryer(t *testing.T) {
name: "object retryer configures retry",
objectOptions: []RetryOption{
WithPolicy(RetryAlways),
WithMaxRetry(5),
WithErrorFunc(ShouldRetry),
},
want: &retryConfig{
shouldRetry: ShouldRetry,
policy: RetryAlways,
shouldRetry: ShouldRetry,
maxRetryCount: 5,
policy: RetryAlways,
},
},
{
Expand All @@ -1166,6 +1190,7 @@ func TestRetryer(t *testing.T) {
Multiplier: 6,
}),
WithPolicy(RetryAlways),
WithMaxRetry(11),
WithErrorFunc(ShouldRetry),
},
want: &retryConfig{
Expand All @@ -1174,8 +1199,9 @@ func TestRetryer(t *testing.T) {
Max: time.Hour,
Multiplier: 6,
},
shouldRetry: ShouldRetry,
policy: RetryAlways,
shouldRetry: ShouldRetry,
maxRetryCount: 11,
policy: RetryAlways,
},
},
{
Expand All @@ -1187,6 +1213,7 @@ func TestRetryer(t *testing.T) {
Multiplier: 6,
}),
WithPolicy(RetryAlways),
WithMaxRetry(7),
WithErrorFunc(ShouldRetry),
},
want: &retryConfig{
Expand All @@ -1195,8 +1222,9 @@ func TestRetryer(t *testing.T) {
Max: time.Hour,
Multiplier: 6,
},
shouldRetry: ShouldRetry,
policy: RetryAlways,
shouldRetry: ShouldRetry,
maxRetryCount: 7,
policy: RetryAlways,
},
},
{
Expand All @@ -1206,11 +1234,13 @@ func TestRetryer(t *testing.T) {
},
objectOptions: []RetryOption{
WithPolicy(RetryNever),
WithMaxRetry(5),
WithErrorFunc(ShouldRetry),
},
want: &retryConfig{
policy: RetryNever,
shouldRetry: ShouldRetry,
policy: RetryNever,
maxRetryCount: 5,
shouldRetry: ShouldRetry,
},
},
{
Expand All @@ -1220,11 +1250,13 @@ func TestRetryer(t *testing.T) {
},
objectOptions: []RetryOption{
WithPolicy(RetryNever),
WithMaxRetry(11),
WithErrorFunc(ShouldRetry),
},
want: &retryConfig{
policy: RetryNever,
shouldRetry: ShouldRetry,
policy: RetryNever,
maxRetryCount: 11,
shouldRetry: ShouldRetry,
},
},
{
Expand All @@ -1243,10 +1275,12 @@ func TestRetryer(t *testing.T) {
Max: time.Microsecond,
}),
WithErrorFunc(ShouldRetry),
WithMaxRetry(5),
},
want: &retryConfig{
policy: RetryAlways,
shouldRetry: ShouldRetry,
policy: RetryAlways,
maxRetryCount: 5,
shouldRetry: ShouldRetry,
backoff: &gax.Backoff{
Initial: time.Nanosecond,
Max: time.Microsecond,
Expand Down Expand Up @@ -1280,6 +1314,7 @@ func TestRetryer(t *testing.T) {
bucketOptions: []RetryOption{
WithPolicy(RetryNever),
WithErrorFunc(ShouldRetry),
WithMaxRetry(5),
},
objectOptions: []RetryOption{
WithBackoff(gax.Backoff{
Expand All @@ -1288,8 +1323,9 @@ func TestRetryer(t *testing.T) {
}),
},
want: &retryConfig{
policy: RetryNever,
shouldRetry: ShouldRetry,
policy: RetryNever,
maxRetryCount: 5,
shouldRetry: ShouldRetry,
backoff: &gax.Backoff{
Initial: time.Nanosecond,
Max: time.Second,
Expand Down