opendev/gerrit
Code Issues Proposed changes

Merge "Retry more often on submit"

Browse Source
This commit is contained in:
Gal Paikin
2021-02-08 11:34:33 +00:00
committed by Gerrit Code Review
parent 34dfd88bb5 d072ca5648
commit 8b257cf545
2 changed files with 4 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
java/com/google/gerrit/server/submit/MergeOp.java
View File

@@ -533,6 +533,9 @@ public class MergeOp implements AutoCloseable {
// Multiply the timeout by the number of projects we're actually attempting to
// submit.
.defaultTimeoutMultiplier(cs.projects().size())
// By default, we only retry lock failures. Here it's better to also retry unexpected
// runtime exceptions.
.retryOn(t -> t instanceof RuntimeException)
.call();
submissionExecutor.afterExecutions(orm);

46
javatests/com/google/gerrit/acceptance/rest/TraceIT.java
View File

@@ -43,7 +43,6 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.httpd.restapi.ParameterParser;
import com.google.gerrit.httpd.restapi.RestApiServlet;
import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.validators.CommitValidationException;
@@ -713,7 +712,7 @@ public class TraceIT extends AbstractDaemonTest {
@Test
@GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
public void autoRetryWithTrace() throws Exception {
public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
String changeId = createChange().getChangeId();
approve(changeId);
@@ -723,49 +722,6 @@ public class TraceIT extends AbstractDaemonTest {
RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).startsWith("retry-on-failure-");
assertThat(traceSubmitRule.traceId).startsWith("retry-on-failure-");
assertThat(traceSubmitRule.isLoggingForced).isTrue();
}
}
@Test
@GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
String changeId = createChange().getChangeId();
approve(changeId);
TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
traceSubmitRule.failAlways = true;
try (Registration registration =
extensionRegistry
.newRegistration()
.add(traceSubmitRule)
.add(
new ExceptionHook() {
@Override
public boolean shouldRetry(String actionType, String actionName, Throwable t) {
return true;
}
})) {
RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
assertThat(traceSubmitRule.traceId).isNull();
assertThat(traceSubmitRule.isLoggingForced).isFalse();
}
}
@Test
public void noAutoRetryWithTraceIfDisabled() throws Exception {
String changeId = createChange().getChangeId();
approve(changeId);
TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
traceSubmitRule.failOnce = true;
try (Registration registration = extensionRegistry.newRegistration().add(traceSubmitRule)) {
RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
assertThat(traceSubmitRule.traceId).isNull();
assertThat(traceSubmitRule.isLoggingForced).isFalse();
}