Do not wait for async fetches to be completed
Fix an issue where async fetch were not executed
in background but the incoming REST-API was blocking until
the fetch completion, which was unintentional for async
operations.
Change-Id: I559c92863de89e82a2c392dbfb4b254ddaf33b92
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
index 7984873..88e2b1c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
@@ -31,14 +31,9 @@
import com.googlesource.gerrit.plugins.replication.pull.Source;
import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
-import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.TransportException;
@@ -66,14 +61,12 @@
String label,
Set<FetchRefSpec> refsSpecs,
PullReplicationApiRequestMetrics apiRequestMetrics)
- throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
- TimeoutException, TransportException {
+ throws InterruptedException, RemoteConfigurationMissingException, TransportException {
fetch(name, label, refsSpecs, ASYNC, Optional.of(apiRequestMetrics));
}
public void fetchSync(Project.NameKey name, String label, Set<FetchRefSpec> refsSpecs)
- throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
- TimeoutException, TransportException {
+ throws InterruptedException, RemoteConfigurationMissingException, TransportException {
fetch(name, label, refsSpecs, SYNC, Optional.empty());
}
@@ -83,8 +76,7 @@
Set<FetchRefSpec> refSpecs,
ReplicationType fetchType,
Optional<PullReplicationApiRequestMetrics> apiRequestMetrics)
- throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
- TimeoutException, TransportException {
+ throws InterruptedException, RemoteConfigurationMissingException, TransportException {
ReplicationState state =
fetchReplicationStateFactory.create(
new FetchResultProcessing.CommandProcessing(this, eventDispatcher.get()));
@@ -98,17 +90,8 @@
try {
if (fetchType == ReplicationType.ASYNC) {
state.markAllFetchTasksScheduled();
- List<Future<?>> futures = new ArrayList<>();
for (FetchRefSpec refSpec : refSpecs) {
- futures.add(source.get().schedule(name, refSpec, state, apiRequestMetrics));
- }
- int timeout = source.get().getTimeout();
- for (Future future : futures) {
- if (timeout == 0) {
- future.get();
- } else {
- future.get(timeout, TimeUnit.SECONDS);
- }
+ source.get().schedule(name, refSpec, state, apiRequestMetrics);
}
} else {
Optional<FetchOne> maybeFetch =
@@ -121,10 +104,7 @@
throw newTransportException(maybeFetch.get());
}
}
- } catch (ExecutionException
- | IllegalStateException
- | TimeoutException
- | InterruptedException e) {
+ } catch (IllegalStateException e) {
fetchStateLog.error("Exception during the fetch operation", e, state);
throw e;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java
index f5ee3a1..4437bef 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchJob.java
@@ -20,8 +20,6 @@
import com.google.inject.assistedinject.Assisted;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.BatchInput;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
import org.eclipse.jgit.errors.TransportException;
public class FetchJob implements Runnable {
@@ -53,11 +51,7 @@
public void run() {
try {
command.fetchAsync(project, batchInput.label, batchInput.getRefSpecs(), metrics);
- } catch (InterruptedException
- | ExecutionException
- | RemoteConfigurationMissingException
- | TimeoutException
- | TransportException e) {
+ } catch (InterruptedException | RemoteConfigurationMissingException | TransportException e) {
log.atSevere().withCause(e).log(
"Exception during the async fetch call for project %s, label %s and ref(s) name(s) %s",
project.get(), batchInput.label, batchInput.getRefSpecs());
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
index cb5758f..68f6bf0 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
@@ -37,9 +37,7 @@
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
import java.util.Optional;
import java.util.Set;
-import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeoutException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -200,19 +198,6 @@
}
@Test(expected = RestApiException.class)
- public void shouldThrowRestApiExceptionWhenIssueDuringPocessing() throws Exception {
- FetchAction.Input inputParams = new FetchAction.Input();
- inputParams.label = label;
- inputParams.refName = refName;
-
- doThrow(new ExecutionException(new RuntimeException()))
- .when(fetchCommand)
- .fetchSync(any(), any(), any());
-
- fetchAction.apply(projectResource, inputParams);
- }
-
- @Test(expected = RestApiException.class)
public void shouldThrowRestApiExceptionWhenIssueWithUrlParam() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
@@ -223,17 +208,6 @@
fetchAction.apply(projectResource, inputParams);
}
- @Test(expected = RestApiException.class)
- public void shouldThrowRestApiExceptionWhenTimeout() throws Exception {
- FetchAction.Input inputParams = new FetchAction.Input();
- inputParams.label = label;
- inputParams.refName = refName;
-
- doThrow(new TimeoutException()).when(fetchCommand).fetchSync(any(), any(), any());
-
- fetchAction.apply(projectResource, inputParams);
- }
-
@Test(expected = AuthException.class)
public void shouldThrowAuthExceptionWhenCallFetchActionCapabilityNotAssigned() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
