opendev/gerrit
Code Issues Proposed changes

Fix init with elasticsearch and companion reindex

Browse Source 
AbstractElasticIndex: use the real (raw) index name in markReady
(setReady, towards index config), rather than the one formatted with
prefix and version suffix. Indeed, setReady assumes no prefix in index
config and already appends the version -independently of the raw index
name.

Concatenating the prefix with index names (alongside versions) is
already assumed internally within gerrit when interacting with its
Elasticsearch indices. Such a dynamic, on-the-fly behaviour is based on:

1. the prefix -for this site instance, configured in gerrit.config, and
2. index names with version suffixes, configured in gerrit_index.config.

VersionManager classes: align the Elasticsearch implementation with the
one for Lucene. Namely, make the 'dirtiness' of Elastic versions
consistent with Lucene's. Meaning, align the existence nature of an
index between the two implementations: have an Elasticsearch index exist
only if part of the Elasticsearch server indices, even if listed as a
known schema version. The lucene implementation does this through the
existence of a corresponding index directory, within the site.

Before the aforementioned co-fixes, reindexing a site just initialized
with elasticsearch failed, by means of extra indices generated within
gerrit_index.config:

1. some with previous schema versions and ready = false set for them,
2. and some others with wrongfully prefixed index names, which also had
their version suffix doubled; e.g., [index "gerrit_changes_0039_0039"],
which reads [index "changes_0039"] and appears only once after this fix
(also expectedly set as ready = true).

The above sample exposes the fact that such a 'gerrit_' prefix does not
match the default 'gerrit' (no trailing underscore). A follow-up change
could consider changing that default to a more readable gerrit_ prefix.

The current Elasticsearch implementation still requires a reindex after
a site init, as per the ui console message already shown -telling the
init user about such a necessity. So, a reindex is required in order to
generate gerrit_index.config after an init, and this very change is to
properly generate that file, for the site to work with Elasticsearch.

Bug: Issue 8553
Change-Id: I33d0f02fcca1bacb9d66c57f3d0bc3fa3b233197
This commit is contained in:
Marco Miller
2018-03-27 17:44:18 -04:00
parent 029e576a87
commit db5b37e106
7 changed files with 21 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
View File

@@ -62,6 +62,7 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
private final Schema<V> schema;
private final FillArgs fillArgs;
private final SitePaths sitePaths;
private final String indexNameRaw;
protected final String indexName;
protected final JestHttpClient client;
@@ -82,10 +83,11 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
this.queryBuilder = new ElasticQueryBuilder();
this.indexName =
String.format(
"%s%s%04d",
"%s%s_%04d",
Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix")),
indexName,
schema.getVersion());
this.indexNameRaw = indexName;
this.client = clientBuilder.build();
}
@@ -101,7 +103,7 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
@Override
public void markReady(boolean ready) throws IOException {
IndexUtils.setReady(sitePaths, indexName, schema.getVersion(), ready);
IndexUtils.setReady(sitePaths, indexNameRaw, schema.getVersion(), ready);
}
@Override

2
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
View File

@@ -84,7 +84,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex<Account.Id, Accoun
JestClientBuilder clientBuilder,
@Assisted Schema<AccountState> schema) {
// No parts of FillArgs are currently required, just use null.
super(cfg, null, sitePaths, schema, clientBuilder, ACCOUNTS_PREFIX);
super(cfg, null, sitePaths, schema, clientBuilder, ACCOUNTS);
this.accountCache = accountCache;
this.mapping = new AccountMapping(schema);
}

2
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -109,7 +109,7 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
SitePaths sitePaths,
JestClientBuilder clientBuilder,
@Assisted Schema<ChangeData> schema) {
super(cfg, fillArgs, sitePaths, schema, clientBuilder, CHANGES_PREFIX);
super(cfg, fillArgs, sitePaths, schema, clientBuilder, CHANGES);
this.db = db;
this.changeDataFactory = changeDataFactory;
mapping = new ChangeMapping(schema);

2
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
View File

@@ -81,7 +81,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex<AccountGroup.UUID, A
JestClientBuilder clientBuilder,
@Assisted Schema<AccountGroup> schema) {
// No parts of FillArgs are currently required, just use null.
super(cfg, null, sitePaths, schema, clientBuilder, GROUPS_PREFIX);
super(cfg, null, sitePaths, schema, clientBuilder, GROUPS);
this.groupCache = groupCache;
this.mapping = new GroupMapping(schema);
}

20
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticVersionManager.java
View File

@@ -51,20 +51,10 @@ public class ElasticVersionManager extends AbstractVersionManager implements Lif
prefix = MoreObjects.firstNonNull(cfg.getString("index", null, "prefix"), "gerrit");
}
@Override
protected <V> boolean isDirty(Collection<Version<V>> inUse, Version<V> v) {
return !inUse.contains(v);
}
@Override
protected <K, V, I extends Index<K, V>> TreeMap<Integer, Version<V>> scanVersions(
IndexDefinition<K, V, I> def, GerritIndexStatus cfg) {
TreeMap<Integer, Version<V>> versions = new TreeMap<>();
for (Schema<V> schema : def.getSchemas().values()) {
int v = schema.getVersion();
versions.put(v, new Version<>(schema, v, cfg.getReady(def.getName(), v)));
}
try {
for (String version : versionDiscovery.discover(prefix, def.getName())) {
Integer v = Ints.tryParse(version);
@@ -72,13 +62,17 @@ public class ElasticVersionManager extends AbstractVersionManager implements Lif
log.warn("Unrecognized version in index {}: {}", def.getName(), version);
continue;
}
if (!versions.containsKey(v)) {
versions.put(v, new Version<V>(null, v, cfg.getReady(def.getName(), v)));
}
versions.put(v, new Version<V>(null, v, true, cfg.getReady(def.getName(), v)));
}
} catch (IOException e) {
log.error("Error scanning index: " + def.getName(), e);
}
for (Schema<V> schema : def.getSchemas().values()) {
int v = schema.getVersion();
boolean exists = versions.containsKey(v);
versions.put(v, new Version<>(schema, v, exists, cfg.getReady(def.getName(), v)));
}
return versions;
}
}

16
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneVersionManager.java
View File

@@ -39,15 +39,6 @@ import org.slf4j.LoggerFactory;
public class LuceneVersionManager extends AbstractVersionManager implements LifecycleListener {
private static final Logger log = LoggerFactory.getLogger(LuceneVersionManager.class);
private static class Version<V> extends AbstractVersionManager.Version<V> {
private final boolean exists;
private Version(Schema<V> schema, int version, boolean exists, boolean ready) {
super(schema, version, ready);
this.exists = exists;
}
}
static Path getDir(SitePaths sitePaths, String prefix, Schema<?> schema) {
return sitePaths.index_dir.resolve(String.format("%s%04d", prefix, schema.getVersion()));
}
@@ -60,13 +51,6 @@ public class LuceneVersionManager extends AbstractVersionManager implements Life
super(cfg, sitePaths, defs);
}
@Override
protected <V> boolean isDirty(
Collection<com.google.gerrit.server.index.AbstractVersionManager.Version<V>> inUse,
com.google.gerrit.server.index.AbstractVersionManager.Version<V> v) {
return !inUse.contains(v) && ((Version<V>) v).exists;
}
@Override
protected <K, V, I extends Index<K, V>>
TreeMap<Integer, AbstractVersionManager.Version<V>> scanVersions(

10
gerrit-server/src/main/java/com/google/gerrit/server/index/AbstractVersionManager.java
View File

@@ -35,12 +35,14 @@ public abstract class AbstractVersionManager implements LifecycleListener {
public static class Version<V> {
public final Schema<V> schema;
public final int version;
public final boolean exists;
public final boolean ready;
public Version(Schema<V> schema, int version, boolean ready) {
public Version(Schema<V> schema, int version, boolean exists, boolean ready) {
checkArgument(schema == null || schema.getVersion() == version);
this.schema = schema;
this.version = version;
this.exists = exists;
this.ready = ready;
}
}
@@ -179,11 +181,13 @@ public abstract class AbstractVersionManager implements LifecycleListener {
}
}
protected abstract <V> boolean isDirty(Collection<Version<V>> inUse, Version<V> v);
protected abstract <K, V, I extends Index<K, V>> TreeMap<Integer, Version<V>> scanVersions(
IndexDefinition<K, V, I> def, GerritIndexStatus cfg);
private <V> boolean isDirty(Collection<Version<V>> inUse, Version<V> v) {
return !inUse.contains(v) && v.exists;
}
private boolean isLatestIndexVersion(String name, OnlineReindexer<?, ?, ?> reindexer) {
int readVersion = defs.get(name).getIndexCollection().getSearchIndex().getSchema().getVersion();
return reindexer == null || reindexer.getVersion() == readVersion;