Merge branch 'stable-2.16'
* stable-2.16:
Skip checks in batch-update when there are no updates
ZK compareAndPut should alway ignore the ignored refs
Remove redundant ref-database migration flag
Disable replication on startup
Validate replication.config at Gerrit startup
Write clear text data into ZK znodes
Differentiate the multi-site Gson from Gerrit's
Remove forwarded-aware event broker
Revert "Remove the thread-local event forwarding logic"
Use prim/backup logic for HAProxy reads
Deploy multi-site plugin as gerrit lib module
Ignore immutable and draft-comments Refs
Remove hardcoded localhost from ZK test
Wrap batch refupdate for shared refdb validation
Always run in migration mode
Fix NPE when creating new refs
Bind GitRepositoryManager and associated factories
Turn the multi-site plugin into a libModule
Reformat with google-java-format
Remove the thread-local event forwarding logic
Remove plugin name from logs message
Wrap Gerrit GitRepositoryManager into a Multisite fashion
Wrap JGit Repository into a Multisite fashion
Wrap RefDatabase into a MultiSite fashion
Validate JGit RefUpdates events
Add ref-database to local environment
Remove redundant RefDatabaseConfig
Always enable the shared ref-database
Give proper name to the shared ref-database config
Check for errors during downloading of plugins
Removed unused obsolete constant
Format config.md with wrapped lines and spaces
Add migration mode to support missing entries
Simplify the Zookeeper-based SharedDB code
Implement Zookeeper backend for consistency check
Fix directory creation in Docker test env
Requires Gerrit change I5b5a9432eb3db2559ce6a4393a42a4d227a371b5
Change-Id: Ic465e48a14376bb97e8ec10d69df66a5de87e201
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
deleted file mode 100644
index ff932da..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// 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.gerrit.server.notedb.NotesMigration;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-@Singleton
-public class GerritNoteDbStatus implements NoteDbStatus {
- private final NotesMigration notesMigration;
-
- @Inject
- public GerritNoteDbStatus(NotesMigration notesMigration) {
- this.notesMigration = notesMigration;
- }
-
- @Override
- public boolean enabled() {
- return notesMigration.commitChangeWrites();
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
index b933635..43fab21 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
@@ -16,12 +16,13 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.ModuleImpl;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gson.Gson;
import com.google.inject.CreationException;
import com.google.inject.Inject;
import com.google.inject.Provides;
-import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import com.google.inject.spi.Message;
import com.googlesource.gerrit.plugins.multisite.broker.BrokerGson;
@@ -46,15 +47,15 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+@ModuleImpl(name = GitRepositoryManager.LOCAL_REPOSITORY_MODULE)
public class Module extends LifecycleModule {
private static final Logger log = LoggerFactory.getLogger(Module.class);
- private Configuration config;
- private NoteDbStatus noteDb;
+ private final Configuration config;
private final boolean disableGitRepositoryValidation;
@Inject
- public Module(Configuration config, NoteDbStatus noteDb) {
- this(config, noteDb, false);
+ public Module(Configuration config) {
+ this(config, false);
}
// TODO: It is not possible to properly test the libModules in Gerrit.
@@ -62,23 +63,13 @@
// support
// in Gerrit for it.
@VisibleForTesting
- public Module(Configuration config, NoteDbStatus noteDb, boolean disableGitRepositoryValidation) {
- init(config, noteDb);
- this.disableGitRepositoryValidation = disableGitRepositoryValidation;
- }
-
- private void init(Configuration config, NoteDbStatus noteDb) {
+ public Module(Configuration config, boolean disableGitRepositoryValidation) {
this.config = config;
- this.noteDb = noteDb;
+ this.disableGitRepositoryValidation = disableGitRepositoryValidation;
}
@Override
protected void configure() {
- if (!noteDb.enabled()) {
- throw new ProvisionException(
- "Gerrit is still running on ReviewDb: please migrate to NoteDb "
- + "and then reload the multi-site plugin.");
- }
Collection<Message> validationErrors = config.validate();
if (!validationErrors.isEmpty()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java
deleted file mode 100644
index f47e503..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/NoteDbStatus.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// 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.ImplementedBy;
-
-/** Returns the status of changes migration. */
-@ImplementedBy(GerritNoteDbStatus.class)
-public interface NoteDbStatus {
-
- /**
- * Status of NoteDb migration.
- *
- * @return true if Gerrit has been migrated to NoteDb
- */
- boolean enabled();
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedAwareEventBroker.java
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
index 4b0a142..2d25c51 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
@@ -135,7 +135,7 @@
private void reindex(ChangeNotes notes) throws IOException, OrmException {
try (ManualRequestContext ctx = oneOffCtx.open()) {
notes.reload();
- indexer.index(ctx.getReviewDbProvider().get(), notes.getChange());
+ indexer.index(notes.getChange());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
index 29435bc..d96a8b2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
@@ -16,7 +16,6 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -148,16 +147,14 @@
}
private Optional<Long> computeLastChangeTs() throws OrmException {
- try (ReviewDb db = oneOffReqCtx.open().getReviewDbProvider().get()) {
- return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(db, notes));
- }
+ return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(notes));
}
- private long getTsFromChangeAndDraftComments(ReviewDb db, ChangeNotes notes) {
+ private long getTsFromChangeAndDraftComments(ChangeNotes notes) {
Change change = notes.getChange();
Timestamp changeTs = change.getLastUpdatedOn();
try {
- for (Comment comment : commentsUtil.draftByChange(db, changeNotes.get())) {
+ for (Comment comment : commentsUtil.draftByChange(changeNotes.get())) {
Timestamp commentTs = comment.writtenOn;
changeTs = commentTs.after(changeTs) ? commentTs : changeTs;
}
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index 9e29e71..980aa06 100644
--- a/src/main/resources/Documentation/about.md
+++ b/src/main/resources/Documentation/about.md
@@ -8,7 +8,6 @@
The masters must be:
-* migrated to NoteDb
* connected to the same message broker
* behind a load balancer (e.g., HAProxy)
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
index 759fe7c..5281371 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/ModuleTest.java
@@ -36,15 +36,13 @@
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Configuration configMock;
- @Mock private NoteDbStatus noteDb;
-
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
private Module module;
@Before
public void setUp() {
- module = new Module(configMock, noteDb);
+ module = new Module(configMock);
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
index 2793253..d6c3576 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -26,7 +26,6 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -34,7 +33,6 @@
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gwtorm.server.OrmException;
-import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
@@ -61,9 +59,7 @@
private static final boolean CHANGE_EXISTS = true;
private static final boolean CHANGE_DOES_NOT_EXIST = false;
private static final boolean DO_NOT_THROW_IO_EXCEPTION = false;
- private static final boolean DO_NOT_THROW_ORM_EXCEPTION = false;
private static final boolean THROW_IO_EXCEPTION = true;
- private static final boolean THROW_ORM_EXCEPTION = true;
private static final boolean CHANGE_UP_TO_DATE = true;
private static final boolean CHANGE_OUTDATED = false;
@@ -71,7 +67,6 @@
@Mock private ChangeIndexer indexerMock;
@Mock private OneOffRequestContext ctxMock;
@Mock private ManualRequestContext manualRequestContextMock;
- @Mock private ReviewDb dbMock;
@Mock private ChangeNotes changeNotes;
@Mock private Configuration configurationMock;
@Mock private Configuration.Index index;
@@ -87,7 +82,6 @@
@Before
public void setUp() throws Exception {
when(ctxMock.open()).thenReturn(manualRequestContextMock);
- when(manualRequestContextMock.getReviewDbProvider()).thenReturn(Providers.of(dbMock));
id = new Change.Id(TEST_CHANGE_NUMBER);
change = new Change(null, id, null, null, TimeUtil.nowTs());
when(changeNotes.getChange()).thenReturn(change);
@@ -103,7 +97,7 @@
public void changeIsIndexedWhenUpToDate() throws Exception {
setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_UP_TO_DATE);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -111,7 +105,7 @@
setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_OUTDATED);
handler.index(
TEST_CHANGE_ID, Operation.INDEX, Optional.of(new ChangeIndexEvent("foo", 1, false)));
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -125,21 +119,12 @@
setupChangeAccessRelatedMocks(CHANGE_DOES_NOT_EXIST, CHANGE_OUTDATED);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
verify(indexerMock, never()).delete(id);
- verify(indexerMock, never())
- .index(any(ReviewDb.class), any(Project.NameKey.class), any(Change.Id.class));
- }
-
- @Test
- public void schemaThrowsExceptionWhenLookingUpForChange() throws Exception {
- setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_ORM_EXCEPTION, CHANGE_UP_TO_DATE);
- exception.expect(OrmException.class);
- handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
+ verify(indexerMock, never()).index(any(Project.NameKey.class), any(Change.Id.class));
}
@Test
public void indexerThrowsIOExceptionTryingToIndexChange() throws Exception {
- setupChangeAccessRelatedMocks(
- CHANGE_EXISTS, DO_NOT_THROW_ORM_EXCEPTION, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE);
+ setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE);
exception.expect(IOException.class);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
}
@@ -156,13 +141,13 @@
return null;
})
.when(indexerMock)
- .index(any(ReviewDb.class), any(Change.class));
+ .index(any(Change.class));
assertThat(Context.isForwardedEvent()).isFalse();
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty());
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
@Test
@@ -175,7 +160,7 @@
throw new IOException("someMessage");
})
.when(indexerMock)
- .index(any(ReviewDb.class), any(Change.class));
+ .index(any(Change.class));
assertThat(Context.isForwardedEvent()).isFalse();
try {
@@ -186,36 +171,22 @@
}
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ReviewDb.class), any(Change.class));
+ verify(indexerMock, times(1)).index(any(Change.class));
}
private void setupChangeAccessRelatedMocks(boolean changeExist, boolean changeUpToDate)
throws Exception {
- setupChangeAccessRelatedMocks(
- changeExist, DO_NOT_THROW_ORM_EXCEPTION, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate);
+ setupChangeAccessRelatedMocks(changeExist, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate);
}
private void setupChangeAccessRelatedMocks(
- boolean changeExist, boolean ormException, boolean changeUpToDate)
- throws OrmException, IOException {
- setupChangeAccessRelatedMocks(
- changeExist, ormException, DO_NOT_THROW_IO_EXCEPTION, changeUpToDate);
- }
-
- private void setupChangeAccessRelatedMocks(
- boolean changeExists, boolean ormException, boolean ioException, boolean changeIsUpToDate)
- throws OrmException, IOException {
- if (ormException) {
- doThrow(new OrmException("")).when(ctxMock).open();
- }
-
+ boolean changeExists, boolean ioException, boolean changeIsUpToDate)
+ throws IOException, OrmException {
if (changeExists) {
when(changeCheckerFactoryMock.create(TEST_CHANGE_ID)).thenReturn(changeCheckerPresentMock);
when(changeCheckerPresentMock.getChangeNotes()).thenReturn(Optional.of(changeNotes));
if (ioException) {
- doThrow(new IOException("io-error"))
- .when(indexerMock)
- .index(any(ReviewDb.class), any(Change.class));
+ doThrow(new IOException("io-error")).when(indexerMock).index(any(Change.class));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
index 3e8b3ca..5d64d28 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/EventConsumerIT.java
@@ -39,7 +39,6 @@
import com.google.inject.TypeLiteral;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.Module;
-import com.googlesource.gerrit.plugins.multisite.NoteDbStatus;
import com.googlesource.gerrit.plugins.multisite.broker.BrokerGson;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
import java.io.IOException;
@@ -69,10 +68,6 @@
"com.googlesource.gerrit.plugins.multisite.kafka.consumer.EventConsumerIT$KafkaTestContainerModule";
private static final int QUEUE_POLL_TIMEOUT_MSECS = 10000;
- static {
- System.setProperty("gerrit.notedb", "ON");
- }
-
public static class KafkaTestContainerModule extends LifecycleModule {
public class KafkaStopAtShutdown implements LifecycleListener {
@@ -97,11 +92,11 @@
private final Module multiSiteModule;
@Inject
- public KafkaTestContainerModule(SitePaths sitePaths, NoteDbStatus noteDb) {
+ public KafkaTestContainerModule(SitePaths sitePaths) {
this.config =
new FileBasedConfig(
sitePaths.etc_dir.resolve(Configuration.MULTI_SITE_CONFIG).toFile(), FS.DETECTED);
- this.multiSiteModule = new Module(new Configuration(config, new Config()), noteDb, true);
+ this.multiSiteModule = new Module(new Configuration(config, new Config()), true);
}
@Override
@@ -127,6 +122,10 @@
config.setBoolean("kafka", "subscriber", "enabled", true);
config.setBoolean("ref-database", null, "enabled", false);
config.save();
+ Configuration multiSiteConfig = new Configuration(config, new Config());
+ bind(Configuration.class).toInstance(multiSiteConfig);
+
+ listener().toInstance(new KafkaStopAtShutdown(kafkaContainer));
return kafkaContainer;
}
