Move ForwarderIndexProjectHandler to use common retry logic
Adapt IndexProjecthandler to use common retry functionality by extending
the ForwardedIndexingHandlerWithRetries.
Bug: Issue 14332
Change-Id: Id62f5bc298a3755ee5d24d16787286ada263d95f
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
index ce6804e..3787a80 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
@@ -16,6 +16,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.index.project.ProjectIndexer;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.Configuration;
@@ -24,7 +25,6 @@
import com.googlesource.gerrit.plugins.multisite.index.ProjectChecker;
import java.util.Optional;
import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
/**
* Index a project using {@link ProjectIndexer}. This class is meant to be used on the receiving
@@ -34,78 +34,40 @@
*/
@Singleton
public class ForwardedIndexProjectHandler
- extends ForwardedIndexingHandler<String, ProjectIndexEvent> {
+ extends ForwardedIndexingHandlerWithRetries<String, ProjectIndexEvent> {
private final ProjectIndexer indexer;
- private final int retryInterval;
- private final int maxTries;
private final ProjectChecker projectChecker;
- private final ScheduledExecutorService indexExecutor;
@Inject
ForwardedIndexProjectHandler(
ProjectIndexer indexer,
ProjectChecker projectChecker,
+ OneOffRequestContext oneOffRequestContext,
@ForwardedIndexExecutor ScheduledExecutorService indexExecutor,
Configuration config) {
- super(config.index().numStripedLocks());
+ super(indexExecutor, config, oneOffRequestContext);
this.indexer = indexer;
- Configuration.Index indexConfig = config.index();
- this.retryInterval = indexConfig != null ? indexConfig.retryInterval() : 0;
- this.maxTries = indexConfig != null ? indexConfig.maxTries() : 0;
- this.indexExecutor = indexExecutor;
this.projectChecker = projectChecker;
}
@Override
protected void doIndex(String projectName, Optional<ProjectIndexEvent> event) {
- if (!attemptIndex(projectName, event)) {
- log.warn("First Attempt failed, scheduling again after {} msecs", retryInterval);
- rescheduleIndex(projectName, event, 1);
- }
+ attemptToIndex(projectName, event, 0);
}
- public boolean attemptIndex(String projectName, Optional<ProjectIndexEvent> event) {
- log.debug("Attempt to index project {}, event: [{}]", projectName, event);
- final Project.NameKey projectNameKey = Project.nameKey(projectName);
- if (projectChecker.isProjectUpToDate(projectNameKey)) {
- indexer.index(projectNameKey);
- log.debug("Project {} successfully indexed", projectName);
- return true;
- }
- return false;
+ @Override
+ protected void reindex(String id) {
+ indexer.index(Project.nameKey(id));
}
- public void rescheduleIndex(
- String projectName, Optional<ProjectIndexEvent> event, int retryCount) {
- if (retryCount > maxTries) {
- log.error(
- "Project {} could not be indexed after {} retries. index could be stale.",
- projectName,
- retryCount);
+ @Override
+ protected String indexName() {
+ return "project";
+ }
- return;
- }
-
- log.warn(
- "Retrying for the #{} time to index project {} after {} msecs",
- retryCount,
- projectName,
- retryInterval);
-
- indexExecutor.schedule(
- () -> {
- Context.setForwardedEvent(true);
- if (!attemptIndex(projectName, event)) {
- log.warn(
- "Attempt {} to index project {} failed, scheduling again after {} msecs",
- retryCount,
- projectName,
- retryInterval);
- rescheduleIndex(projectName, event, retryCount + 1);
- }
- },
- retryInterval,
- TimeUnit.MILLISECONDS);
+ @Override
+ protected void attemptToIndex(String id, Optional<ProjectIndexEvent> indexEvent, int retryCount) {
+ reindexAndCheckIsUpToDate(id, indexEvent, projectChecker, retryCount);
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectChecker.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectChecker.java
index ece2443..e040fce 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectChecker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectChecker.java
@@ -14,9 +14,7 @@
package com.googlesource.gerrit.plugins.multisite.index;
-import com.google.gerrit.entities.Project;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
/** Encapsulates the logic of verifying the up-to-date status of a project. */
-public interface ProjectChecker {
- boolean isProjectUpToDate(Project.NameKey projectName);
-}
+public interface ProjectChecker extends UpToDateChecker<ProjectIndexEvent> {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectCheckerImpl.java
index 548ccc0..ba2a7d6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectCheckerImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ProjectCheckerImpl.java
@@ -17,6 +17,8 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
+import java.util.Optional;
public class ProjectCheckerImpl implements ProjectChecker {
private final ProjectCache projectCache;
@@ -27,7 +29,7 @@
}
@Override
- public boolean isProjectUpToDate(Project.NameKey projectName) {
- return projectCache.get(projectName) != null;
+ public boolean isUpToDate(Optional<ProjectIndexEvent> indexEvent) {
+ return indexEvent.map(e -> Project.nameKey(e.projectName)).map(projectCache::get).isPresent();
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandlerTest.java
index 1102cd2..3ce5e14 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandlerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandlerTest.java
@@ -23,6 +23,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.index.project.ProjectIndexer;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation;
import com.googlesource.gerrit.plugins.multisite.index.ProjectChecker;
@@ -44,6 +45,7 @@
@Rule public ExpectedException exception = ExpectedException.none();
@Mock private ProjectIndexer indexerMock;
@Mock private Configuration configMock;
+ @Mock private OneOffRequestContext ctxMock;
@Mock private ProjectChecker projectCheckerMock;
@Mock private Configuration.Index indexMock;
@Mock private ScheduledExecutorService indexExecutorMock;
@@ -56,10 +58,10 @@
when(indexMock.numStripedLocks()).thenReturn(10);
when(indexMock.retryInterval()).thenReturn(0);
when(indexMock.maxTries()).thenReturn(2);
- when(projectCheckerMock.isProjectUpToDate(any())).thenReturn(true);
+ when(projectCheckerMock.isUpToDate(any())).thenReturn(true);
handler =
new ForwardedIndexProjectHandler(
- indexerMock, projectCheckerMock, indexExecutorMock, configMock);
+ indexerMock, projectCheckerMock, ctxMock, indexExecutorMock, configMock);
nameKey = "project/name";
}
@@ -116,13 +118,4 @@
verify(indexerMock).index(Project.nameKey(nameKey));
}
-
- @Test
- public void indexAttemptShouldFailWhenCheckerFails() throws Exception {
- handler =
- new ForwardedIndexProjectHandler(
- indexerMock, (projectName) -> false, indexExecutorMock, configMock);
-
- assertThat(handler.attemptIndex(nameKey, Optional.empty())).isFalse();
- }
}
