Fix auto-replay of stored events during config reload
Use a proper configuration reload listener to allow ReplicationQueue
to be aware of what's going on and perform a proper stop/start sequence
instead of relying on a low-level WorkQueue access.
The configuration reload behaves now as a soft-restart so that
stored events parked on the filesystem won't be missed any more.
If the new configuration is invalid and fails to load, keep the
current one without retrying until it gets modified again. That avoids
the start/stop of the ReplicationQueue continuously.
Bug: Issue 10260
Change-Id: Ia34b460221a2e0512a4ffba66d1f08c201402917
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
index 6ed438a..7374c4a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -19,6 +19,7 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.WorkQueue;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.nio.file.Path;
@@ -31,17 +32,20 @@
private ReplicationFileBasedConfig currentConfig;
private long currentConfigTs;
+ private long lastFailedConfigTs;
private final SitePaths site;
- private final WorkQueue workQueue;
private final DestinationFactory destinationFactory;
private final Path pluginDataDir;
+ // Use Provider<> instead of injecting the ReplicationQueue because of circular dependency with
+ // ReplicationConfig
+ private final Provider<ReplicationQueue> replicationQueue;
@Inject
public AutoReloadConfigDecorator(
SitePaths site,
- WorkQueue workQueue,
DestinationFactory destinationFactory,
+ Provider<ReplicationQueue> replicationQueue,
@PluginData Path pluginDataDir)
throws ConfigInvalidException, IOException {
this.site = site;
@@ -49,7 +53,7 @@
this.pluginDataDir = pluginDataDir;
this.currentConfig = loadConfig();
this.currentConfigTs = getLastModified(currentConfig);
- this.workQueue = workQueue;
+ this.replicationQueue = replicationQueue;
}
private static long getLastModified(ReplicationFileBasedConfig cfg) {
@@ -71,25 +75,27 @@
}
private void reloadIfNeeded() {
- try {
- if (isAutoReload()) {
- long lastModified = getLastModified(currentConfig);
- if (lastModified > currentConfigTs) {
- ReplicationFileBasedConfig newConfig = loadConfig();
- newConfig.startup(workQueue);
- int discarded = currentConfig.shutdown();
-
- this.currentConfig = newConfig;
- this.currentConfigTs = lastModified;
+ if (isAutoReload()) {
+ ReplicationQueue queue = replicationQueue.get();
+ long lastModified = getLastModified(currentConfig);
+ try {
+ if (lastModified > currentConfigTs && lastModified > lastFailedConfigTs) {
+ queue.stop();
+ currentConfig = loadConfig();
+ currentConfigTs = lastModified;
+ lastFailedConfigTs = 0;
logger.atInfo().log(
- "Configuration reloaded: %d destinations, %d replication events discarded",
- currentConfig.getDestinations(FilterType.ALL).size(), discarded);
+ "Configuration reloaded: %d destinations",
+ currentConfig.getDestinations(FilterType.ALL).size());
}
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot reload replication configuration: keeping existing settings");
+ lastFailedConfigTs = lastModified;
+ return;
+ } finally {
+ queue.start();
}
- } catch (Exception e) {
- logger.atSevere().withCause(e).log(
- "Cannot reload replication configuration: keeping existing settings");
- return;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
index ae06f43..519d0b4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -91,9 +91,11 @@
@Override
public void start() {
- config.startup(workQueue);
- running = true;
- firePendingEvents();
+ if (!running) {
+ config.startup(workQueue);
+ running = true;
+ firePendingEvents();
+ }
}
@Override
