Do not block ref-updates during the configuration reload
For setups with a large number of remote configurations, configuration
reload can be a time consuming operation. During the reload, the list of
all destinations is blocked by a synchronized and thus any ref-update
in Gerrit is blocked and cause a general outage until the replication
plugin configuration is reloaded.
This situation happens because of a ref-update post action
GitReferenceUpdatedListener.onGitReferenceUpdated(Event). This is
a blocking operation and it is calling another blocking method
ReplicationQueue.fire(NameKey,String, String, ReplicationState, boolean)
and finally synchronized method DestinationsCollection.getAll(FilterType)
is called. If DestinationCollection is during the configuration reload
ref-update have to wait on DestinationsCollection.getAll, this is causing
general lock for all operations against Gerrit instance.
With NoteDb, almost every operation in Gerrit causes a ref-update and,
therefore, would be blocked in that chain, causing a unique single sync
in the entire Gerrit JVM that can bring an entire server down.
Solution to that issue is to remove synchronization on
DestinationsCollection.getAll to stop blocking ref-updates. Removing
synchronize keyword is still thread safe because we store destinations
as an immutable list the only way of updating it is to replace it with
a new list. Since it is volatile, the update of destinations is atomic
and thread safe.
Bug: Issue 12592
Change-Id: Idcba1ef20a0ecde31e6a7ef6d827bf08746e5cb2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
index 71c38d0..27e3756 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
@@ -138,7 +138,7 @@
}
@Override
- public synchronized List<Destination> getAll(FilterType filterType) {
+ public List<Destination> getAll(FilterType filterType) {
Predicate<? super Destination> filter;
switch (filterType) {
case PROJECT_CREATION:
@@ -156,7 +156,7 @@
}
@Override
- public synchronized boolean isEmpty() {
+ public boolean isEmpty() {
return destinations.isEmpty();
}
