Skip to content

Commit ef9776f

Browse files
Address part of race condition #149: add git retry headroom
Add some headroom among git clone retries (5s=60s:CC default polling period -55s: retry clone duration)
1 parent a0586ef commit ef9776f

File tree

3 files changed

+38
-2
lines changed

3 files changed

+38
-2
lines changed

cf-ops-automation-broker-core/src/main/java/com/orange/oss/cloudfoundry/broker/opsautomation/ondemandbroker/git/RetrierGitManager.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ public RetrierGitManager(String repositoryAliasName, GitManager gitManager, Retr
1818
this.retryPolicy = retryPolicy;
1919
this.retryPolicy
2020
.onRetry(e -> logger.warn("Transient (?) failure, retrying. Cause: {}", e.getLastFailure()))
21-
.onRetriesExceeded(e -> logger.warn("Aborting. Max attempts reached: #" + this.retryPolicy.getMaxAttempts() + " Rethrowing failure:" + e.getFailure()))
21+
.onRetriesExceeded(e -> logger.warn("Aborting. Max attempts reached: #" + this.retryPolicy.getMaxAttempts() +
22+
" or max duration reached (" + this.retryPolicy.getMaxDuration().toString() + "). Rethrowing failure:" + e.getFailure()))
2223
.handleIf(e -> isCauseSubclassOf(e, org.eclipse.jgit.api.errors.TransportException.class))
2324
;
2425
logger.debug("Configured for {} with retry policy {}", repositoryAliasName, ToStringBuilder.reflectionToString(retryPolicy));
26+
//Duration would typically be displayed as "PT2S" which means P (for duration prefixes), T (for time in units smaller than the day, in our case, usually seconds or minutes
27+
//Learn more at https://en.wikipedia.org/wiki/ISO_8601#Durations
2528
}
2629

2730
protected boolean isCauseSubclassOf(Throwable e, Class superClassToCheck) {

cf-ops-automation-broker-core/src/main/java/com/orange/oss/cloudfoundry/broker/opsautomation/ondemandbroker/git/RetryProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public class RetryProperties {
2323
* the max duration to perform retries for, else the execution will be failed.
2424
* See {@link RetryPolicy#withMaxDuration(Duration)}
2525
*/
26-
private int maxDurationMilliSeconds=60000;
26+
private int maxDurationMilliSeconds=50000;
2727

2828
public int getMaxAttempts() { return maxAttempts; }
2929
public void setMaxAttempts(int maxAttempts) { this.maxAttempts = maxAttempts; }

cf-ops-automation-broker-core/src/test/java/com/orange/oss/cloudfoundry/broker/opsautomation/ondemandbroker/git/RetrierGitManagerTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@
77
import org.junit.Test;
88
import org.junit.runner.RunWith;
99
import org.mockito.Mock;
10+
import org.mockito.internal.stubbing.answers.AnswersWithDelay;
11+
import org.mockito.internal.stubbing.answers.ThrowsException;
1012
import org.mockito.junit.MockitoJUnitRunner;
1113

14+
import java.time.Duration;
15+
1216
import static org.mockito.ArgumentMatchers.any;
1317
import static org.mockito.Mockito.*;
1418

@@ -20,6 +24,35 @@ public class RetrierGitManagerTest {
2024

2125

2226

27+
@Test
28+
public void retries_clones_until_max_duration_reached() {
29+
//Given a retrier is configured with a retry policy and a max retry duration of 2s
30+
RetryPolicy<Object> retryPolicy = new RetryPolicy<>()
31+
.withMaxAttempts(3)
32+
.withMaxDuration(Duration.ofMillis(2*1000));
33+
GitManager retrier = new RetrierGitManager("repoAlias", gitManager, retryPolicy);
34+
35+
//Given 2 network problems when trying to clone
36+
TransportException gitException = new TransportException("https://elpaaso-gitlab.mycompany.com/paas-templates.git: 502 Bad Gateway");
37+
IllegalArgumentException wrappedException = new IllegalArgumentException(gitException);
38+
//Inject delay, see https://stackoverflow.com/questions/12813881/can-i-delay-a-stubbed-method-response-with-mockito
39+
doAnswer( new AnswersWithDelay( 3*1000, new ThrowsException(wrappedException))). //1st attempt consumming max retry time budget
40+
doNothing(). //2nd attempt that should not happen
41+
doNothing(). //3nd attempt that should not happen
42+
when(gitManager).cloneRepo(any());
43+
44+
try {
45+
//when trying to clone
46+
retrier.cloneRepo(new Context());
47+
//then it rethrows the exception
48+
Assertions.fail("expected max attempts reached to rethrow last exception, as max duration exceeded");
49+
} catch (IllegalArgumentException e) {
50+
verify(gitManager, times(1)).cloneRepo(any());
51+
//success
52+
}
53+
54+
}
55+
2356
@Test
2457
public void retries_clones() {
2558
//Given a retrier is configured with a retry policy

0 commit comments

Comments
 (0)