Support generic indexes in LuceneVersionManager
The original change Id6c58a595 wasn't ready and broke master.
The revert was I5d56a163; this reverts the revert.
Due to a mistake in a condition in LuceneVersionManager#start()
method writer index was never initialized and thus reindexing
of changes was broken. All tests were still passing because the
tests are using SingleVersionModule and not MultiVersionModule.
This reverts commit 6247b52f4129213c62b93bd7f3e5ce9a32084978.
Change-Id: I5856d3ca2ae6d9549b00aa575abd074ea1ff0bd9
diff --git a/Documentation/cmd-index-activate.txt b/Documentation/cmd-index-activate.txt
index 0135cfe..37783ef 100644
--- a/Documentation/cmd-index-activate.txt
+++ b/Documentation/cmd-index-activate.txt
@@ -5,7 +5,7 @@
== SYNOPSIS
--
-'ssh' -p @SSH_PORT@ @SSH_HOST@ 'gerrit index activate'
+'ssh' -p @SSH_PORT@ @SSH_HOST@ 'gerrit index activate <index>'
--
== DESCRIPTION
@@ -18,6 +18,9 @@
This command allows to activate the latest index even if there were some
failures.
+The <index> argument controls which secondary index is activated. Currently, the
+only supported value is "changes".
+
== ACCESS
Caller must be a member of the privileged 'Administrators' group.
diff --git a/Documentation/cmd-index-start.txt b/Documentation/cmd-index-start.txt
index 4148b24..cee283e 100644
--- a/Documentation/cmd-index-start.txt
+++ b/Documentation/cmd-index-start.txt
@@ -5,7 +5,7 @@
== SYNOPSIS
--
-'ssh' -p @SSH_PORT@ @SSH_HOST@ 'gerrit index start'
+'ssh' -p @SSH_PORT@ @SSH_HOST@ 'gerrit index start <index>'
--
== DESCRIPTION
@@ -19,6 +19,9 @@
Gerrit. This command will not start the indexer if it is already running or if
the active index is the latest.
+The <index> argument controls which secondary index is started. Currently, the
+only supported value is "changes".
+
== ACCESS
Caller must be a member of the privileged 'Administrators' group.
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneVersionManager.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneVersionManager.java
index 9dd4399..6ccf829 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneVersionManager.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneVersionManager.java
@@ -20,15 +20,14 @@
import com.google.common.collect.Maps;
import com.google.common.primitives.Ints;
import com.google.gerrit.extensions.events.LifecycleListener;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.index.Index;
+import com.google.gerrit.server.index.IndexCollection;
+import com.google.gerrit.server.index.IndexDefinition;
+import com.google.gerrit.server.index.IndexDefinition.IndexFactory;
import com.google.gerrit.server.index.OnlineReindexer;
import com.google.gerrit.server.index.Schema;
-import com.google.gerrit.server.index.change.ChangeIndex;
-import com.google.gerrit.server.index.change.ChangeIndexCollection;
-import com.google.gerrit.server.index.change.ChangeIndexDefinition;
-import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
@@ -46,6 +45,7 @@
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
+import java.util.Map;
import java.util.TreeMap;
@Singleton
@@ -55,13 +55,13 @@
static final String CHANGES_PREFIX = "changes_";
- private static class Version {
- private final Schema<ChangeData> schema;
+ private static class Version<V> {
+ private final Schema<V> schema;
private final int version;
private final boolean exists;
private final boolean ready;
- private Version(Schema<ChangeData> schema, int version, boolean exists,
+ private Version(Schema<V> schema, int version, boolean exists,
boolean ready) {
checkArgument(schema == null || schema.getVersion() == version);
this.schema = schema;
@@ -94,33 +94,32 @@
}
private final SitePaths sitePaths;
- private final LuceneChangeIndex.Factory indexFactory;
- private final ChangeIndexCollection indexes;
- private final ChangeIndexDefinition changeDef;
+ private final Map<String, IndexDefinition<?, ?, ?>> defs;
+ private final Map<String, OnlineReindexer<?, ?, ?>> reindexers;
private final boolean onlineUpgrade;
- private OnlineReindexer<Change.Id, ChangeData, ChangeIndex> reindexer;
+ private final String runReindexMsg;
@Inject
LuceneVersionManager(
@GerritServerConfig Config cfg,
SitePaths sitePaths,
- LuceneChangeIndex.Factory indexFactory,
- ChangeIndexCollection indexes,
- ChangeIndexDefinition changeDef) {
+ Collection<IndexDefinition<?, ?, ?>> defs) {
this.sitePaths = sitePaths;
- this.indexFactory = indexFactory;
- this.indexes = indexes;
- this.changeDef = changeDef;
- this.onlineUpgrade = cfg.getBoolean("index", null, "onlineUpgrade", true);
+ this.defs = Maps.newHashMapWithExpectedSize(defs.size());
+ for (IndexDefinition<?, ?, ?> def : defs) {
+ this.defs.put(def.getName(), def);
+ }
+
+ reindexers = Maps.newHashMapWithExpectedSize(defs.size());
+ onlineUpgrade = cfg.getBoolean("index", null, "onlineUpgrade", true);
+ runReindexMsg =
+ "No index versions ready; run java -jar " +
+ sitePaths.gerrit_war.toAbsolutePath() +
+ " reindex";
}
@Override
public void start() {
- String runReindex =
- "No index versions ready; run java -jar " +
- sitePaths.gerrit_war.toAbsolutePath() +
- " reindex";
-
FileBasedConfig cfg;
try {
cfg = loadGerritIndexConfig(sitePaths);
@@ -129,18 +128,25 @@
}
if (!Files.exists(sitePaths.index_dir)) {
- throw new ProvisionException(runReindex);
+ throw new ProvisionException(runReindexMsg);
} else if (!Files.exists(sitePaths.index_dir)) {
log.warn("Not a directory: %s", sitePaths.index_dir.toAbsolutePath());
- throw new ProvisionException(runReindex);
+ throw new ProvisionException(runReindexMsg);
}
- TreeMap<Integer, Version> versions = scanVersions(cfg);
+ for (IndexDefinition<?, ?, ?> def : defs.values()) {
+ initIndex(def, cfg);
+ }
+ }
+
+ private <K, V, I extends Index<K, V>> void initIndex(
+ IndexDefinition<K, V, I> def, FileBasedConfig cfg) {
+ TreeMap<Integer, Version<V>> versions = scanVersions(def, cfg);
// Search from the most recent ready version.
// Write to the most recent ready version and the most recent version.
- Version search = null;
- List<Version> write = Lists.newArrayListWithCapacity(2);
- for (Version v : versions.descendingMap().values()) {
+ Version<V> search = null;
+ List<Version<V>> write = Lists.newArrayListWithCapacity(2);
+ for (Version<V> v : versions.descendingMap().values()) {
if (v.schema == null) {
continue;
}
@@ -156,27 +162,35 @@
}
}
if (search == null) {
- throw new ProvisionException(runReindex);
+ throw new ProvisionException(runReindexMsg);
}
- markNotReady(cfg, versions.values(), write);
- LuceneChangeIndex searchIndex =
- (LuceneChangeIndex) indexFactory.create(search.schema);
+ IndexFactory<K, V, I> factory = def.getIndexFactory();
+ I searchIndex = factory.create(search.schema);
+ IndexCollection<K, V, I> indexes = def.getIndexCollection();
indexes.setSearchIndex(searchIndex);
- for (Version v : write) {
+ for (Version<V> v : write) {
if (v.schema != null) {
if (v.version != search.version) {
- indexes.addWriteIndex(indexFactory.create(v.schema));
+ indexes.addWriteIndex(factory.create(v.schema));
} else {
indexes.addWriteIndex(searchIndex);
}
}
}
+ // TODO: include index name.
+ markNotReady(cfg, versions.values(), write);
+
int latest = write.get(0).version;
if (onlineUpgrade && latest != search.version) {
- reindexer = new OnlineReindexer<>(changeDef, latest);
- reindexer.start();
+ OnlineReindexer<K, V, I> reindexer = new OnlineReindexer<>(def, latest);
+ synchronized (this) {
+ if (!reindexers.containsKey(def.getName())) {
+ reindexers.put(def.getName(), reindexer);
+ reindexer.start();
+ }
+ }
}
}
@@ -186,10 +200,11 @@
* @return true if started, otherwise false.
* @throws ReindexerAlreadyRunningException
*/
- public synchronized boolean startReindexer()
+ public synchronized boolean startReindexer(String name)
throws ReindexerAlreadyRunningException {
- validateReindexerNotRunning();
- if (!isCurrentIndexVersionLatest()) {
+ OnlineReindexer<?, ?, ?> reindexer = reindexers.get(name);
+ validateReindexerNotRunning(reindexer);
+ if (!isCurrentIndexVersionLatest(name, reindexer)) {
reindexer.start();
return true;
}
@@ -202,49 +217,56 @@
* @return true if index was activate, otherwise false.
* @throws ReindexerAlreadyRunningException
*/
- public synchronized boolean activateLatestIndex()
+ public synchronized boolean activateLatestIndex(String name)
throws ReindexerAlreadyRunningException {
- validateReindexerNotRunning();
- if (!isCurrentIndexVersionLatest()) {
+ OnlineReindexer<?, ?, ?> reindexer = reindexers.get(name);
+ validateReindexerNotRunning(reindexer);
+ if (!isCurrentIndexVersionLatest(name, reindexer)) {
reindexer.activateIndex();
return true;
}
return false;
}
- private boolean isCurrentIndexVersionLatest() {
+ private boolean isCurrentIndexVersionLatest(
+ String name, OnlineReindexer<?, ?, ?> reindexer) {
+ int readVersion = defs.get(name).getIndexCollection().getSearchIndex()
+ .getSchema().getVersion();
return reindexer == null
- || reindexer.getVersion() == indexes.getSearchIndex().getSchema()
- .getVersion();
+ || reindexer.getVersion() == readVersion;
}
- private void validateReindexerNotRunning()
+ private static void validateReindexerNotRunning(
+ OnlineReindexer<?, ?, ?> reindexer)
throws ReindexerAlreadyRunningException {
if (reindexer != null && reindexer.isRunning()) {
throw new ReindexerAlreadyRunningException();
}
}
- private TreeMap<Integer, Version> scanVersions(Config cfg) {
- TreeMap<Integer, Version> versions = Maps.newTreeMap();
- for (Schema<ChangeData> schema : changeDef.getSchemas().values()) {
- Path p = getDir(sitePaths, CHANGES_PREFIX, schema);
+ private <K, V, I extends Index<K, V>> TreeMap<Integer, Version<V>>
+ scanVersions(IndexDefinition<K, V, I> def, Config cfg) {
+ TreeMap<Integer, Version<V>> versions = Maps.newTreeMap();
+ for (Schema<V> schema : def.getSchemas().values()) {
+ // This part is Lucene-specific.
+ Path p = getDir(sitePaths, def.getName(), schema);
boolean isDir = Files.isDirectory(p);
if (Files.exists(p) && !isDir) {
log.warn("Not a directory: %s", p.toAbsolutePath());
}
int v = schema.getVersion();
- versions.put(v, new Version(schema, v, isDir, getReady(cfg, v)));
+ versions.put(v, new Version<>(schema, v, isDir, getReady(cfg, v)));
}
+ String prefix = def.getName() + "_";
try (DirectoryStream<Path> paths =
Files.newDirectoryStream(sitePaths.index_dir)) {
for (Path p : paths) {
String n = p.getFileName().toString();
- if (!n.startsWith(CHANGES_PREFIX)) {
+ if (!n.startsWith(prefix)) {
continue;
}
- String versionStr = n.substring(CHANGES_PREFIX.length());
+ String versionStr = n.substring(prefix.length());
Integer v = Ints.tryParse(versionStr);
if (v == null || versionStr.length() != 4) {
log.warn("Unrecognized version in index directory: {}",
@@ -252,7 +274,7 @@
continue;
}
if (!versions.containsKey(v)) {
- versions.put(v, new Version(null, v, true, getReady(cfg, v)));
+ versions.put(v, new Version<V>(null, v, true, getReady(cfg, v)));
}
}
} catch (IOException e) {
@@ -261,10 +283,10 @@
return versions;
}
- private void markNotReady(FileBasedConfig cfg, Iterable<Version> versions,
- Collection<Version> inUse) {
+ private <V> void markNotReady(FileBasedConfig cfg, Iterable<Version<V>> versions,
+ Collection<Version<V>> inUse) {
boolean dirty = false;
- for (Version v : versions) {
+ for (Version<V> v : versions) {
if (!inUse.contains(v) && v.exists) {
setReady(cfg, v.version, false);
dirty = true;
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
index dc67ac3..c508b1d 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
@@ -24,19 +24,25 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
+import org.kohsuke.args4j.Argument;
+
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(name = "activate",
description = "Activate the latest index version available",
runsAt = MASTER)
public class IndexActivateCommand extends SshCommand {
+ @Argument(index = 0, required = true, metaVar = "INDEX",
+ usage = "index name to activate")
+ private String name;
+
@Inject
private LuceneVersionManager luceneVersionManager;
@Override
protected void run() throws UnloggedFailure {
try {
- if (luceneVersionManager.activateLatestIndex()) {
+ if (luceneVersionManager.activateLatestIndex(name)) {
stdout.println("Activated latest index version");
} else {
stdout.println("Not activating index, already using latest version");
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
index 1b3b819..c2c565f 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
@@ -24,18 +24,24 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
+import org.kohsuke.args4j.Argument;
+
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(name = "start", description = "Start the online reindexer",
runsAt = MASTER)
public class IndexStartCommand extends SshCommand {
+ @Argument(index = 0, required = true, metaVar = "INDEX",
+ usage = "index name to start")
+ private String name;
+
@Inject
private LuceneVersionManager luceneVersionManager;
@Override
protected void run() throws UnloggedFailure {
try {
- if (luceneVersionManager.startReindexer()) {
+ if (luceneVersionManager.startReindexer(name)) {
stdout.println("Reindexer started");
} else {
stdout.println("Nothing to reindex, index is already the latest version");
