Merge "Avoid lucene index deletes during offline reindexing" into stable-3.2
diff --git a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
index 271d15c..7bd0c73 100644
--- a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
@@ -45,6 +45,11 @@
}
@Override
+ public void insert(AccountState obj) {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
public void replace(AccountState obj) {
throw new UnsupportedOperationException("AccountIndex is disabled");
}
diff --git a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
index 34f72f5c..7671ad4 100644
--- a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java
@@ -52,6 +52,11 @@
}
@Override
+ public void insert(ChangeData obj) {
+ throw new UnsupportedOperationException("ChangeIndex is disabled");
+ }
+
+ @Override
public void replace(ChangeData obj) {
throw new UnsupportedOperationException("ChangeIndex is disabled");
}
diff --git a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
index ed119ff..2e3dd90 100644
--- a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
+++ b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java
@@ -50,6 +50,11 @@
}
@Override
+ public void insert(ProjectData obj) {
+ throw new UnsupportedOperationException("ProjectIndex is disabled");
+ }
+
+ @Override
public void replace(ProjectData obj) {
throw new UnsupportedOperationException("ProjectIndex is disabled");
}
diff --git a/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java b/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java
index e943519..f7a0669 100644
--- a/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java
+++ b/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java
@@ -45,6 +45,11 @@
}
@Override
+ public void insert(ChangeData obj) {
+ // do nothing
+ }
+
+ @Override
public void replace(ChangeData obj) {
// do nothing
}
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index e56f470..d53490b 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -175,6 +175,12 @@
}
@Override
+ public void insert(V obj) {
+ // TODO: Implement real insert() if it helps performance
+ replace(obj);
+ }
+
+ @Override
public void deleteAll() {
// Delete the index, if it exists.
String endpoint = indexName + client.adapter().indicesExistParams();
diff --git a/java/com/google/gerrit/index/Index.java b/java/com/google/gerrit/index/Index.java
index 44f8b42..e662bc8 100644
--- a/java/com/google/gerrit/index/Index.java
+++ b/java/com/google/gerrit/index/Index.java
@@ -40,6 +40,16 @@
void close();
/**
+ * Insert a document into the index.
+ *
+ * <p>Results may not be immediately visible to searchers, but should be visible within a
+ * reasonable amount of time.
+ *
+ * @param obj document object
+ */
+ void insert(V obj);
+
+ /**
* Update a document in the index.
*
* <p>Semantically equivalent to deleting the document and reinserting it with new field values. A
diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java
index 5c24b9a..360331a 100644
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -89,6 +89,11 @@
}
@Override
+ public void insert(ChangeData obj) {
+ throw new UnsupportedOperationException("don't use ChangeSubIndex directly");
+ }
+
+ @Override
public void replace(ChangeData obj) {
throw new UnsupportedOperationException("don't use ChangeSubIndex directly");
}
diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
index 87b7cce..c4a5240 100644
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -144,6 +144,15 @@
}
@Override
+ public void insert(AccountState as) {
+ try {
+ insert(toDocument(as)).get();
+ } catch (ExecutionException | InterruptedException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ @Override
public void delete(Account.Id key) {
try {
delete(idTerm(getSchema().useLegacyNumericFields(), key)).get();
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index bb97936b..78f4624 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -287,6 +287,23 @@
}
@Override
+ public void insert(ChangeData cd) {
+ Term id = LuceneChangeIndex.idTerm(idTerm, idField, cd);
+ // toDocument is essentially static and doesn't depend on the specific
+ // sub-index, so just pick one.
+ Document doc = openIndex.toDocument(cd);
+ try {
+ if (cd.change().isNew()) {
+ openIndex.insert(doc).get();
+ } else {
+ closedIndex.insert(doc).get();
+ }
+ } catch (ExecutionException | InterruptedException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ @Override
public void delete(Change.Id changeId) {
Term id = LuceneChangeIndex.idTerm(idTerm, idField, changeId);
try {
diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
index c1b44c7..9f8b62e 100644
--- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
@@ -125,6 +125,15 @@
}
@Override
+ public void insert(InternalGroup group) {
+ try {
+ insert(toDocument(group)).get();
+ } catch (ExecutionException | InterruptedException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ @Override
public void delete(AccountGroup.UUID key) {
try {
delete(idTerm(key)).get();
diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
index 7e7aecb..11707be 100644
--- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
@@ -125,6 +125,15 @@
}
@Override
+ public void insert(ProjectData projectState) {
+ try {
+ insert(toDocument(projectState)).get();
+ } catch (ExecutionException | InterruptedException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ @Override
public void delete(Project.NameKey nameKey) {
try {
delete(idTerm(nameKey)).get();
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index 8eaf3b3..e8be0e6 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -38,12 +38,15 @@
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.index.options.AutoFlush;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
import com.google.gerrit.server.util.ReplicaUtil;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Key;
import com.google.inject.Module;
+import com.google.inject.multibindings.OptionalBinder;
import java.io.StringWriter;
import java.io.Writer;
import java.util.ArrayList;
@@ -172,6 +175,16 @@
throw new IllegalStateException("unsupported index.type = " + indexType);
}
modules.add(indexModule);
+ modules.add(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ super.configure();
+ OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class)
+ .setBinding()
+ .toInstance(IsFirstInsertForEntry.YES);
+ }
+ });
modules.add(new BatchProgramModule());
modules.add(
new FactoryModule() {
diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java
index 17665c0..8b04c8d 100644
--- a/java/com/google/gerrit/server/index/IndexModule.java
+++ b/java/com/google/gerrit/server/index/IndexModule.java
@@ -51,6 +51,7 @@
import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.index.group.GroupIndexerImpl;
import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.gerrit.server.index.project.ProjectIndexDefinition;
import com.google.gerrit.server.index.project.ProjectIndexerImpl;
import com.google.inject.Inject;
@@ -59,6 +60,7 @@
import com.google.inject.Provides;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
+import com.google.inject.multibindings.OptionalBinder;
import java.util.Collection;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -143,6 +145,9 @@
}
DynamicSet.setOf(binder(), OnlineUpgradeListener.class);
+ OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class)
+ .setDefault()
+ .toInstance(IsFirstInsertForEntry.NO);
}
@Provides
diff --git a/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java b/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
index 63889b7..ec27db0 100644
--- a/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
+++ b/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -50,15 +51,18 @@
private final ListeningExecutorService executor;
private final Accounts accounts;
private final AccountCache accountCache;
+ private final IsFirstInsertForEntry isFirstInsertForEntry;
@Inject
AllAccountsIndexer(
@IndexExecutor(BATCH) ListeningExecutorService executor,
Accounts accounts,
- AccountCache accountCache) {
+ AccountCache accountCache,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.accounts = accounts;
this.accountCache = accountCache;
+ this.isFirstInsertForEntry = isFirstInsertForEntry;
}
@Override
@@ -92,7 +96,11 @@
try {
Optional<AccountState> a = accountCache.get(id);
if (a.isPresent()) {
- index.replace(a.get());
+ if (isFirstInsertForEntry.equals(isFirstInsertForEntry.YES)) {
+ index.insert(a.get());
+ } else {
+ index.replace(a.get());
+ }
} else {
index.delete(id);
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index e0f6bec..a088af0 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.index.StalenessCheckResult;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
@@ -78,6 +79,7 @@
private final PluginSetContext<ChangeIndexedListener> indexedListeners;
private final StalenessChecker stalenessChecker;
private final boolean autoReindexIfStale;
+ private final IsFirstInsertForEntry isFirstInsertForEntry;
private final Set<IndexTask> queuedIndexTasks =
Collections.newSetFromMap(new ConcurrentHashMap<>());
@@ -94,7 +96,8 @@
StalenessChecker stalenessChecker,
@IndexExecutor(BATCH) ListeningExecutorService batchExecutor,
@Assisted ListeningExecutorService executor,
- @Assisted ChangeIndex index) {
+ @Assisted ChangeIndex index,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
@@ -105,6 +108,7 @@
this.autoReindexIfStale = autoReindexIfStale(cfg);
this.index = index;
this.indexes = null;
+ this.isFirstInsertForEntry = isFirstInsertForEntry;
}
@AssistedInject
@@ -117,7 +121,8 @@
StalenessChecker stalenessChecker,
@IndexExecutor(BATCH) ListeningExecutorService batchExecutor,
@Assisted ListeningExecutorService executor,
- @Assisted ChangeIndexCollection indexes) {
+ @Assisted ChangeIndexCollection indexes,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
@@ -128,6 +133,7 @@
this.autoReindexIfStale = autoReindexIfStale(cfg);
this.index = null;
this.indexes = indexes;
+ this.isFirstInsertForEntry = isFirstInsertForEntry;
}
private static boolean autoReindexIfStale(Config cfg) {
@@ -198,21 +204,25 @@
}
private void indexImpl(ChangeData cd) {
- logger.atFine().log("Replace change %d in index.", cd.getId().get());
+ logger.atFine().log("Reindex change %d in index.", cd.getId().get());
for (Index<?, ChangeData> i : getWriteIndexes()) {
try (TraceTimer traceTimer =
TraceContext.newTimer(
- "Replacing change in index",
+ "Reindexing change in index",
Metadata.builder()
.changeId(cd.getId().get())
.patchSetId(cd.currentPatchSet().number())
.indexVersion(i.getSchema().getVersion())
.build())) {
- i.replace(cd);
+ if (isFirstInsertForEntry.equals(isFirstInsertForEntry.YES)) {
+ i.insert(cd);
+ } else {
+ i.replace(cd);
+ }
} catch (RuntimeException e) {
throw new StorageException(
String.format(
- "Failed to replace change %d in index version %d (current patch set = %d)",
+ "Failed to reindex change %d in index version %d (current patch set = %d)",
cd.getId().get(), i.getSchema().getVersion(), cd.currentPatchSet().number()),
e);
}
diff --git a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
index 51c7730..84d6a9d 100644
--- a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
+++ b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.group.db.GroupsNoteDbConsistencyChecker;
import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -54,15 +55,18 @@
private final ListeningExecutorService executor;
private final GroupCache groupCache;
private final Groups groups;
+ private final IsFirstInsertForEntry isFirstInsertForEntry;
@Inject
AllGroupsIndexer(
@IndexExecutor(BATCH) ListeningExecutorService executor,
GroupCache groupCache,
- Groups groups) {
+ Groups groups,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.groupCache = groupCache;
this.groups = groups;
+ this.isFirstInsertForEntry = isFirstInsertForEntry;
}
@Override
@@ -97,7 +101,11 @@
groupCache.evict(uuid);
Optional<InternalGroup> internalGroup = groupCache.get(uuid);
if (internalGroup.isPresent()) {
- index.replace(internalGroup.get());
+ if (isFirstInsertForEntry.equals(isFirstInsertForEntry.YES)) {
+ index.insert(internalGroup.get());
+ } else {
+ index.replace(internalGroup.get());
+ }
} else {
index.delete(uuid);
diff --git a/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java b/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java
new file mode 100644
index 0000000..f943309
--- /dev/null
+++ b/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2021 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.server.index.options;
+
+/**
+ * This enum should be injected and checked to decide on which operation ({@link
+ * com.google.gerrit.index.Index#replace(Object) replace()} or {@link
+ * com.google.gerrit.index.Index#insert(Object) insert()}) should be performed on a specific {@link
+ * com.google.gerrit.index.Index index}.
+ */
+public enum IsFirstInsertForEntry {
+ YES,
+ NO
+}
diff --git a/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java b/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java
index 0e4b688..86c7e94 100644
--- a/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java
+++ b/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java
@@ -27,6 +27,7 @@
import com.google.gerrit.index.project.ProjectData;
import com.google.gerrit.index.project.ProjectIndex;
import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.index.options.IsFirstInsertForEntry;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -48,12 +49,16 @@
private final ListeningExecutorService executor;
private final ProjectCache projectCache;
+ private final IsFirstInsertForEntry isFirstInsertForEntry;
@Inject
AllProjectsIndexer(
- @IndexExecutor(BATCH) ListeningExecutorService executor, ProjectCache projectCache) {
+ @IndexExecutor(BATCH) ListeningExecutorService executor,
+ ProjectCache projectCache,
+ IsFirstInsertForEntry isFirstInsertForEntry) {
this.executor = executor;
this.projectCache = projectCache;
+ this.isFirstInsertForEntry = isFirstInsertForEntry;
}
@Override
@@ -79,8 +84,13 @@
() -> {
try {
projectCache.evict(name);
- index.replace(
- projectCache.get(name).orElseThrow(illegalState(name)).toProjectData());
+ ProjectData projectData =
+ projectCache.get(name).orElseThrow(illegalState(name)).toProjectData();
+ if (isFirstInsertForEntry.equals(isFirstInsertForEntry.YES)) {
+ index.insert(projectData);
+ } else {
+ index.replace(projectData);
+ }
verboseWriter.println("Reindexed " + desc);
done.incrementAndGet();
} catch (Exception e) {
diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
index a23ccab..fc56a3c 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
@@ -74,6 +74,11 @@
}
@Override
+ public void insert(ChangeData obj) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void replace(ChangeData cd) {
throw new UnsupportedOperationException();
}
