Skip to content

Commit c34d885

Browse files
committed
Fix retrypolicy bug, see issue #646
1 parent d72067e commit c34d885

File tree

12 files changed

+361
-18
lines changed

12 files changed

+361
-18
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "bugfix",
4+
"description": "RetryPolicy bug fix: adding throttlingBackoffStrategy to `RetryPolicy.Builder`. see [#646](https://github.com/aws/aws-sdk-java-v2/issues/646)"
5+
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryPolicy.java

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import software.amazon.awssdk.core.retry.conditions.AndRetryCondition;
2424
import software.amazon.awssdk.core.retry.conditions.MaxNumberOfRetriesCondition;
2525
import software.amazon.awssdk.core.retry.conditions.RetryCondition;
26+
import software.amazon.awssdk.utils.ToString;
2627
import software.amazon.awssdk.utils.builder.CopyableBuilder;
2728
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
2829

@@ -54,7 +55,8 @@ private RetryPolicy(BuilderImpl builder) {
5455
this.throttlingBackoffStrategy = builder.throttlingBackoffStrategy;
5556
this.numRetries = builder.numRetries;
5657
this.retryConditionFromBuilder = builder.retryCondition;
57-
this.retryCondition = AndRetryCondition.create(MaxNumberOfRetriesCondition.create(numRetries), retryConditionFromBuilder);
58+
this.retryCondition = AndRetryCondition.create(MaxNumberOfRetriesCondition.create(numRetries),
59+
retryConditionFromBuilder);
5860
}
5961

6062
public RetryCondition retryCondition() {
@@ -74,7 +76,52 @@ public Integer numRetries() {
7476
}
7577

7678
public Builder toBuilder() {
77-
return builder().numRetries(numRetries).retryCondition(retryConditionFromBuilder).backoffStrategy(backoffStrategy);
79+
return builder().numRetries(numRetries)
80+
.retryCondition(retryConditionFromBuilder)
81+
.backoffStrategy(backoffStrategy)
82+
.throttlingBackoffStrategy(throttlingBackoffStrategy);
83+
}
84+
85+
@Override
86+
public String toString() {
87+
return ToString.builder("RetryPolicy")
88+
.add("numRetries", numRetries)
89+
.add("retryCondition", retryCondition)
90+
.add("backoffStrategy", backoffStrategy)
91+
.add("throttlingBackoffStrategy", throttlingBackoffStrategy)
92+
.build();
93+
}
94+
95+
@Override
96+
public boolean equals(Object o) {
97+
if (this == o) {
98+
return true;
99+
}
100+
if (o == null || getClass() != o.getClass()) {
101+
return false;
102+
}
103+
104+
final RetryPolicy that = (RetryPolicy) o;
105+
106+
if (!retryCondition.equals(that.retryCondition)) {
107+
return false;
108+
}
109+
if (!backoffStrategy.equals(that.backoffStrategy)) {
110+
return false;
111+
}
112+
if (!throttlingBackoffStrategy.equals(that.throttlingBackoffStrategy)) {
113+
return false;
114+
}
115+
return numRetries.equals(that.numRetries);
116+
}
117+
118+
@Override
119+
public int hashCode() {
120+
int result = retryCondition.hashCode();
121+
result = 31 * result + backoffStrategy.hashCode();
122+
result = 31 * result + throttlingBackoffStrategy.hashCode();
123+
result = 31 * result + numRetries.hashCode();
124+
return result;
78125
}
79126

80127
public static Builder builder() {
@@ -84,14 +131,17 @@ public static Builder builder() {
84131
public static RetryPolicy defaultRetryPolicy() {
85132
return RetryPolicy.builder()
86133
.backoffStrategy(BackoffStrategy.defaultStrategy())
134+
.throttlingBackoffStrategy(BackoffStrategy.defaultThrottlingStrategy())
87135
.numRetries(SdkDefaultRetrySetting.DEFAULT_MAX_RETRIES)
88136
.retryCondition(RetryCondition.defaultRetryCondition())
89137
.build();
90138
}
91139

92140
public static RetryPolicy none() {
93141
return RetryPolicy.builder()
142+
.numRetries(0)
94143
.backoffStrategy(BackoffStrategy.none())
144+
.throttlingBackoffStrategy(BackoffStrategy.none())
95145
.retryCondition(RetryCondition.none())
96146
.build();
97147
}
@@ -105,6 +155,10 @@ public interface Builder extends CopyableBuilder<Builder, RetryPolicy> {
105155

106156
BackoffStrategy backoffStrategy();
107157

158+
Builder throttlingBackoffStrategy(BackoffStrategy backoffStrategy);
159+
160+
BackoffStrategy throttlingBackoffStrategy();
161+
108162
Builder retryCondition(RetryCondition retryCondition);
109163

110164
RetryCondition retryCondition();
@@ -155,13 +209,19 @@ public BackoffStrategy backoffStrategy() {
155209
return backoffStrategy;
156210
}
157211

158-
public Builder throttlingBackoffStrategy(BackoffStrategy backoffStrategy) {
159-
this.backoffStrategy = backoffStrategy;
212+
@Override
213+
public Builder throttlingBackoffStrategy(BackoffStrategy throttlingBackoffStrategy) {
214+
this.throttlingBackoffStrategy = throttlingBackoffStrategy;
160215
return this;
161216
}
162217

218+
@Override
163219
public BackoffStrategy throttlingBackoffStrategy() {
164-
return backoffStrategy;
220+
return throttlingBackoffStrategy;
221+
}
222+
223+
public void setThrottlingBackoffStrategy(BackoffStrategy throttlingBackoffStrategy) {
224+
this.throttlingBackoffStrategy = throttlingBackoffStrategy;
165225
}
166226

167227
@Override

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Random;
2222
import software.amazon.awssdk.annotations.SdkPublicApi;
2323
import software.amazon.awssdk.core.retry.RetryPolicyContext;
24+
import software.amazon.awssdk.utils.ToString;
2425
import software.amazon.awssdk.utils.builder.CopyableBuilder;
2526
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
2627

@@ -122,4 +123,36 @@ public EqualJitterBackoffStrategy build() {
122123
return new EqualJitterBackoffStrategy(this);
123124
}
124125
}
126+
127+
@Override
128+
public boolean equals(Object o) {
129+
if (this == o) {
130+
return true;
131+
}
132+
if (o == null || getClass() != o.getClass()) {
133+
return false;
134+
}
135+
136+
final EqualJitterBackoffStrategy that = (EqualJitterBackoffStrategy) o;
137+
138+
if (!baseDelay.equals(that.baseDelay)) {
139+
return false;
140+
}
141+
return maxBackoffTime.equals(that.maxBackoffTime);
142+
}
143+
144+
@Override
145+
public int hashCode() {
146+
int result = baseDelay.hashCode();
147+
result = 31 * result + maxBackoffTime.hashCode();
148+
return result;
149+
}
150+
151+
@Override
152+
public String toString() {
153+
return ToString.builder("EqualJitterBackoffStrategy")
154+
.add("baseDelay", baseDelay)
155+
.add("maxBackoffTime", maxBackoffTime)
156+
.build();
157+
}
125158
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FixedDelayBackoffStrategy.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.time.Duration;
2121
import software.amazon.awssdk.annotations.SdkPublicApi;
2222
import software.amazon.awssdk.core.retry.RetryPolicyContext;
23+
import software.amazon.awssdk.utils.ToString;
2324

2425
/**
2526
* Simple backoff strategy that always uses a fixed delay for the delay before the next retry attempt.
@@ -41,4 +42,30 @@ public Duration computeDelayBeforeNextRetry(RetryPolicyContext context) {
4142
public static FixedDelayBackoffStrategy create(Duration fixedBackoff) {
4243
return new FixedDelayBackoffStrategy(fixedBackoff);
4344
}
45+
46+
@Override
47+
public boolean equals(Object o) {
48+
if (this == o) {
49+
return true;
50+
}
51+
if (o == null || getClass() != o.getClass()) {
52+
return false;
53+
}
54+
55+
final FixedDelayBackoffStrategy that = (FixedDelayBackoffStrategy) o;
56+
57+
return fixedBackoff.equals(that.fixedBackoff);
58+
}
59+
60+
@Override
61+
public int hashCode() {
62+
return fixedBackoff.hashCode();
63+
}
64+
65+
@Override
66+
public String toString() {
67+
return ToString.builder("FixedDelayBackoffStrategy")
68+
.add("fixedBackoff", fixedBackoff)
69+
.build();
70+
}
4471
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Random;
2222
import software.amazon.awssdk.annotations.SdkPublicApi;
2323
import software.amazon.awssdk.core.retry.RetryPolicyContext;
24+
import software.amazon.awssdk.utils.ToString;
2425
import software.amazon.awssdk.utils.builder.CopyableBuilder;
2526
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
2627

@@ -119,4 +120,36 @@ public FullJitterBackoffStrategy build() {
119120
return new FullJitterBackoffStrategy(this);
120121
}
121122
}
123+
124+
@Override
125+
public boolean equals(Object o) {
126+
if (this == o) {
127+
return true;
128+
}
129+
if (o == null || getClass() != o.getClass()) {
130+
return false;
131+
}
132+
133+
final FullJitterBackoffStrategy that = (FullJitterBackoffStrategy) o;
134+
135+
if (!baseDelay.equals(that.baseDelay)) {
136+
return false;
137+
}
138+
return maxBackoffTime.equals(that.maxBackoffTime);
139+
}
140+
141+
@Override
142+
public int hashCode() {
143+
int result = baseDelay.hashCode();
144+
result = 31 * result + maxBackoffTime.hashCode();
145+
return result;
146+
}
147+
148+
@Override
149+
public String toString() {
150+
return ToString.builder("FullJitterBackoffStrategy")
151+
.add("baseDelay", baseDelay)
152+
.add("maxBackoffTime", maxBackoffTime)
153+
.build();
154+
}
122155
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/conditions/AndRetryCondition.java

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@
1515

1616
package software.amazon.awssdk.core.retry.conditions;
1717

18-
import java.util.ArrayList;
1918
import java.util.Collections;
20-
import java.util.List;
19+
import java.util.HashSet;
20+
import java.util.Set;
2121
import software.amazon.awssdk.annotations.SdkPublicApi;
2222
import software.amazon.awssdk.core.retry.RetryPolicyContext;
23+
import software.amazon.awssdk.utils.ToString;
2324
import software.amazon.awssdk.utils.Validate;
2425

2526
/**
@@ -28,7 +29,7 @@
2829
@SdkPublicApi
2930
public final class AndRetryCondition implements RetryCondition {
3031

31-
private List<RetryCondition> conditions = new ArrayList<>();
32+
private Set<RetryCondition> conditions = new HashSet<>();
3233

3334
private AndRetryCondition(RetryCondition... conditions) {
3435
Collections.addAll(this.conditions, Validate.notEmpty(conditions, "%s cannot be empty.", "conditions"));
@@ -45,4 +46,30 @@ public boolean shouldRetry(RetryPolicyContext context) {
4546
public static AndRetryCondition create(RetryCondition... conditions) {
4647
return new AndRetryCondition(conditions);
4748
}
49+
50+
@Override
51+
public boolean equals(Object o) {
52+
if (this == o) {
53+
return true;
54+
}
55+
if (o == null || getClass() != o.getClass()) {
56+
return false;
57+
}
58+
59+
final AndRetryCondition that = (AndRetryCondition) o;
60+
61+
return conditions != null ? conditions.equals(that.conditions) : that.conditions == null;
62+
}
63+
64+
@Override
65+
public int hashCode() {
66+
return conditions != null ? conditions.hashCode() : 0;
67+
}
68+
69+
@Override
70+
public String toString() {
71+
return ToString.builder("AndRetryCondition")
72+
.add("conditions", conditions)
73+
.build();
74+
}
4875
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/conditions/MaxNumberOfRetriesCondition.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import software.amazon.awssdk.annotations.SdkPublicApi;
1919
import software.amazon.awssdk.core.retry.RetryPolicyContext;
20+
import software.amazon.awssdk.utils.ToString;
2021
import software.amazon.awssdk.utils.Validate;
2122

2223
/**
@@ -39,4 +40,30 @@ public boolean shouldRetry(RetryPolicyContext context) {
3940
public static MaxNumberOfRetriesCondition create(int maxNumberOfRetries) {
4041
return new MaxNumberOfRetriesCondition(maxNumberOfRetries);
4142
}
43+
44+
@Override
45+
public boolean equals(Object o) {
46+
if (this == o) {
47+
return true;
48+
}
49+
if (o == null || getClass() != o.getClass()) {
50+
return false;
51+
}
52+
53+
final MaxNumberOfRetriesCondition that = (MaxNumberOfRetriesCondition) o;
54+
55+
return maxNumberOfRetries == that.maxNumberOfRetries;
56+
}
57+
58+
@Override
59+
public int hashCode() {
60+
return maxNumberOfRetries;
61+
}
62+
63+
@Override
64+
public String toString() {
65+
return ToString.builder("MaxNumberOfRetriesCondition")
66+
.add("maxNumberOfRetries", maxNumberOfRetries)
67+
.build();
68+
}
4269
}

0 commit comments

Comments
 (0)