Merge "Allow to reindex without notifying listeners"
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 4cd5bb2..37121350 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1728,7 +1728,7 @@
This endpoint allows to trigger background reindexing of an index version. It is
also supported to specify whether to reuse existing up-to-date (non-stale) index
-documents.
+documents and whether to notifyListeners or not.
.Request
----
@@ -1736,7 +1736,8 @@
Content-Type: application/json; charset=UTF-8
{
- "reuse": "true"
+ "reuse": "true",
+ "notifyListeners": "false"
}
----
diff --git a/java/com/google/gerrit/index/SiteIndexer.java b/java/com/google/gerrit/index/SiteIndexer.java
index 32b4b21..bfb4407 100644
--- a/java/com/google/gerrit/index/SiteIndexer.java
+++ b/java/com/google/gerrit/index/SiteIndexer.java
@@ -76,6 +76,16 @@
/** Indexes all entities for the provided index. */
public abstract Result indexAll(I index);
+ /**
+ * Indexes all entities for the provided index.
+ *
+ * <p>NOTE: This method does not implement the 'notifyListeners' logic which is effectively
+ * ignored and all listeners are always notified.
+ */
+ public Result indexAll(I index, @SuppressWarnings("unused") boolean notifyListeners) {
+ return indexAll(index);
+ }
+
protected final void addErrorListener(
ListenableFuture<?> future, String desc, ProgressMonitor progress, AtomicBoolean ok) {
future.addListener(
diff --git a/java/com/google/gerrit/server/index/IndexVersionReindexer.java b/java/com/google/gerrit/server/index/IndexVersionReindexer.java
index 5d136e8..84be97e 100644
--- a/java/com/google/gerrit/server/index/IndexVersionReindexer.java
+++ b/java/com/google/gerrit/server/index/IndexVersionReindexer.java
@@ -35,14 +35,14 @@
}
public <K, V, I extends Index<K, V>> Future<SiteIndexer.Result> reindex(
- IndexDefinition<K, V, I> def, int version, boolean reuse) {
+ IndexDefinition<K, V, I> def, int version, boolean reuse, boolean notifyListeners) {
I index = def.getIndexCollection().getWriteIndex(version);
SiteIndexer<K, V, I> siteIndexer = def.getSiteIndexer(reuse);
return executor.submit(
() -> {
String name = def.getName();
logger.atInfo().log("Starting reindex of %s version %d", name, version);
- SiteIndexer.Result result = siteIndexer.indexAll(index);
+ SiteIndexer.Result result = siteIndexer.indexAll(index, notifyListeners);
if (result.success()) {
logger.atInfo().log("Reindex %s version %s complete", name, version);
} else {
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 925f68d..19a0223 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -177,6 +177,11 @@
@Override
public Result indexAll(ChangeIndex index) {
+ return indexAll(index, true);
+ }
+
+ @Override
+ public Result indexAll(ChangeIndex index, boolean notifyListeners) {
// The simplest approach to distribute indexing would be to let each thread grab a project
// and index it fully. But if a site has one big project and 100s of small projects, then
// in the beginning all CPUs would be busy reindexing projects. But soon enough all small
@@ -199,7 +204,7 @@
failedTask = mpm.beginSubTask("failed", MultiProgressMonitor.UNKNOWN);
List<ListenableFuture<?>> futures;
try {
- futures = new SliceScheduler(index, ok).schedule();
+ futures = new SliceScheduler(index, ok, notifyListeners).schedule();
} catch (ProjectsCollectionFailure e) {
logger.atSevere().log("%s", e.getMessage());
return Result.create(sw, false, 0, 0);
@@ -359,6 +364,7 @@
private class SliceScheduler {
final ChangeIndex index;
final AtomicBoolean ok;
+ final boolean notifyListeners;
final AtomicInteger changeCount = new AtomicInteger(0);
final AtomicInteger projectsFailed = new AtomicInteger(0);
final List<ListenableFuture<?>> sliceIndexerFutures = new ArrayList<>();
@@ -366,9 +372,10 @@
VolatileTask projTask = mpm.beginVolatileSubTask("project-slices");
Task slicingProjects;
- public SliceScheduler(ChangeIndex index, AtomicBoolean ok) {
+ public SliceScheduler(ChangeIndex index, AtomicBoolean ok, boolean notifyListeners) {
this.index = index;
this.ok = ok;
+ this.notifyListeners = notifyListeners;
}
private List<ListenableFuture<?>> schedule() throws ProjectsCollectionFailure {
@@ -376,7 +383,7 @@
int projectCount = projects.size();
slicingProjects = mpm.beginSubTask("Slicing projects", projectCount);
for (Project.NameKey name : projects) {
- sliceCreationFutures.add(executor.submit(new ProjectSliceCreator(name)));
+ sliceCreationFutures.add(executor.submit(new ProjectSliceCreator(name, notifyListeners)));
}
try {
@@ -407,9 +414,11 @@
private class ProjectSliceCreator implements Callable<Void> {
final Project.NameKey name;
+ final boolean notifyListeners;
- public ProjectSliceCreator(Project.NameKey name) {
+ public ProjectSliceCreator(Project.NameKey name, boolean notifyListeners) {
this.name = name;
+ this.notifyListeners = notifyListeners;
}
@Override
@@ -434,9 +443,10 @@
ChangeIndexer indexer;
if (reuseExistingDocuments) {
indexer =
- indexerFactory.create(executor, index, stalenessCheckerFactory.create(index));
+ indexerFactory.create(
+ executor, index, stalenessCheckerFactory.create(index), notifyListeners);
} else {
- indexer = indexerFactory.create(executor, index);
+ indexer = indexerFactory.create(executor, index, notifyListeners);
}
ListenableFuture<?> future =
executor.submit(reindexProjectSlice(indexer, projectSlice, doneTask, failedTask));
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 4ec390d..1e7d0f2 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -71,7 +71,13 @@
ChangeIndexer create(ListeningExecutorService executor, ChangeIndex index);
ChangeIndexer create(
- ListeningExecutorService executor, ChangeIndex index, StalenessChecker stalenessChecker);
+ ListeningExecutorService executor, ChangeIndex index, boolean notifyListeners);
+
+ ChangeIndexer create(
+ ListeningExecutorService executor,
+ ChangeIndex index,
+ StalenessChecker stalenessChecker,
+ boolean notifyListeners);
ChangeIndexer create(ListeningExecutorService executor, ChangeIndexCollection indexes);
}
@@ -87,6 +93,7 @@
private final StalenessChecker stalenessChecker;
private final boolean autoReindexIfStale;
private final IsFirstInsertForEntry isFirstInsertForEntry;
+ private final boolean notifyListeners;
private final Map<Change.Id, IndexTask> queuedIndexTasks = new ConcurrentHashMap<>();
private final Set<ReindexIfStaleTask> queuedReindexIfStaleTasks =
@@ -104,6 +111,33 @@
@Assisted ListeningExecutorService executor,
@Assisted ChangeIndex index,
IsFirstInsertForEntry isFirstInsertForEntry) {
+ this(
+ cfg,
+ changeDataFactory,
+ notesFactory,
+ context,
+ indexedListeners,
+ stalenessChecker,
+ batchExecutor,
+ executor,
+ index,
+ true,
+ isFirstInsertForEntry);
+ }
+
+ @AssistedInject
+ ChangeIndexer(
+ @GerritServerConfig Config cfg,
+ ChangeData.Factory changeDataFactory,
+ ChangeNotes.Factory notesFactory,
+ ThreadLocalRequestContext context,
+ PluginSetContext<ChangeIndexedListener> indexedListeners,
+ StalenessChecker stalenessChecker,
+ @IndexExecutor(BATCH) ListeningExecutorService batchExecutor,
+ @Assisted ListeningExecutorService executor,
+ @Assisted ChangeIndex index,
+ @Assisted boolean notifyListeners,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
@@ -115,6 +149,7 @@
this.index = index;
this.indexes = null;
this.isFirstInsertForEntry = isFirstInsertForEntry;
+ this.notifyListeners = notifyListeners;
}
@AssistedInject
@@ -128,7 +163,8 @@
IsFirstInsertForEntry isFirstInsertForEntry,
@Assisted ListeningExecutorService executor,
@Assisted ChangeIndex index,
- @Assisted StalenessChecker stalenessChecker) {
+ @Assisted StalenessChecker stalenessChecker,
+ @Assisted boolean notifyListeners) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
@@ -140,6 +176,7 @@
this.index = index;
this.indexes = null;
this.stalenessChecker = stalenessChecker;
+ this.notifyListeners = notifyListeners;
}
@AssistedInject
@@ -165,6 +202,7 @@
this.index = null;
this.indexes = indexes;
this.isFirstInsertForEntry = isFirstInsertForEntry;
+ this.notifyListeners = true;
}
private static boolean autoReindexIfStale(Config cfg) {
@@ -290,19 +328,27 @@
}
private void fireChangeScheduledForIndexingEvent(String projectName, int id) {
- indexedListeners.runEach(l -> l.onChangeScheduledForIndexing(projectName, id));
+ if (notifyListeners) {
+ indexedListeners.runEach(l -> l.onChangeScheduledForIndexing(projectName, id));
+ }
}
private void fireChangeIndexedEvent(String projectName, int id) {
- indexedListeners.runEach(l -> l.onChangeIndexed(projectName, id));
+ if (notifyListeners) {
+ indexedListeners.runEach(l -> l.onChangeIndexed(projectName, id));
+ }
}
private void fireChangeScheduledForDeletionFromIndexEvent(int id) {
- indexedListeners.runEach(l -> l.onChangeScheduledForDeletionFromIndex(id));
+ if (notifyListeners) {
+ indexedListeners.runEach(l -> l.onChangeScheduledForDeletionFromIndex(id));
+ }
}
private void fireChangeDeletedFromIndexEvent(int id) {
- indexedListeners.runEach(l -> l.onChangeDeleted(id));
+ if (notifyListeners) {
+ indexedListeners.runEach(l -> l.onChangeDeleted(id));
+ }
}
/**
diff --git a/java/com/google/gerrit/server/restapi/config/ReindexIndexVersion.java b/java/com/google/gerrit/server/restapi/config/ReindexIndexVersion.java
index d923155..21cd1c1 100644
--- a/java/com/google/gerrit/server/restapi/config/ReindexIndexVersion.java
+++ b/java/com/google/gerrit/server/restapi/config/ReindexIndexVersion.java
@@ -25,7 +25,8 @@
public class ReindexIndexVersion implements RestModifyView<IndexVersionResource, Input> {
public static class Input {
- boolean reuse;
+ public boolean reuse;
+ public boolean notifyListeners;
}
private final IndexVersionReindexer indexVersionReindexer;
@@ -41,7 +42,7 @@
IndexDefinition<?, ?, ?> def = rsrc.getIndexDefinition();
int version = rsrc.getIndex().getSchema().getVersion();
@SuppressWarnings("unused")
- var unused = indexVersionReindexer.reindex(def, version, input.reuse);
+ var unused = indexVersionReindexer.reindex(def, version, input.reuse, input.notifyListeners);
return Response.accepted(
String.format("Index %s version %d submitted for reindexing", def.getName(), version));
}
diff --git a/javatests/com/google/gerrit/acceptance/server/index/change/BUILD b/javatests/com/google/gerrit/acceptance/server/index/BUILD
similarity index 81%
rename from javatests/com/google/gerrit/acceptance/server/index/change/BUILD
rename to javatests/com/google/gerrit/acceptance/server/index/BUILD
index 3be5249..1d4ef02 100644
--- a/javatests/com/google/gerrit/acceptance/server/index/change/BUILD
+++ b/javatests/com/google/gerrit/acceptance/server/index/BUILD
@@ -1,7 +1,7 @@
load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
acceptance_tests(
- srcs = glob(["*IT.java"]),
+ srcs = glob(["**/*IT.java"]),
group = "server_index",
labels = ["server"],
)
diff --git a/javatests/com/google/gerrit/acceptance/server/index/ReindexIndexVersionIT.java b/javatests/com/google/gerrit/acceptance/server/index/ReindexIndexVersionIT.java
new file mode 100644
index 0000000..d433ca7
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/index/ReindexIndexVersionIT.java
@@ -0,0 +1,87 @@
+// Copyright (C) 2024 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.google.gerrit.acceptance.server.index;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.index.Index;
+import com.google.gerrit.index.IndexDefinition;
+import com.google.gerrit.server.config.IndexVersionResource;
+import com.google.gerrit.server.restapi.config.ReindexIndexVersion;
+import com.google.inject.Inject;
+import java.util.Collection;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ReindexIndexVersionIT extends AbstractDaemonTest {
+
+ @Inject private ReindexIndexVersion reindexIndexVersion;
+ @Inject private Collection<IndexDefinition<?, ?, ?>> indexDefs;
+ @Inject private ExtensionRegistry extensionRegistry;
+
+ private IndexDefinition<?, ?, ?> def;
+ private Index<?, ?> changeIndex;
+ private Change.Id C1;
+ private Change.Id C2;
+
+ private ChangeIndexedListener changeIndexedListener;
+ private ReindexIndexVersion.Input input = new ReindexIndexVersion.Input();
+
+ @Before
+ public void setUp() throws Exception {
+ def = indexDefs.stream().filter(i -> i.getName().equals("changes")).findFirst().get();
+ changeIndex = def.getIndexCollection().getSearchIndex();
+ C1 = createChange().getChange().getId();
+ C2 = createChange().getChange().getId();
+ changeIndexedListener = mock(ChangeIndexedListener.class);
+ input = new ReindexIndexVersion.Input();
+ }
+
+ @Test
+ public void reindexWithListenerNotification() throws Exception {
+ input.notifyListeners = true;
+ reindex();
+ verify(changeIndexedListener, times(1)).onChangeIndexed(project.get(), C1.get());
+ verify(changeIndexedListener, times(1)).onChangeIndexed(project.get(), C2.get());
+ }
+
+ @Test
+ public void reindexWithoutListenerNotification() throws Exception {
+ input.notifyListeners = false;
+ reindex();
+ verifyNoInteractions(changeIndexedListener);
+ }
+
+ private void reindex() throws ResourceNotFoundException {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedListener)) {
+ Response<?> rsp =
+ reindexIndexVersion.apply(new IndexVersionResource(def, changeIndex), input);
+ assertThat(rsp.statusCode()).isEqualTo(HttpServletResponse.SC_ACCEPTED);
+ }
+ }
+}
