opendev/gerrit
Code Issues Proposed changes

Migrate change index to use dimensional numeric types

Browse Source 
Lucene 6.x deprecated IntField and replaced it with IntPoint that is
using different backend storage: [1]. Instead of continuing to
represent numeric data using a structure specifically designed and
tuned for text, the Bkd implementation introduced the first flexible
tree structure designed specifically for indexing discrete numeric
points: [1]. While the new data types are mostly the drop in
replacement for old IntField and LongField types, new type cannot be
used for document id types.

The previous migration from Lucene 5 to Lucene 6 switched to using
deprecated LegacyIntField type. In the next Lucene release 7, this
class and friends were extracted from Lucene distribution and moved
for one release to Apache Solr library. So theoretically we could
still use Apache Solr dependency by adding this dependency to Gerrit
and continue to use the old/deprecated/removed data types for one
major Lucene release.

We prefer forward migration strategy and switch to using string field
type as document id for account, change and groups indexes. Only change
index is handled in this commit. Other indexes are handled in follow-up
changes.

To support online migration, legacy numeric field types are still used
in the old index schema version, but new dimensional point field types
are used in new schema version. Old integer document id field type is
replaced with string type id in new change index schema. Therefore, in
different code paths it must be decided whether the legacy number field
types or the new dimensional point field types should be used depending
on the currently used index schema version. To support this logic, new
attribute is added to the index schema class: useLegacyNumericFields.

While this approach temporarily complicates the code, it can be removed
when a next gerrit version is released. Until then the deprecated type
classes are still used.

Non id fields are replaced with new IntPoint and LongPoint fields so
that we do not use any deprecated and removed features in Lucene and
could easily upgrade to the next major Lucene release without relying
on third party dependency (Apache Solr).

One side effect of this change is that ChangeQueryBuilder in the
AbandonUtil must be used with Guice provider. The reason for that is
because index collection must be accessed to retrieve schema instance,
to detect the useLegacyNumericFields attribute. Given that AbandonUtil
is bound in singleton scope, index collection is only provided when
multiversion index module is started. When the support for legacy
numeric field is removed in later gerrit releases this change can be
reverted.

[1] https://users.cs.duke.edu/~pankaj/publications/papers/bkd-sstd.pdf

Bug: Issue 11643
Change-Id: Icbc80d8a775a6ffea97e99717b24d3e8cacaee14
This commit is contained in:
David Ostrovsky
2019-01-24 21:11:54 +01:00
parent 0fb1fd1936
commit f957999476
21 changed files with 276 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -40,6 +40,7 @@ import com.google.gerrit.entities.converter.ChangeProtoConverter;
import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.DataSource;
@@ -91,6 +92,7 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
private final ChangeMapping mapping;
private final ChangeData.Factory changeDataFactory;
private final Schema<ChangeData> schema;
private final FieldDef<ChangeData, ?> idField;
@Inject
ElasticChangeIndex(
@@ -102,7 +104,9 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
super(cfg, sitePaths, schema, clientBuilder, CHANGES);
this.changeDataFactory = changeDataFactory;
this.schema = schema;
mapping = new ChangeMapping(schema, client.adapter());
this.mapping = new ChangeMapping(schema, client.adapter());
this.idField =
this.schema.useLegacyNumericFields() ? ChangeField.LEGACY_ID : ChangeField.LEGACY_ID_STR;
}
@Override
@@ -157,7 +161,8 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
}
}
QueryOptions filteredOpts = opts.filterFields(IndexUtils::changeFields);
QueryOptions filteredOpts =
opts.filterFields(o -> IndexUtils.changeFields(o, schema.useLegacyNumericFields()));
return new ElasticQuerySource(p, filteredOpts, getURI(indexes), getSortArray());
}
@@ -167,7 +172,7 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
JsonArray sortArray = new JsonArray();
addNamedElement(ChangeField.UPDATED.getName(), properties, sortArray);
addNamedElement(ChangeField.LEGACY_ID.getName(), properties, sortArray);
addNamedElement(idField.getName(), properties, sortArray);
return sortArray;
}
@@ -206,7 +211,7 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
JsonElement c = source.get(ChangeField.CHANGE.getName());
if (c == null) {
int id = source.get(ChangeField.LEGACY_ID.getName()).getAsInt();
int id = source.get(idField.getName()).getAsInt();
// IndexUtils#changeFields ensures either CHANGE or PROJECT is always present.
String projectName = requireNonNull(source.get(ChangeField.PROJECT.getName()).getAsString());
return changeDataFactory.create(Project.nameKey(projectName), Change.id(id));

20
java/com/google/gerrit/index/Schema.java
View File

@@ -34,6 +34,7 @@ public class Schema<T> {
public static class Builder<T> {
private final List<FieldDef<T, ?>> fields = new ArrayList<>();
private boolean useLegacyNumericFields;
public Builder<T> add(Schema<T> schema) {
this.fields.addAll(schema.getFields().values());
@@ -52,8 +53,13 @@ public class Schema<T> {
return this;
}
public Builder<T> legacyNumericFields(boolean useLegacyNumericFields) {
this.useLegacyNumericFields = useLegacyNumericFields;
return this;
}
public Schema<T> build() {
return new Schema<>(ImmutableList.copyOf(fields));
return new Schema<>(useLegacyNumericFields, ImmutableList.copyOf(fields));
}
}
@@ -82,14 +88,15 @@ public class Schema<T> {
private final ImmutableMap<String, FieldDef<T, ?>> fields;
private final ImmutableMap<String, FieldDef<T, ?>> storedFields;
private final boolean useLegacyNumericFields;
private int version;
public Schema(Iterable<FieldDef<T, ?>> fields) {
this(0, fields);
public Schema(boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
this(0, useLegacyNumericFields, fields);
}
public Schema(int version, Iterable<FieldDef<T, ?>> fields) {
public Schema(int version, boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
this.version = version;
ImmutableMap.Builder<String, FieldDef<T, ?>> b = ImmutableMap.builder();
ImmutableMap.Builder<String, FieldDef<T, ?>> sb = ImmutableMap.builder();
@@ -101,12 +108,17 @@ public class Schema<T> {
}
this.fields = b.build();
this.storedFields = sb.build();
this.useLegacyNumericFields = useLegacyNumericFields;
}
public final int getVersion() {
return version;
}
public final boolean useLegacyNumericFields() {
return useLegacyNumericFields;
}
/**
* Get all fields in this schema.
*

5
java/com/google/gerrit/index/SchemaUtil.java
View File

@@ -67,12 +67,13 @@ public class SchemaUtil {
}
public static <V> Schema<V> schema(Collection<FieldDef<V, ?>> fields) {
return new Schema<>(ImmutableList.copyOf(fields));
return new Schema<>(true, ImmutableList.copyOf(fields));
}
@SafeVarargs
public static <V> Schema<V> schema(Schema<V> schema, FieldDef<V, ?>... moreFields) {
return new Schema<>(
true,
new ImmutableList.Builder<FieldDef<V, ?>>()
.addAll(schema.getFields().values())
.addAll(ImmutableList.copyOf(moreFields))
@@ -81,7 +82,7 @@ public class SchemaUtil {
@SafeVarargs
public static <V> Schema<V> schema(FieldDef<V, ?>... fields) {
return schema(ImmutableList.copyOf(fields));
return new Schema<>(true, ImmutableList.copyOf(fields));
}
public static Set<String> getPersonParts(PersonIdent person) {

27
java/com/google/gerrit/lucene/AbstractLuceneIndex.java
View File

@@ -63,8 +63,10 @@ import java.util.function.Function;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.Field.Store;
import org.apache.lucene.document.IntPoint;
import org.apache.lucene.document.LegacyIntField;
import org.apache.lucene.document.LegacyLongField;
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.document.StringField;
import org.apache.lucene.document.TextField;
@@ -334,15 +336,23 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
if (type == FieldType.INTEGER || type == FieldType.INTEGER_RANGE) {
for (Object value : values.getValues()) {
doc.add(new LegacyIntField(name, (Integer) value, store));
Integer intValue = (Integer) value;
if (schema.useLegacyNumericFields()) {
doc.add(new LegacyIntField(name, intValue, store));
} else {
doc.add(new IntPoint(name, intValue));
if (store == Store.YES) {
doc.add(new StoredField(name, intValue));
}
}
}
} else if (type == FieldType.LONG) {
for (Object value : values.getValues()) {
doc.add(new LegacyLongField(name, (Long) value, store));
addLongField(doc, name, store, (Long) value);
}
} else if (type == FieldType.TIMESTAMP) {
for (Object value : values.getValues()) {
doc.add(new LegacyLongField(name, ((Timestamp) value).getTime(), store));
addLongField(doc, name, store, ((Timestamp) value).getTime());
}
} else if (type == FieldType.EXACT || type == FieldType.PREFIX) {
for (Object value : values.getValues()) {
@@ -361,6 +371,17 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
}
}
private void addLongField(Document doc, String name, Store store, Long longValue) {
if (schema.useLegacyNumericFields()) {
doc.add(new LegacyLongField(name, longValue, store));
} else {
doc.add(new LongPoint(name, longValue));
if (store == Store.YES) {
doc.add(new StoredField(name, longValue));
}
}
}
protected FieldBundle toFieldBundle(Document doc) {
Map<String, FieldDef<V, ?>> allFields = getSchema().getFields();
ListMultimap<String, Object> rawFields = ArrayListMultimap.create();

4
java/com/google/gerrit/lucene/ChangeSubIndex.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.lucene;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.lucene.LuceneChangeIndex.ID2_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.ID_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.UPDATED_SORT_FIELD;
import static com.google.gerrit.server.index.change.ChangeSchemaDefinitions.NAME;
@@ -99,6 +100,9 @@ public class ChangeSubIndex extends AbstractLuceneIndex<Change.Id, ChangeData>
if (f == ChangeField.LEGACY_ID) {
int v = (Integer) getOnlyElement(values.getValues());
doc.add(new NumericDocValuesField(ID_SORT_FIELD, v));
} else if (f == ChangeField.LEGACY_ID_STR) {
String v = (String) getOnlyElement(values.getValues());
doc.add(new NumericDocValuesField(ID2_SORT_FIELD, Integer.valueOf(v)));
} else if (f == ChangeField.UPDATED) {
long t = ((Timestamp) getOnlyElement(values.getValues())).getTime();
doc.add(new NumericDocValuesField(UPDATED_SORT_FIELD, t));

62
java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
@@ -44,6 +45,7 @@ import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.FieldBundle;
@@ -106,6 +108,7 @@ public class LuceneChangeIndex implements ChangeIndex {
static final String UPDATED_SORT_FIELD = sortFieldName(ChangeField.UPDATED);
static final String ID_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID);
static final String ID2_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID_STR);
private static final String CHANGES = "changes";
private static final String CHANGES_OPEN = "open";
@@ -134,12 +137,22 @@ public class LuceneChangeIndex implements ChangeIndex {
private static final String UNRESOLVED_COMMENT_COUNT_FIELD =
ChangeField.UNRESOLVED_COMMENT_COUNT.getName();
static Term idTerm(ChangeData cd) {
return QueryBuilder.intTerm(LEGACY_ID.getName(), cd.getId().get());
@FunctionalInterface
static interface IdTerm {
Term get(String name, int id);
}
static Term idTerm(Change.Id id) {
return QueryBuilder.intTerm(LEGACY_ID.getName(), id.get());
static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, ChangeData cd) {
return idTerm(idTerm, idField, cd.getId());
}
static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, Change.Id id) {
return idTerm.get(idField.getName(), id.get());
}
@FunctionalInterface
static interface ChangeIdExtractor {
Change.Id extract(IndexableField f);
}
private final ListeningExecutorService executor;
@@ -149,6 +162,12 @@ public class LuceneChangeIndex implements ChangeIndex {
private final ChangeSubIndex openIndex;
private final ChangeSubIndex closedIndex;
// TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
private final FieldDef<ChangeData, ?> idField;
private final String idSortFieldName;
private final IdTerm idTerm;
private final ChangeIdExtractor extractor;
@Inject
LuceneChangeIndex(
@GerritServerConfig Config cfg,
@@ -183,6 +202,20 @@ public class LuceneChangeIndex implements ChangeIndex {
new ChangeSubIndex(
schema, sitePaths, dir.resolve(CHANGES_CLOSED), closedConfig, searcherFactory);
}
idField = this.schema.useLegacyNumericFields() ? LEGACY_ID : LEGACY_ID_STR;
idSortFieldName = schema.useLegacyNumericFields() ? ID_SORT_FIELD : ID2_SORT_FIELD;
idTerm =
(name, id) ->
this.schema.useLegacyNumericFields()
? QueryBuilder.intTerm(name, id)
: QueryBuilder.stringTerm(name, Integer.toString(id));
extractor =
(f) ->
Change.id(
this.schema.useLegacyNumericFields()
? f.numericValue().intValue()
: Integer.valueOf(f.stringValue()));
}
@Override
@@ -201,7 +234,7 @@ public class LuceneChangeIndex implements ChangeIndex {
@Override
public void replace(ChangeData cd) {
Term id = LuceneChangeIndex.idTerm(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);
@@ -217,10 +250,10 @@ public class LuceneChangeIndex implements ChangeIndex {
}
@Override
public void delete(Change.Id id) {
Term idTerm = LuceneChangeIndex.idTerm(id);
public void delete(Change.Id changeId) {
Term id = LuceneChangeIndex.idTerm(idTerm, idField, changeId);
try {
Futures.allAsList(openIndex.delete(idTerm), closedIndex.delete(idTerm)).get();
Futures.allAsList(openIndex.delete(id), closedIndex.delete(id)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -256,7 +289,7 @@ public class LuceneChangeIndex implements ChangeIndex {
private Sort getSort() {
return new Sort(
new SortField(UPDATED_SORT_FIELD, SortField.Type.LONG, true),
new SortField(ID_SORT_FIELD, SortField.Type.LONG, true));
new SortField(idSortFieldName, SortField.Type.LONG, true));
}
private class QuerySource implements ChangeDataSource {
@@ -304,7 +337,7 @@ public class LuceneChangeIndex implements ChangeIndex {
throw new StorageException("interrupted");
}
final Set<String> fields = IndexUtils.changeFields(opts);
final Set<String> fields = IndexUtils.changeFields(opts, schema.useLegacyNumericFields());
return new ChangeDataResults(
executor.submit(
new Callable<List<Document>>() {
@@ -325,7 +358,7 @@ public class LuceneChangeIndex implements ChangeIndex {
public ResultSet<FieldBundle> readRaw() {
List<Document> documents;
try {
documents = doRead(IndexUtils.changeFields(opts));
documents = doRead(IndexUtils.changeFields(opts, schema.useLegacyNumericFields()));
} catch (IOException e) {
throw new StorageException(e);
}
@@ -403,9 +436,8 @@ public class LuceneChangeIndex implements ChangeIndex {
List<Document> docs = future.get();
ImmutableList.Builder<ChangeData> result =
ImmutableList.builderWithExpectedSize(docs.size());
String idFieldName = LEGACY_ID.getName();
for (Document doc : docs) {
result.add(toChangeData(fields(doc, fields), fields, idFieldName));
result.add(toChangeData(fields(doc, fields), fields, idField.getName()));
}
return result.build();
} catch (InterruptedException e) {
@@ -446,10 +478,10 @@ public class LuceneChangeIndex implements ChangeIndex {
cd = changeDataFactory.create(parseProtoFrom(proto, ChangeProtoConverter.INSTANCE));
} else {
IndexableField f = Iterables.getFirst(doc.get(idFieldName), null);
Change.Id id = Change.id(f.numericValue().intValue());
// IndexUtils#changeFields ensures either CHANGE or PROJECT is always present.
IndexableField project = doc.get(PROJECT.getName()).iterator().next();
cd = changeDataFactory.create(Project.nameKey(project.stringValue()), id);
cd = changeDataFactory.create(Project.nameKey(project.stringValue()), extractor.extract(f));
}
// Any decoding that is done here must also be done in {@link ElasticChangeIndex}.

61
java/com/google/gerrit/lucene/QueryBuilder.java
View File

@@ -36,6 +36,8 @@ import com.google.gerrit.index.query.TimestampRangePredicate;
import java.util.Date;
import java.util.List;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.document.IntPoint;
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.LegacyNumericRangeQuery;
@@ -49,6 +51,21 @@ import org.apache.lucene.util.LegacyNumericUtils;
@SuppressWarnings("deprecation")
public class QueryBuilder<V> {
@FunctionalInterface
static interface IntTermQuery {
Query get(String name, int value);
}
@FunctionalInterface
static interface IntRangeQuery {
Query get(String name, int min, int max);
}
@FunctionalInterface
static interface LongRangeQuery {
Query get(String name, long min, long max);
}
static Term intTerm(String name, int value) {
BytesRefBuilder builder = new BytesRefBuilder();
LegacyNumericUtils.intToPrefixCoded(value, 0, builder);
@@ -61,12 +78,36 @@ public class QueryBuilder<V> {
return new Term(name, builder.get());
}
static Query intPoint(String name, int value) {
return IntPoint.newExactQuery(name, value);
}
private final Schema<V> schema;
private final org.apache.lucene.util.QueryBuilder queryBuilder;
// TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
private final IntTermQuery intTermQuery;
private final IntRangeQuery intRangeTermQuery;
private final LongRangeQuery longRangeQuery;
public QueryBuilder(Schema<V> schema, Analyzer analyzer) {
this.schema = schema;
queryBuilder = new org.apache.lucene.util.QueryBuilder(analyzer);
intTermQuery =
(name, value) ->
this.schema.useLegacyNumericFields()
? new TermQuery(intTerm(name, value))
: intPoint(name, value);
intRangeTermQuery =
(name, min, max) ->
this.schema.useLegacyNumericFields()
? LegacyNumericRangeQuery.newIntRange(name, min, max, true, true)
: IntPoint.newRangeQuery(name, min, max);
longRangeQuery =
(name, min, max) ->
this.schema.useLegacyNumericFields()
? LegacyNumericRangeQuery.newLongRange(name, min, max, true, true)
: LongPoint.newRangeQuery(name, min, max);
}
public Query toQuery(Predicate<V> p) throws QueryParseException {
@@ -169,20 +210,20 @@ public class QueryBuilder<V> {
} catch (NumberFormatException e) {
throw new QueryParseException("not an integer: " + p.getValue());
}
return new TermQuery(intTerm(p.getField().getName(), value));
return intTermQuery.get(p.getField().getName(), value);
}
private Query intRangeQuery(IndexPredicate<V> p) throws QueryParseException {
if (p instanceof IntegerRangePredicate) {
IntegerRangePredicate<V> r = (IntegerRangePredicate<V>) p;
String name = r.getField().getName();
int minimum = r.getMinimumValue();
int maximum = r.getMaximumValue();
if (minimum == maximum) {
// Just fall back to a standard integer query.
return new TermQuery(intTerm(p.getField().getName(), minimum));
return intTermQuery.get(name, minimum);
}
return LegacyNumericRangeQuery.newIntRange(
r.getField().getName(), minimum, maximum, true, true);
return intRangeTermQuery.get(name, minimum, maximum);
}
throw new QueryParseException("not an integer range: " + p);
}
@@ -190,20 +231,16 @@ public class QueryBuilder<V> {
private Query timestampQuery(IndexPredicate<V> p) throws QueryParseException {
if (p instanceof TimestampRangePredicate) {
TimestampRangePredicate<V> r = (TimestampRangePredicate<V>) p;
return LegacyNumericRangeQuery.newLongRange(
r.getField().getName(),
r.getMinTimestamp().getTime(),
r.getMaxTimestamp().getTime(),
true,
true);
return longRangeQuery.get(
r.getField().getName(), r.getMinTimestamp().getTime(), r.getMaxTimestamp().getTime());
}
throw new QueryParseException("not a timestamp: " + p);
}
private Query notTimestamp(TimestampRangePredicate<V> r) throws QueryParseException {
if (r.getMinTimestamp().getTime() == 0) {
return LegacyNumericRangeQuery.newLongRange(
r.getField().getName(), r.getMaxTimestamp().getTime(), null, true, true);
return longRangeQuery.get(
r.getField().getName(), r.getMaxTimestamp().getTime(), Long.MAX_VALUE);
}
throw new QueryParseException("cannot negate: " + r);
}

17
java/com/google/gerrit/server/change/AbandonUtil.java
View File

@@ -40,7 +40,10 @@ public class AbandonUtil {
private final ChangeCleanupConfig cfg;
private final Provider<ChangeQueryProcessor> queryProvider;
private final ChangeQueryBuilder queryBuilder;
// Provider is needed, because AbandonUtil is singleton, but ChangeQueryBuilder accesses
// index collection, that is only provided when multiversion index module is started.
// TODO(davido); Remove provider again, when support for legacy numeric fields is removed.
private final Provider<ChangeQueryBuilder> queryBuilderProvider;
private final BatchAbandon batchAbandon;
private final InternalUser internalUser;
@@ -49,11 +52,11 @@ public class AbandonUtil {
ChangeCleanupConfig cfg,
InternalUser.Factory internalUserFactory,
Provider<ChangeQueryProcessor> queryProvider,
ChangeQueryBuilder queryBuilder,
Provider<ChangeQueryBuilder> queryBuilderProvider,
BatchAbandon batchAbandon) {
this.cfg = cfg;
this.queryProvider = queryProvider;
this.queryBuilder = queryBuilder;
this.queryBuilderProvider = queryBuilderProvider;
this.batchAbandon = batchAbandon;
internalUser = internalUserFactory.create();
}
@@ -71,7 +74,11 @@ public class AbandonUtil {
}
List<ChangeData> changesToAbandon =
queryProvider.get().enforceVisibility(false).query(queryBuilder.parse(query)).entities();
queryProvider
.get()
.enforceVisibility(false)
.query(queryBuilderProvider.get().parse(query))
.entities();
ImmutableListMultimap.Builder<Project.NameKey, ChangeData> builder =
ImmutableListMultimap.builder();
for (ChangeData cd : changesToAbandon) {
@@ -111,7 +118,7 @@ public class AbandonUtil {
queryProvider
.get()
.enforceVisibility(false)
.query(queryBuilder.parse(newQuery))
.query(queryBuilderProvider.get().parse(newQuery))
.entities();
if (!changesToAbandon.isEmpty()) {
validChanges.add(cd);

10
java/com/google/gerrit/server/index/IndexUtils.java
View File

@@ -16,18 +16,21 @@ package com.google.gerrit.server.index;
import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.project.ProjectField;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.account.AccountField;
import com.google.gerrit.server.index.group.GroupField;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.SingleGroupUser;
import java.io.IOException;
import java.util.Set;
@@ -66,7 +69,8 @@ public final class IndexUtils {
: Sets.union(fields, ImmutableSet.of(AccountField.ID.getName()));
}
public static Set<String> changeFields(QueryOptions opts) {
public static Set<String> changeFields(QueryOptions opts, boolean useLegacyNumericFields) {
FieldDef<ChangeData, ?> idField = useLegacyNumericFields ? LEGACY_ID : LEGACY_ID_STR;
// Ensure we request enough fields to construct a ChangeData. We need both
// change ID and project, which can either come via the Change field or
// separate fields.
@@ -75,10 +79,10 @@ public final class IndexUtils {
// A Change is always sufficient.
return fs;
}
if (fs.contains(PROJECT.getName()) && fs.contains(LEGACY_ID.getName())) {
if (fs.contains(PROJECT.getName()) && fs.contains(idField.getName())) {
return fs;
}
return Sets.union(fs, ImmutableSet.of(LEGACY_ID.getName(), PROJECT.getName()));
return Sets.union(fs, ImmutableSet.of(idField.getName(), PROJECT.getName()));
}
public static Set<String> groupFields(QueryOptions opts) {

3
java/com/google/gerrit/server/index/change/ChangeField.java
View File

@@ -107,6 +107,9 @@ public class ChangeField {
public static final FieldDef<ChangeData, Integer> LEGACY_ID =
integer("legacy_id").stored().build(cd -> cd.getId().get());
public static final FieldDef<ChangeData, String> LEGACY_ID_STR =
exact("legacy_id_str").stored().build(cd -> String.valueOf(cd.getId().get()));
/** Newer style Change-Id key. */
public static final FieldDef<ChangeData, String> ID =
prefix(ChangeQueryBuilder.FIELD_CHANGE_ID).build(changeGetter(c -> c.getKey().get()));

5
java/com/google/gerrit/server/index/change/ChangeIndex.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.LegacyChangeIdPredicate;
import com.google.gerrit.server.query.change.LegacyChangeIdStrPredicate;
public interface ChangeIndex extends Index<Change.Id, ChangeData> {
public interface Factory
@@ -27,6 +28,8 @@ public interface ChangeIndex extends Index<Change.Id, ChangeData> {
@Override
default Predicate<ChangeData> keyPredicate(Change.Id id) {
return new LegacyChangeIdPredicate(id);
return getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(id)
: new LegacyChangeIdStrPredicate(id);
}
}

13
java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
View File

@@ -85,7 +85,18 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
ChangeField.WIP);
// The computation of the 'extension' field is changed, hence reindexing is required.
static final Schema<ChangeData> V56 = schema(V55);
@Deprecated static final Schema<ChangeData> V56 = schema(V55);
// New numeric types: use dimensional points using the k-d tree geo-spatial data structure
// to offer fast single- and multi-dimensional numeric range. As the consequense, integer
// document id type is replaced with string document id type.
static final Schema<ChangeData> V57 =
new Schema.Builder<ChangeData>()
.add(V56)
.remove(ChangeField.LEGACY_ID)
.add(ChangeField.LEGACY_ID_STR)
.legacyNumericFields(false)
.build();
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();

4
java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -461,7 +461,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
if (PAT_LEGACY_ID.matcher(query).matches()) {
Integer id = Ints.tryParse(query);
if (id != null) {
return new LegacyChangeIdPredicate(Change.id(id));
return args.getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(Change.id(id))
: new LegacyChangeIdStrPredicate(Change.id(id));
}
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return new ChangeIdPredicate(parseChangeId(query));

11
java/com/google/gerrit/server/query/change/CommentPredicate.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
import com.google.gerrit.entities.Change;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
@@ -32,9 +33,15 @@ public class CommentPredicate extends ChangeIndexPredicate {
@Override
public boolean match(ChangeData object) {
try {
Predicate<ChangeData> p = Predicate.and(new LegacyChangeIdPredicate(object.getId()), this);
Change.Id id = object.getId();
Predicate<ChangeData> p =
Predicate.and(
index.getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(id)
: new LegacyChangeIdStrPredicate(id),
this);
for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
if (cData.getId().equals(object.getId())) {
if (cData.getId().equals(id)) {
return true;
}
}

6
java/com/google/gerrit/server/query/change/ConflictsPredicate.java
View File

@@ -87,7 +87,11 @@ public class ConflictsPredicate {
List<Predicate<ChangeData>> and = new ArrayList<>(5);
and.add(new ProjectPredicate(c.getProject().get()));
and.add(new RefPredicate(c.getDest().branch()));
and.add(Predicate.not(new LegacyChangeIdPredicate(c.getId())));
and.add(
Predicate.not(
args.getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(c.getId())
: new LegacyChangeIdStrPredicate(c.getId())));
and.add(Predicate.or(filePredicates));
ChangeDataCache changeDataCache = new ChangeDataCache(cd, args.projectCache);

5
java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
View File

@@ -43,7 +43,10 @@ public class FuzzyTopicPredicate extends ChangeIndexPredicate {
return false;
}
try {
Predicate<ChangeData> thisId = new LegacyChangeIdPredicate(cd.getId());
Predicate<ChangeData> thisId =
index.getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(cd.getId())
: new LegacyChangeIdStrPredicate(cd.getId());
Iterable<ChangeData> results =
index.getSource(and(thisId, this), IndexedChangeQuery.oneResult()).read();
return !Iterables.isEmpty(results);

17
java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@@ -55,6 +55,11 @@ import org.eclipse.jgit.lib.Repository;
* holding on to a single instance.
*/
public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChangeQuery> {
@FunctionalInterface
static interface ChangeIdPredicateFactory {
Predicate<ChangeData> create(Change.Id id);
}
private static Predicate<ChangeData> ref(BranchNameKey branch) {
return new RefPredicate(branch.branch());
}
@@ -78,6 +83,9 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
// TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
private final ChangeIdPredicateFactory predicateFactory;
@Inject
InternalChangeQuery(
ChangeQueryProcessor queryProcessor,
@@ -88,6 +96,11 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang
super(queryProcessor, indexes, indexConfig);
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
predicateFactory =
(id) ->
schema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(id)
: new LegacyChangeIdStrPredicate(id);
}
public List<ChangeData> byKey(Change.Key key) {
@@ -99,13 +112,13 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang
}
public List<ChangeData> byLegacyChangeId(Change.Id id) {
return query(new LegacyChangeIdPredicate(id));
return query(predicateFactory.create(id));
}
public List<ChangeData> byLegacyChangeIds(Collection<Change.Id> ids) {
List<Predicate<ChangeData>> preds = new ArrayList<>(ids.size());
for (Change.Id id : ids) {
preds.add(new LegacyChangeIdPredicate(id));
preds.add(predicateFactory.create(id));
}
return query(or(preds));
}

39
java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java Normal file
View File

@@ -0,0 +1,39 @@
// 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.google.gerrit.server.query.change;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import com.google.gerrit.entities.Change;
/** Predicate over change number (aka legacy ID or Change.Id). */
public class LegacyChangeIdStrPredicate extends ChangeIndexPredicate {
protected final Change.Id id;
public LegacyChangeIdStrPredicate(Change.Id id) {
super(LEGACY_ID_STR, ChangeQueryBuilder.FIELD_CHANGE, id.toString());
this.id = id;
}
@Override
public boolean match(ChangeData object) {
return id.equals(object.getId());
}
@Override
public int getCost() {
return 1;
}
}

7
java/com/google/gerrit/server/query/change/MessagePredicate.java
View File

@@ -33,7 +33,12 @@ public class MessagePredicate extends ChangeIndexPredicate {
@Override
public boolean match(ChangeData object) {
try {
Predicate<ChangeData> p = Predicate.and(new LegacyChangeIdPredicate(object.getId()), this);
Predicate<ChangeData> p =
Predicate.and(
index.getSchema().useLegacyNumericFields()
? new LegacyChangeIdPredicate(object.getId())
: new LegacyChangeIdStrPredicate(object.getId()),
this);
for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
if (cData.getId().equals(object.getId())) {
return true;

5
javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
View File

@@ -28,10 +28,11 @@ import org.junit.Ignore;
@Ignore
public class FakeChangeIndex implements ChangeIndex {
static final Schema<ChangeData> V1 = new Schema<>(1, ImmutableList.of(ChangeField.STATUS));
static final Schema<ChangeData> V1 = new Schema<>(1, false, ImmutableList.of(ChangeField.STATUS));
static final Schema<ChangeData> V2 =
new Schema<>(2, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
new Schema<>(
2, false, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
private static class Source implements ChangeDataSource {
private final Predicate<ChangeData> p;

3
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -154,7 +154,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
@Inject protected ChangeInserter.Factory changeFactory;
@Inject protected ChangeQueryBuilder queryBuilder;
@Inject protected Provider<ChangeQueryBuilder> queryBuilderProvider;
@Inject protected GerritApi gApi;
@Inject protected IdentifiedUser.GenericFactory userFactory;
@Inject protected ChangeIndexCollection indexes;
@@ -3052,6 +3052,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery(ChangeIndexPredicate.none());
ChangeQueryBuilder queryBuilder = queryBuilderProvider.get();
for (Predicate<ChangeData> matchingOneChange :
ImmutableList.of(
// One index query, one post-filtering query.