Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
Reformati with GJF 1.7
Sanitize license headers
Improve plugin documentation
Do not write into SharedRef DB when git update fails
Add remote debug option to the local Gerrit setup
Edit the design doc for readability. No content changes.
Change-Id: I3a80b418be404a5c4808d734ba7ad87bc9a5bbc5
diff --git a/README.md b/README.md
index 3e57d63..7c731af 100644
--- a/README.md
+++ b/README.md
@@ -76,6 +76,7 @@
```
[gerrit]
+ installDbModule = com.googlesource.gerrit.plugins.multisite.GitModule
installModule = com.googlesource.gerrit.plugins.multisite.Module
```
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/GitModule.java
similarity index 64%
rename from src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
rename to src/main/java/com/googlesource/gerrit/plugins/multisite/GitModule.java
index ff932da..83cc99b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/GerritNoteDbStatus.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/GitModule.java
@@ -14,21 +14,22 @@
package com.googlesource.gerrit.plugins.multisite;
-import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
-import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.multisite.validation.ValidationModule;
-@Singleton
-public class GerritNoteDbStatus implements NoteDbStatus {
- private final NotesMigration notesMigration;
+public class GitModule extends AbstractModule {
+ private final Configuration config;
@Inject
- public GerritNoteDbStatus(NotesMigration notesMigration) {
- this.notesMigration = notesMigration;
+ public GitModule(Configuration config) {
+ this.config = config;
}
@Override
- public boolean enabled() {
- return notesMigration.commitChangeWrites();
+ protected void configure() {
+ if (config.getZookeeperConfig().isEnabled()) {
+ install(new ValidationModule(config));
+ }
}
}
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 5e91c80..7117cd6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
@@ -14,14 +14,12 @@
package com.googlesource.gerrit.plugins.multisite;
-import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.config.SitePaths;
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;
@@ -33,7 +31,6 @@
import com.googlesource.gerrit.plugins.multisite.index.IndexModule;
import com.googlesource.gerrit.plugins.multisite.kafka.consumer.KafkaConsumerModule;
import com.googlesource.gerrit.plugins.multisite.kafka.router.ForwardedEventRouterModule;
-import com.googlesource.gerrit.plugins.multisite.validation.ValidationModule;
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileReader;
@@ -48,37 +45,15 @@
public class Module extends LifecycleModule {
private static final Logger log = LoggerFactory.getLogger(Module.class);
- private Configuration config;
- private NoteDbStatus noteDb;
- private final boolean disableGitRepositoryValidation;
+ private final Configuration config;
@Inject
- public Module(Configuration config, NoteDbStatus noteDb) {
- this(config, noteDb, false);
- }
-
- // TODO: It is not possible to properly test the libModules in Gerrit.
- // Disable the Git repository validation during integration test and then build the necessary
- // 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) {
this.config = config;
- this.noteDb = noteDb;
}
@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()) {
@@ -108,9 +83,6 @@
install(new BrokerForwarderModule(config.kafkaPublisher()));
}
- install(
- new ValidationModule(
- config, disableGitRepositoryValidation || !config.getZookeeperConfig().isEnabled()));
bind(Gson.class)
.annotatedWith(BrokerGson.class)
.toProvider(GsonProvider.class)
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/ForwardedEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandler.java
index 85dab30..278ba4e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandler.java
@@ -19,7 +19,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.slf4j.Logger;
@@ -47,9 +46,8 @@
* Dispatch an event in the local node, event will not be forwarded to the other node.
*
* @param event The event to dispatch
- * @throws OrmException If an error occur while retrieving the change the event belongs to.
*/
- public void dispatch(Event event) throws OrmException, PermissionBackendException {
+ public void dispatch(Event event) throws PermissionBackendException {
try (ManualRequestContext ctx = oneOffCtx.open()) {
Context.setForwardedEvent(true);
log.debug("dispatching event {}", event.getType());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java
index 29bfa44..31256b7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java
@@ -20,7 +20,6 @@
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.AccountIndexEvent;
-import java.io.IOException;
import java.util.Optional;
/**
@@ -41,7 +40,7 @@
}
@Override
- protected void doIndex(Account.Id id, Optional<AccountIndexEvent> event) throws IOException {
+ protected void doIndex(Account.Id id, Optional<AccountIndexEvent> event) {
indexer.index(id);
log.debug("Account {} successfully indexed", id);
}
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..118c9df 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
@@ -18,10 +18,8 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.Configuration;
@@ -30,7 +28,6 @@
import com.googlesource.gerrit.plugins.multisite.index.ChangeChecker;
import com.googlesource.gerrit.plugins.multisite.index.ChangeCheckerImpl;
import com.googlesource.gerrit.plugins.multisite.index.ForwardedIndexExecutor;
-import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
@@ -70,72 +67,48 @@
}
@Override
- protected void doIndex(String id, Optional<ChangeIndexEvent> indexEvent)
- throws IOException, OrmException {
+ protected void doIndex(String id, Optional<ChangeIndexEvent> indexEvent) {
doIndex(id, indexEvent, 0);
}
- private void doIndex(String id, Optional<ChangeIndexEvent> indexEvent, int retryCount)
- throws IOException, OrmException {
- try {
- ChangeChecker checker = changeCheckerFactory.create(id);
- Optional<ChangeNotes> changeNotes = checker.getChangeNotes();
- if (changeNotes.isPresent()) {
- ChangeNotes notes = changeNotes.get();
- reindex(notes);
+ private void doIndex(String id, Optional<ChangeIndexEvent> indexEvent, int retryCount) {
+ ChangeChecker checker = changeCheckerFactory.create(id);
+ Optional<ChangeNotes> changeNotes = checker.getChangeNotes();
+ if (changeNotes.isPresent()) {
+ ChangeNotes notes = changeNotes.get();
+ reindex(notes);
- if (checker.isChangeUpToDate(indexEvent)) {
- if (retryCount > 0) {
- log.warn("Change {} has been eventually indexed after {} attempt(s)", id, retryCount);
- } else {
- log.debug("Change {} successfully indexed", id);
- }
+ if (checker.isChangeUpToDate(indexEvent)) {
+ if (retryCount > 0) {
+ log.warn("Change {} has been eventually indexed after {} attempt(s)", id, retryCount);
} else {
- log.warn(
- "Change {} seems too old compared to the event timestamp (event={} >> change-Ts={})",
- id,
- indexEvent,
- checker);
- rescheduleIndex(id, indexEvent, retryCount + 1);
+ log.debug("Change {} successfully indexed", id);
}
} else {
log.warn(
- "Change {} not present yet in local Git repository (event={}) after {} attempt(s)",
+ "Change {} seems too old compared to the event timestamp (event={} >> change-Ts={})",
id,
indexEvent,
- retryCount);
- if (!rescheduleIndex(id, indexEvent, retryCount + 1)) {
- log.error(
- "Change {} could not be found in the local Git repository (event={})",
- id,
- indexEvent);
- }
+ checker);
+ rescheduleIndex(id, indexEvent, retryCount + 1);
}
- } catch (Exception e) {
- if (isCausedByNoSuchChangeException(e)) {
- indexer.delete(parseChangeId(id));
- log.warn("Error trying to index Change {}. Deleted from index", id, e);
- return;
+ } else {
+ log.warn(
+ "Change {} not present yet in local Git repository (event={}) after {} attempt(s)",
+ id,
+ indexEvent,
+ retryCount);
+ if (!rescheduleIndex(id, indexEvent, retryCount + 1)) {
+ log.error(
+ "Change {} could not be found in the local Git repository (event={})", id, indexEvent);
}
-
- throw e;
}
}
- private static boolean isCausedByNoSuchChangeException(Throwable throwable) {
- while (throwable != null) {
- if (throwable instanceof NoSuchChangeException) {
- return true;
- }
- throwable = throwable.getCause();
- }
- return false;
- }
-
- private void reindex(ChangeNotes notes) throws IOException, OrmException {
+ private void reindex(ChangeNotes notes) {
try (ManualRequestContext ctx = oneOffCtx.open()) {
notes.reload();
- indexer.index(ctx.getReviewDbProvider().get(), notes.getChange());
+ indexer.index(notes.getChange());
}
}
@@ -169,7 +142,7 @@
}
@Override
- protected void doDelete(String id, Optional<ChangeIndexEvent> indexEvent) throws IOException {
+ protected void doDelete(String id, Optional<ChangeIndexEvent> indexEvent) {
indexer.delete(parseChangeId(id));
log.debug("Change {} successfully deleted from index", id);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java
index 76ce260..368dffe 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java
@@ -16,12 +16,10 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.index.group.GroupIndexer;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.GroupIndexEvent;
-import java.io.IOException;
import java.util.Optional;
/**
@@ -41,8 +39,7 @@
}
@Override
- protected void doIndex(String uuid, Optional<GroupIndexEvent> event)
- throws IOException, OrmException {
+ protected void doIndex(String uuid, Optional<GroupIndexEvent> event) {
indexer.index(new AccountGroup.UUID(uuid));
log.debug("Group {} successfully indexed", uuid);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
index ff2e111..e5f7e10 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
@@ -22,7 +22,6 @@
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
import com.googlesource.gerrit.plugins.multisite.index.ForwardedIndexExecutor;
import com.googlesource.gerrit.plugins.multisite.index.ProjectChecker;
-import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
@@ -58,15 +57,14 @@
}
@Override
- protected void doIndex(String projectName, Optional<ProjectIndexEvent> event) throws IOException {
+ protected void doIndex(String projectName, Optional<ProjectIndexEvent> event) {
if (!attemptIndex(projectName, event)) {
log.warn("First Attempt failed, scheduling again after {} msecs", retryInterval);
rescheduleIndex(projectName, event, 1);
}
}
- public boolean attemptIndex(String projectName, Optional<ProjectIndexEvent> event)
- throws IOException {
+ public boolean attemptIndex(String projectName, Optional<ProjectIndexEvent> event) {
log.debug("Attempt to index project {}, event: [{}]", projectName, event);
final Project.NameKey projectNameKey = new Project.NameKey(projectName);
if (projectChecker.isProjectUpToDate(projectNameKey)) {
@@ -97,17 +95,13 @@
indexExecutor.schedule(
() -> {
Context.setForwardedEvent(true);
- try {
- if (!attemptIndex(projectName, event)) {
- log.warn(
- "Attempt {} to index project {} failed, scheduling again after {} msecs",
- retryCount,
- projectName,
- retryInterval);
- rescheduleIndex(projectName, event, retryCount + 1);
- }
- } catch (IOException e) {
- log.warn("Project {} could not be indexed", projectName, e);
+ if (!attemptIndex(projectName, event)) {
+ log.warn(
+ "Attempt {} to index project {} failed, scheduling again after {} msecs",
+ retryCount,
+ projectName,
+ retryInterval);
+ rescheduleIndex(projectName, event, retryCount + 1);
}
},
retryInterval,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java
index 67662f6..de6b836 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.multisite.forwarder;
import com.google.common.util.concurrent.Striped;
-import com.google.gwtorm.server.OrmException;
import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.locks.Lock;
@@ -43,9 +42,9 @@
private final Striped<Lock> idLocks;
- protected abstract void doIndex(T id, Optional<E> indexEvent) throws IOException, OrmException;
+ protected abstract void doIndex(T id, Optional<E> indexEvent);
- protected abstract void doDelete(T id, Optional<E> indexEvent) throws IOException;
+ protected abstract void doDelete(T id, Optional<E> indexEvent);
protected ForwardedIndexingHandler(int lockStripes) {
idLocks = Striped.lock(lockStripes);
@@ -58,9 +57,8 @@
* @param operation The operation to do; index or delete
* @param event The index event details.
* @throws IOException If an error occur while indexing.
- * @throws OrmException If an error occur while retrieving a change related to the item to index
*/
- public void index(T id, Operation operation, Optional<E> event) throws IOException, OrmException {
+ public void index(T id, Operation operation, Optional<E> event) throws IOException {
log.debug("{} {} {}", operation, id, event);
try {
Context.setForwardedEvent(true);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeChecker.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeChecker.java
index 1b0fea8..3646b3a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeChecker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeChecker.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.multisite.index;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gwtorm.server.OrmException;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
import java.io.IOException;
import java.util.Optional;
@@ -27,9 +26,8 @@
* Return the Change nodes read from ReviewDb or NoteDb.
*
* @return notes of the Change
- * @throws OrmException if ReviewDb or NoteDb cannot be opened
*/
- public Optional<ChangeNotes> getChangeNotes() throws OrmException;
+ public Optional<ChangeNotes> getChangeNotes();
/**
* Create a new index event POJO associated with the current Change.
@@ -40,21 +38,17 @@
* index
* @return new IndexEvent
* @throws IOException if the current Change cannot read
- * @throws OrmException if ReviewDb cannot be opened
*/
public Optional<ChangeIndexEvent> newIndexEvent(String projectName, int changeId, boolean deleted)
- throws IOException, OrmException;
+ throws IOException;
/**
* Check if the local Change is aligned with the indexEvent received.
*
* @param indexEvent indexing event
* @return true if the local Change is up-to-date, false otherwise.
- * @throws IOException if an I/O error occurred while reading the local Change
- * @throws OrmException if the local ReviewDb cannot be opened
*/
- public boolean isChangeUpToDate(Optional<ChangeIndexEvent> indexEvent)
- throws IOException, OrmException;
+ public boolean isChangeUpToDate(Optional<ChangeIndexEvent> indexEvent);
/**
* Return the last computed up-to-date Change time-stamp.
@@ -63,7 +57,6 @@
*
* @return the Change timestamp epoch in seconds
* @throws IOException if an I/O error occurred while reading the local Change
- * @throws OrmException if the local ReviewDb cannot be opened
*/
- public Optional<Long> getComputedChangeTs() throws IOException, OrmException;
+ public Optional<Long> getComputedChangeTs() throws IOException;
}
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..92d622d 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
@@ -14,16 +14,15 @@
package com.googlesource.gerrit.plugins.multisite.index;
+import com.google.gerrit.exceptions.StorageException;
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;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
@@ -66,7 +65,7 @@
@Override
public Optional<ChangeIndexEvent> newIndexEvent(String projectName, int changeId, boolean deleted)
- throws IOException, OrmException {
+ throws IOException {
return getComputedChangeTs()
.map(
ts -> {
@@ -78,7 +77,7 @@
}
@Override
- public Optional<ChangeNotes> getChangeNotes() throws OrmException {
+ public Optional<ChangeNotes> getChangeNotes() {
try (ManualRequestContext ctx = oneOffReqCtx.open()) {
this.changeNotes = Optional.ofNullable(changeFinder.findOne(changeId));
return changeNotes;
@@ -86,8 +85,7 @@
}
@Override
- public boolean isChangeUpToDate(Optional<ChangeIndexEvent> indexEvent)
- throws IOException, OrmException {
+ public boolean isChangeUpToDate(Optional<ChangeIndexEvent> indexEvent) {
getComputedChangeTs();
if (!computedChangeTs.isPresent()) {
log.warn("Unable to compute last updated ts for change {}", changeId);
@@ -108,7 +106,7 @@
}
@Override
- public Optional<Long> getComputedChangeTs() throws IOException, OrmException {
+ public Optional<Long> getComputedChangeTs() {
if (!computedChangeTs.isPresent()) {
computedChangeTs = computeLastChangeTs();
}
@@ -117,17 +115,12 @@
@Override
public String toString() {
- try {
- return "change-id="
- + changeId
- + "@"
- + getComputedChangeTs().map(ChangeIndexEvent::format)
- + "/"
- + getBranchTargetSha();
- } catch (IOException | OrmException e) {
- log.error("Unable to render change {}", changeId, e);
- return "change-id=" + changeId;
- }
+ return "change-id="
+ + changeId
+ + "@"
+ + getComputedChangeTs().map(ChangeIndexEvent::format)
+ + "/"
+ + getBranchTargetSha();
}
private String getBranchTargetSha() {
@@ -147,21 +140,19 @@
}
}
- private Optional<Long> computeLastChangeTs() throws OrmException {
- try (ReviewDb db = oneOffReqCtx.open().getReviewDbProvider().get()) {
- return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(db, notes));
- }
+ private Optional<Long> computeLastChangeTs() {
+ 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;
}
- } catch (OrmException e) {
+ } catch (StorageException e) {
log.warn("Unable to access draft comments for change {}", change, e);
}
return changeTs.getTime() / 1000;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/AbstractKafkaSubcriber.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/AbstractKafkaSubcriber.java
index 25200ad..2dba261 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/AbstractKafkaSubcriber.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/consumer/AbstractKafkaSubcriber.java
@@ -15,12 +15,12 @@
package com.googlesource.gerrit.plugins.multisite.kafka.consumer;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gson.Gson;
-import com.google.gwtorm.server.OrmException;
import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.InstanceId;
import com.googlesource.gerrit.plugins.multisite.MessageLogger;
@@ -126,7 +126,7 @@
} catch (IOException e) {
logger.atSevere().withCause(e).log(
"Malformed event '%s': [Exception: %s]", event.getHeader().getEventType());
- } catch (PermissionBackendException | OrmException e) {
+ } catch (PermissionBackendException | StorageException e) {
logger.atSevere().withCause(e).log(
"Cannot handle message %s: [Exception: %s]", event.getHeader().getEventType());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/ForwardedEventRouter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/ForwardedEventRouter.java
index 616b40a..f7de8cc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/ForwardedEventRouter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/ForwardedEventRouter.java
@@ -15,11 +15,10 @@
package com.googlesource.gerrit.plugins.multisite.kafka.router;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.OrmException;
import com.googlesource.gerrit.plugins.multisite.forwarder.CacheNotFoundException;
import java.io.IOException;
public interface ForwardedEventRouter<EventType> {
void route(EventType sourceEvent)
- throws IOException, OrmException, PermissionBackendException, CacheNotFoundException;
+ throws IOException, PermissionBackendException, CacheNotFoundException;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/IndexEventRouter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/IndexEventRouter.java
index e859f40..818777c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/IndexEventRouter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/IndexEventRouter.java
@@ -18,7 +18,6 @@
import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexAccountHandler;
@@ -54,7 +53,7 @@
}
@Override
- public void route(IndexEvent sourceEvent) throws IOException, OrmException {
+ public void route(IndexEvent sourceEvent) throws IOException {
if (sourceEvent instanceof ChangeIndexEvent) {
ChangeIndexEvent changeIndexEvent = (ChangeIndexEvent) sourceEvent;
ForwardedIndexingHandler.Operation operation = changeIndexEvent.deleted ? DELETE : INDEX;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/StreamEventRouter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/StreamEventRouter.java
index ed65662..39d96bc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/StreamEventRouter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/kafka/router/StreamEventRouter.java
@@ -16,7 +16,6 @@
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedEventHandler;
@@ -31,7 +30,7 @@
}
@Override
- public void route(Event sourceEvent) throws OrmException, PermissionBackendException {
+ public void route(Event sourceEvent) throws PermissionBackendException {
streamEventHandler.dispatch(sourceEvent);
}
}
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 8a652a9..e10af32 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
@@ -24,11 +24,9 @@
public class ValidationModule extends FactoryModule {
private final Configuration cfg;
- private final boolean disableGitRepositoryValidation;
- public ValidationModule(Configuration cfg, boolean disableGitRepositoryValidation) {
+ public ValidationModule(Configuration cfg) {
this.cfg = cfg;
- this.disableGitRepositoryValidation = disableGitRepositoryValidation;
}
@Override
@@ -40,10 +38,7 @@
factory(RefUpdateValidator.Factory.class);
factory(BatchRefUpdateValidator.Factory.class);
- if (!disableGitRepositoryValidation) {
- bind(GitRepositoryManager.class).to(MultiSiteGitRepositoryManager.class);
- }
-
+ bind(GitRepositoryManager.class).to(MultiSiteGitRepositoryManager.class);
bind(SharedRefEnforcement.class).to(DefaultSharedRefEnforcement.class).in(Scopes.SINGLETON);
install(new ZkValidationModule(cfg));
}
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index 93154d6..5c5d75a 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/ForwardedEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandlerTest.java
index 3b01c61..5c36ada 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandlerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedEventHandlerTest.java
@@ -19,11 +19,11 @@
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.verify;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.EventDispatcher;
import com.google.gerrit.server.events.ProjectCreatedEvent;
import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -81,7 +81,7 @@
(Answer<Void>)
invocation -> {
assertThat(Context.isForwardedEvent()).isTrue();
- throw new OrmException("someMessage");
+ throw new StorageException("someMessage");
})
.when(dispatcherMock)
.postEvent(event);
@@ -89,8 +89,8 @@
assertThat(Context.isForwardedEvent()).isFalse();
try {
handler.dispatch(event);
- fail("should have throw an OrmException");
- } catch (OrmException e) {
+ fail("should have throw an StorageException");
+ } catch (StorageException e) {
assertThat(e.getMessage()).isEqualTo("someMessage");
}
assertThat(Context.isForwardedEvent()).isFalse();
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..0a910c5 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
@@ -24,17 +24,15 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import com.google.gerrit.exceptions.StorageException;
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;
import com.google.gerrit.server.util.ManualRequestContext;
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;
@@ -60,10 +58,8 @@
private static String TEST_CHANGE_ID = TEST_PROJECT + "~" + TEST_CHANGE_NUMBER;
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 DO_NOT_THROW_STORAGE_EXCEPTION = false;
+ private static final boolean THROW_STORAGE_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,22 +119,13 @@
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));
+ verify(indexerMock, never()).index(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());
- }
-
- @Test
- public void indexerThrowsIOExceptionTryingToIndexChange() throws Exception {
- setupChangeAccessRelatedMocks(
- CHANGE_EXISTS, DO_NOT_THROW_ORM_EXCEPTION, THROW_IO_EXCEPTION, CHANGE_UP_TO_DATE);
- exception.expect(IOException.class);
+ public void indexerThrowsStorageExceptionTryingToIndexChange() throws Exception {
+ setupChangeAccessRelatedMocks(CHANGE_EXISTS, THROW_STORAGE_EXCEPTION, CHANGE_UP_TO_DATE);
+ exception.expect(StorageException.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_STORAGE_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 storageException, boolean changeIsUpToDate)
+ throws StorageException {
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));
+ if (storageException) {
+ doThrow(new StorageException("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..302e5f2 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()));
}
@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;
}
