Merge branch 'stable-3.7'
* stable-3.7:
Bump Gerrit and plugin version to v3.7.4
Bump version to v3.6.3.4
Fix duplicate log appender creation
Bump version to v3.5.4.4
Bump version to v3.4.8.6
Consider robot comments as mutable ref for validation
Introduce isUpToDateUnchecked for silent global-refdb checks
Add Bazel build instructions
Define the artifact as Gerrit plugin
Set version to 3.7.2.1
Set version to v3.5.4.3
Don't unnecessarily format error message
[errorprone] Fix FloggerLogString errors
Set version to 3.4.8.4
Configure the set of refs prefixes to be ignored by the global-refdb
Fix flogger formatting issues spotted by errorprone
Set Gerrit and project version to 3.7.2
Bump the global-refdb version to 3.4.8.3
Make SharedRefDatabaseWrapper a singleton
Add warning when the GlobalRefDatabase is not injected
Bump the global-refdb version to 3.4.8.2
Add default implementation for the global-refdb logger
Change-Id: I4334b1065203d8f2e13e4d07e47e832e50934fea
diff --git a/BUILD b/BUILD
index be3ed79..7cb0408 100644
--- a/BUILD
+++ b/BUILD
@@ -7,12 +7,12 @@
"PLUGIN_DEPS",
"PLUGIN_DEPS_NEVERLINK",
"PLUGIN_TEST_DEPS",
+ "gerrit_plugin",
)
-java_library(
+gerrit_plugin(
name = "global-refdb",
srcs = glob(["src/main/java/**/*.java"]),
- deps = PLUGIN_DEPS_NEVERLINK,
)
junit_tests(
diff --git a/README.md b/README.md
index 8249d8c..81da677 100644
--- a/README.md
+++ b/README.md
@@ -19,4 +19,50 @@
## Metrics
Global ref-database expose metrics to measure the global ref-database operation latency.
-List of the available metrics can be found [here](./metrics.md).
\ No newline at end of file
+List of the available metrics can be found [here](./metrics.md).
+
+## How to build
+
+This libModule is built like a Gerrit in-tree plugin, using Bazelisk.
+
+### Build in Gerrit tree
+
+Create a symbolic link of the repository source to the Gerrit source tree /plugins/global-refdb directory.
+
+Example:
+
+git clone https://gerrit.googlesource.com/gerrit
+git clone https://gerrit.googlesource.com/modules/global-refdb
+cd gerrit/plugins
+ln -s ../../global-refdb .
+From the Gerrit source tree issue the command bazelsk build plugins/global-refdb
+
+Example:
+
+```
+bazelisk build plugins/global-refdb
+```
+
+The libModule jar file is created under basel-bin/plugins/global-refdb/global-refdb.jar
+
+To execute the tests run bazelisk test plugins/global-refdb/... from the Gerrit source tree.
+
+Example:
+
+```
+bazelisk test plugins/global-refdb/...
+```
+
+## How to import into Eclipse as a project
+
+Add `global-refdb` in the `CUSTOM_PLUGINS` section of the `tools/bzl/plugins.bzl`.
+
+Example:
+
+```
+CUSTOM_PLUGINS = [
+ "global-refdb",
+]
+```
+
+Run `tools/eclipse/project.py` for generating or updating the Eclipse project.
\ No newline at end of file
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabase.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabase.java
index dbc52e8..df1e20a 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabase.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/GlobalRefDatabase.java
@@ -33,6 +33,26 @@
boolean isUpToDate(Project.NameKey project, Ref ref) throws GlobalRefDbLockException;
/**
+ * Check in global ref-db if ref is up-to-date, without locking, warnings or checked exceptions.
+ *
+ * <p>Differently from the regular {@link #isUpToDate(com.google.gerrit.entities.Project.NameKey,
+ * Ref)} this method is suitable for checking the status of the local-ref against the global-refdb
+ * without having any intention to update its value in a transaction.
+ *
+ * <p>The concrete implementations of GlobalRefDatabase must provide a specific implementation for
+ * this method that does not create any warnings in the logs.
+ *
+ * @param project project name of the ref
+ * @param ref to be checked against global ref-db
+ * @return true if it is; false otherwise
+ * @since 3.4.8.5
+ */
+ default boolean isUpToDateUnchecked(Project.NameKey project, Ref ref) {
+ throw new UnsupportedOperationException(
+ "isUpToDateUnchecked() is not supported by " + this.getClass().getName());
+ }
+
+ /**
* Compare a reference, and put if it is up-to-date with the current.
*
* <p>Two reference match if and only if they satisfy the following:
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LibModuleLogFile.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LibModuleLogFile.java
index c2858b4..7f10ef4 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LibModuleLogFile.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/LibModuleLogFile.java
@@ -33,10 +33,11 @@
* @see org.apache.log4j.PatternLayout
*/
public LibModuleLogFile(SystemLog systemLog, String logName, Layout layout) {
- AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true);
Logger logger = LogManager.getLogger(logName);
- logger.removeAppender(logName);
- logger.addAppender(asyncAppender);
- logger.setAdditivity(false);
+ if (logger.getAppender(logName) == null) {
+ AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true);
+ logger.addAppender(asyncAppender);
+ logger.setAdditivity(false);
+ }
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
index 3086c26..40e168c 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidator.java
@@ -158,8 +158,9 @@
}
private Boolean isRefToBeIgnored(String refName) {
- Boolean isRefToBeIgnored = ignoredRefs.contains(refName);
- logger.atFine().log("Is project version update? %s", isRefToBeIgnored);
+ Boolean isRefToBeIgnored =
+ ignoredRefs.stream().anyMatch(ignoredRefPrefix -> refName.startsWith(ignoredRefPrefix));
+ logger.atFine().log("Is project version update? %b", isRefToBeIgnored);
return isRefToBeIgnored;
}
@@ -175,7 +176,7 @@
protected Boolean isGlobalProject(String projectName) {
Boolean isGlobalProject = projectsFilter.matches(projectName);
- logger.atFine().log("Is global project? %s", isGlobalProject);
+ logger.atFine().log("Is global project? %b", isGlobalProject);
return isGlobalProject;
}
@@ -216,12 +217,6 @@
refEnforcement.getPolicy(projectName, refPair.getName());
if (refEnforcementPolicy == EnforcePolicy.IGNORED) return;
- String errorMessage =
- String.format(
- "Not able to persist the data in Zookeeper for project '%s' and ref '%s',"
- + "the cluster is now in Split Brain since the commit has been "
- + "persisted locally but not in SharedRef the value %s",
- projectName, refPair.getName(), refPair.putValue);
boolean succeeded;
try {
succeeded =
@@ -235,6 +230,12 @@
}
if (!succeeded) {
+ String errorMessage =
+ String.format(
+ "Not able to persist the data in Zookeeper for project '%s' and ref '%s',"
+ + "the cluster is now in Split Brain since the commit has been "
+ + "persisted locally but not in SharedRef the value %s",
+ projectName, refPair.getName(), refPair.putValue);
throw new SharedDbSplitBrainException(errorMessage);
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
index 4349193..c3b41e8 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDatabaseWrapper.java
@@ -19,10 +19,12 @@
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.NoopSharedRefDatabase;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.metrics.Timer0.Context;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -32,7 +34,9 @@
* binding. Such instance is bound optionally and, in case no explicit binding is registered a
* {@link NoopSharedRefDatabase} instance is wrapped instead.
*/
+@Singleton
public class SharedRefDatabaseWrapper implements GlobalRefDatabase {
+ private static final FluentLogger log = FluentLogger.forEnclosingClass();
private static final GlobalRefDatabase NOOP_REFDB = new NoopSharedRefDatabase();
@Inject(optional = true)
@@ -131,6 +135,16 @@
}
private GlobalRefDatabase sharedRefDb() {
- return Optional.ofNullable(sharedRefDbDynamicItem).map(di -> di.get()).orElse(NOOP_REFDB);
+ if (sharedRefDbDynamicItem == null) {
+ log.atWarning().log("DynamicItem<GlobalRefDatabase> has not been injected");
+ }
+
+ return Optional.ofNullable(sharedRefDbDynamicItem)
+ .map(di -> di.get())
+ .orElseGet(
+ () -> {
+ log.atWarning().log("Using NOOP_REFDB");
+ return NOOP_REFDB;
+ });
}
}
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
index a607cdc..986175a 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbConfiguration.java
@@ -21,6 +21,7 @@
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import java.io.IOException;
@@ -102,9 +103,11 @@
public static final String SECTION = "ref-database";
public static final String ENABLE_KEY = "enabled";
public static final String SUBSECTION_ENFORCEMENT_RULES = "enforcementRules";
+ public static final String IGNORED_REFS_PREFIXES = "ignoredRefsPrefixes";
private final boolean enabled;
private final Multimap<EnforcePolicy, String> enforcementRules;
+ private final ImmutableSet<String> ignoredRefsPrefixes;
private SharedRefDatabase(Supplier<Config> cfg) {
enabled = getBoolean(cfg, SECTION, null, ENABLE_KEY, false);
@@ -113,6 +116,8 @@
enforcementRules.putAll(
policy, getList(cfg, SECTION, SUBSECTION_ENFORCEMENT_RULES, policy.name()));
}
+
+ ignoredRefsPrefixes = ImmutableSet.copyOf(getList(cfg, SECTION, null, IGNORED_REFS_PREFIXES));
}
/**
@@ -146,6 +151,16 @@
return enforcementRules;
}
+ /**
+ * Returns the set of refs prefixes that are ignored during the validation and enforcement of
+ * the global refdb.
+ *
+ * @return Set of ignored prefixes of ignored refs
+ */
+ public ImmutableSet<String> getIgnoredRefsPrefixes() {
+ return ignoredRefsPrefixes;
+ }
+
private List<String> getList(
Supplier<Config> cfg, String section, String subsection, String name) {
return ImmutableList.copyOf(cfg.get().getStringList(section, subsection, name));
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
index d2b1822..858ba18 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefLogger.java
@@ -14,9 +14,11 @@
package com.gerritforge.gerrit.globalrefdb.validation;
+import com.google.inject.ImplementedBy;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
+@ImplementedBy(Log4jSharedRefLogger.class)
public interface SharedRefLogger {
/**
diff --git a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java
index a765975..4e8b2f7 100644
--- a/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java
+++ b/src/main/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/SharedRefEnforcement.java
@@ -14,6 +14,8 @@
package com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb;
+import com.google.gerrit.entities.RefNames;
+
/** Type of enforcement to implement between the local and shared RefDb. */
public interface SharedRefEnforcement {
public enum EnforcePolicy {
@@ -54,7 +56,9 @@
default boolean isRefToBeIgnoredBySharedRefDb(String refName) {
return refName == null
|| refName.startsWith("refs/draft-comments")
- || (refName.startsWith("refs/changes") && !refName.endsWith("/meta"))
+ || (refName.startsWith("refs/changes")
+ && !refName.endsWith("/meta")
+ && !refName.endsWith(RefNames.ROBOT_COMMENTS_SUFFIX))
|| refName.startsWith("refs/cache-automerge");
}
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
index a9f0cb2..d8f5ff7 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement.EnforcePolicy;
+import com.google.gerrit.entities.RefNames;
import org.eclipse.jgit.lib.Ref;
import org.junit.Test;
@@ -39,6 +40,14 @@
}
@Test
+ public void aChangeRobotCommentsShouldNotBeIgnored() {
+ Ref robotCommentsMutableRef =
+ newRef("refs/changes/01/1" + RefNames.ROBOT_COMMENTS_SUFFIX, AN_OBJECT_ID_1);
+ assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, robotCommentsMutableRef.getName()))
+ .isEqualTo(EnforcePolicy.REQUIRED);
+ }
+
+ @Test
public void aCacheAutomergeShouldBeIgnored() {
Ref immutableChangeRef = newRef("refs/cache-automerge/01/1/1000000", AN_OBJECT_ID_1);
assertThat(refEnforcement.getPolicy(A_TEST_PROJECT_NAME, immutableChangeRef.getName()))
