Log acquiring and releasing sharedref locks
Before mutating refs entries in the sharedref database, a lock is
acquired to manage concurrency and guarantee exclusive access to that
ref. The lock is then released once the ref
has been created/updated/deleted from the sharedref database.
Add logging of lock acquisition/release to the sharedref_log, since
locks are also held in the sharedref database.
Feature: Issue 11254
Change-Id: Ie9f0d0d3422d5a6c8191703b472db4aaadfba974
diff --git a/BUILD b/BUILD
index d03d116..d6e7348 100644
--- a/BUILD
+++ b/BUILD
@@ -46,6 +46,7 @@
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":multi-site__plugin",
"@curator-client//jar",
+ "@curator-recipes//jar",
"@curator-framework//jar",
"@curator-test//jar",
"@mockito//jar",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/LockWrapper.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/LockWrapper.java
new file mode 100644
index 0000000..0e018d3
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/LockWrapper.java
@@ -0,0 +1,50 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.multisite;
+
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+public class LockWrapper implements AutoCloseable {
+ public interface Factory {
+ LockWrapper create(
+ @Assisted("project") String project,
+ @Assisted("refName") String refName,
+ @Assisted("lock") AutoCloseable lock);
+ }
+
+ private final String project;
+ private final String refName;
+ private final AutoCloseable lock;
+ private final SharedRefLogger sharedRefLogger;
+
+ @Inject
+ public LockWrapper(
+ SharedRefLogger sharedRefLogger,
+ @Assisted("project") String project,
+ @Assisted("refName") String refName,
+ @Assisted("lock") AutoCloseable lock) {
+ this.lock = lock;
+ this.sharedRefLogger = sharedRefLogger;
+ this.project = project;
+ this.refName = refName;
+ }
+
+ @Override
+ public void close() throws Exception {
+ lock.close();
+ sharedRefLogger.logLockRelease(project, refName);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Log4jSharedRefLogger.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Log4jSharedRefLogger.java
index 36ed2a7..5930576 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Log4jSharedRefLogger.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Log4jSharedRefLogger.java
@@ -103,6 +103,16 @@
sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.DeleteProject(project)));
}
+ @Override
+ public void logLockAcquisition(String project, String refName) {
+ sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockAcquire(project, refName)));
+ }
+
+ @Override
+ public void logLockRelease(String project, String refName) {
+ sharedRefDBLog.info(gson.toJson(new SharedRefLogEntry.LockRelease(project, refName)));
+ }
+
@VisibleForTesting
public void setLogger(Logger logger) {
this.sharedRefDBLog = logger;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefDatabaseWrapper.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefDatabaseWrapper.java
index f21f185..7d8ae22 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefDatabaseWrapper.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefDatabaseWrapper.java
@@ -50,7 +50,9 @@
@Override
public AutoCloseable lockRef(String project, String refName) throws SharedLockException {
- return sharedRefDb.lockRef(project, refName);
+ AutoCloseable locker = sharedRefDb.lockRef(project, refName);
+ sharedRefLogger.logLockAcquisition(project, refName);
+ return locker;
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogEntry.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogEntry.java
index 88d38bb..d9e762f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogEntry.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogEntry.java
@@ -22,7 +22,9 @@
public enum Type {
UPDATE_REF,
DELETE_REF,
- DELETE_PROJECT
+ DELETE_PROJECT,
+ LOCK_ACQUIRE,
+ LOCK_RELEASE
}
public String projectName;
@@ -73,4 +75,26 @@
this.oldId = oldId;
}
}
+
+ public static class LockAcquire extends SharedRefLogEntry {
+
+ public String refName;
+
+ LockAcquire(String projectName, String refName) {
+ this.type = Type.LOCK_ACQUIRE;
+ this.projectName = projectName;
+ this.refName = refName;
+ }
+ }
+
+ public static class LockRelease extends SharedRefLogEntry {
+
+ public String refName;
+
+ LockRelease(String projectName, String refName) {
+ this.type = Type.LOCK_RELEASE;
+ this.projectName = projectName;
+ this.refName = refName;
+ }
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogger.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogger.java
index 0311a53..51f9ff0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogger.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/SharedRefLogger.java
@@ -22,4 +22,8 @@
void logRefUpdate(String project, Ref currRef, ObjectId newRefValue);
void logProjectDelete(String project);
+
+ void logLockAcquisition(String project, String refName);
+
+ void logLockRelease(String project, String refName);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
index 15c0a13..4e6bef5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidator.java
@@ -17,6 +17,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.multisite.LockWrapper;
import com.googlesource.gerrit.plugins.multisite.SharedRefDatabaseWrapper;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefDatabase;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedRefEnforcement;
@@ -47,9 +48,10 @@
SharedRefDatabaseWrapper sharedRefDb,
ValidationMetrics validationMetrics,
SharedRefEnforcement refEnforcement,
+ LockWrapper.Factory lockWrapperFactory,
@Assisted String projectName,
@Assisted RefDatabase refDb) {
- super(sharedRefDb, validationMetrics, refEnforcement, projectName, refDb);
+ super(sharedRefDb, validationMetrics, refEnforcement, lockWrapperFactory, projectName, refDb);
}
public void executeBatchUpdateWithValidation(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
index 5330bca..82d4ff0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidator.java
@@ -20,6 +20,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.multisite.LockWrapper;
import com.googlesource.gerrit.plugins.multisite.SharedRefDatabaseWrapper;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.OutOfSyncException;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.SharedDbSplitBrainException;
@@ -41,6 +42,7 @@
protected final ValidationMetrics validationMetrics;
protected final String projectName;
+ private final LockWrapper.Factory lockWrapperFactory;
protected final RefDatabase refDb;
protected final SharedRefEnforcement refEnforcement;
@@ -70,10 +72,12 @@
SharedRefDatabaseWrapper sharedRefDb,
ValidationMetrics validationMetrics,
SharedRefEnforcement refEnforcement,
+ LockWrapper.Factory lockWrapperFactory,
@Assisted String projectName,
@Assisted RefDatabase refDb) {
this.sharedRefDb = sharedRefDb;
this.validationMetrics = validationMetrics;
+ this.lockWrapperFactory = lockWrapperFactory;
this.refDb = refDb;
this.projectName = projectName;
this.refEnforcement = refEnforcement;
@@ -160,7 +164,9 @@
locks.addResourceIfNotExist(
String.format("%s-%s", projectName, refName),
- () -> sharedRefDb.lockRef(projectName, refName));
+ () ->
+ lockWrapperFactory.create(
+ projectName, refName, sharedRefDb.lockRef(projectName, refName)));
RefPair latestRefPair = getLatestLocalRef(refPair);
if (sharedRefDb.isUpToDate(projectName, latestRefPair.compareRef)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
index 087c4ed..86943f4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
@@ -21,6 +21,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Scopes;
import com.googlesource.gerrit.plugins.multisite.Configuration;
+import com.googlesource.gerrit.plugins.multisite.LockWrapper;
import com.googlesource.gerrit.plugins.multisite.Log4jSharedRefLogger;
import com.googlesource.gerrit.plugins.multisite.SharedRefLogger;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.CustomSharedRefEnforcementByProject;
@@ -43,6 +44,7 @@
install(new ReplicationExtensionPointModule());
bind(SharedRefLogger.class).to(Log4jSharedRefLogger.class);
+ factory(LockWrapper.Factory.class);
factory(MultiSiteRepository.Factory.class);
factory(MultiSiteRefDatabase.Factory.class);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidatorTest.java
index 29c0a0c..661a480 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/BatchRefUpdateValidatorTest.java
@@ -148,6 +148,7 @@
zkSharedRefDatabase,
new ValidationMetrics(new DisabledMetricMaker()),
sharedRefEnforcement,
+ new DummyLockWrapper(),
projectName,
diskRepo.getRefDatabase());
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DisabledSharedRefLogger.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DisabledSharedRefLogger.java
index 5fc66ad..37d5008 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DisabledSharedRefLogger.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DisabledSharedRefLogger.java
@@ -27,4 +27,10 @@
@Override
public void logProjectDelete(String project) {}
+
+ @Override
+ public void logLockAcquisition(String project, String refName) {}
+
+ @Override
+ public void logLockRelease(String project, String refName) {}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DummyLockWrapper.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DummyLockWrapper.java
new file mode 100644
index 0000000..1ff0429
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/DummyLockWrapper.java
@@ -0,0 +1,13 @@
+package com.googlesource.gerrit.plugins.multisite.validation;
+
+import com.googlesource.gerrit.plugins.multisite.LockWrapper;
+import org.junit.Ignore;
+
+@Ignore
+public class DummyLockWrapper implements LockWrapper.Factory {
+
+ @Override
+ public LockWrapper create(String project, String refName, AutoCloseable lock) {
+ return new LockWrapper(new DisabledSharedRefLogger(), project, refName, lock);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/Log4jSharedRefLoggerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/Log4jSharedRefLoggerTest.java
index 8b305c7..f368d41 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/Log4jSharedRefLoggerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/Log4jSharedRefLoggerTest.java
@@ -120,6 +120,32 @@
assertThat(gotLogEntry.committer).isNull();
}
+ @Test
+ public void shouldLogLockAcquisition() {
+ String refName = "refs/foo/bar";
+ log4jSharedRefLogger.logLockAcquisition(project.get(), refName);
+
+ SharedRefLogEntry.LockAcquire gotLogEntry =
+ gson.fromJson(logWriter.toString(), SharedRefLogEntry.LockAcquire.class);
+
+ assertThat(gotLogEntry.type).isEqualTo(SharedRefLogEntry.Type.LOCK_ACQUIRE);
+ assertThat(gotLogEntry.projectName).isEqualTo(project.get());
+ assertThat(gotLogEntry.refName).isEqualTo(refName);
+ }
+
+ @Test
+ public void shouldLogLockRelease() {
+ String refName = "refs/foo/bar";
+ log4jSharedRefLogger.logLockRelease(project.get(), refName);
+
+ SharedRefLogEntry.LockAcquire gotLogEntry =
+ gson.fromJson(logWriter.toString(), SharedRefLogEntry.LockAcquire.class);
+
+ assertThat(gotLogEntry.type).isEqualTo(SharedRefLogEntry.Type.LOCK_RELEASE);
+ assertThat(gotLogEntry.projectName).isEqualTo(project.get());
+ assertThat(gotLogEntry.refName).isEqualTo(refName);
+ }
+
private Log4jSharedRefLogger newLog4jSharedRefLogger() throws IOException {
final Log4jSharedRefLogger log4jSharedRefLogger =
new Log4jSharedRefLogger(new SystemLog(new SitePaths(newPath()), baseConfig), repoManager);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
index fb7b58d..730e558 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteBatchRefUpdateTest.java
@@ -155,6 +155,7 @@
sharedRefDb,
validationMetrics,
new DefaultSharedRefEnforcement(),
+ new DummyLockWrapper(),
projectName,
refDb);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
index 74a45e1..a9d8e76 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/MultiSiteRefUpdateTest.java
@@ -197,6 +197,7 @@
sharedRefDb,
validationMetrics,
new DefaultSharedRefEnforcement(),
+ new DummyLockWrapper(),
projectName,
refDb);
return RefUpdateValidator;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
index 4177b02..f0677b8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/RefUpdateValidatorTest.java
@@ -76,7 +76,12 @@
refUpdateValidator =
new RefUpdateValidator(
- sharedRefDb, validationMetrics, defaultRefEnforcement, A_TEST_PROJECT_NAME, localRefDb);
+ sharedRefDb,
+ validationMetrics,
+ defaultRefEnforcement,
+ new DummyLockWrapper(),
+ A_TEST_PROJECT_NAME,
+ localRefDb);
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseIT.java
index 3f10908..317f130 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/zookeeper/ZkSharedRefDatabaseIT.java
@@ -23,6 +23,7 @@
import com.googlesource.gerrit.plugins.multisite.SharedRefDatabaseWrapper;
import com.googlesource.gerrit.plugins.multisite.validation.BatchRefUpdateValidator;
import com.googlesource.gerrit.plugins.multisite.validation.DisabledSharedRefLogger;
+import com.googlesource.gerrit.plugins.multisite.validation.DummyLockWrapper;
import com.googlesource.gerrit.plugins.multisite.validation.MultiSiteBatchRefUpdate;
import com.googlesource.gerrit.plugins.multisite.validation.ValidationMetrics;
import com.googlesource.gerrit.plugins.multisite.validation.ZkConnectionConfig;
@@ -183,6 +184,7 @@
zkSharedRefDatabase,
new ValidationMetrics(new DisabledMetricMaker()),
new DefaultSharedRefEnforcement(),
+ new DummyLockWrapper(),
projectName,
refDb);
}
