opendev/gerrit
Code Issues Proposed changes

Index full sortkey field in secondary index

Browse Source 
Using the sort key of the last element for pagination only works as
long as every sort key is unique. This is true for the general
definition of this field in the Change object, which includes the
unique change ID at the end of the hex string. Previously, we were
incorrectly truncating the change ID off, resulting in many changes in
the same sort key bucket and thus broken pagination.

Having two different definitions of sort key in the same running
server makes it a bit ugly to handle SortKeyPredicates, since the
definition of min/max value is now schema dependent, but at least we
can keep the same field name.

Don't @Deprecate the new SORTKEY field. We were originally hoping to
remove this field and depend only on the UPDATED field (with the
change ID as tiebreaker), but as long as this field is used for
pagination, we have to keep it around. This is because we can't be
sure a secondary index will be able to express a query like "(field X,
field Y) > (N, M)" to allow us to restart a query in the middle of an
UPDATED bucket. We may still decide to scrap the current pagination
system but that will probably not happen until we kill the SQL index
code.

Change-Id: Icb760dbacd01939e5e4936ef87165b6dddcacdc0
This commit is contained in:
Dave Borowitz
2013-09-20 14:55:15 -07:00
parent 8830bb53a4
commit 2e0f89d814
8 changed files with 102 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -250,8 +250,8 @@ public class LuceneChangeIndex implements ChangeIndex {
if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
indexes.add(closedIndex);
}
return new QuerySource(indexes, QueryBuilder.toQuery(p), limit,
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(p));
return new QuerySource(indexes, QueryBuilder.toQuery(schema, p), limit,
ChangeQueryBuilder.hasNonTrivialSortKeyAfter(schema, p));
}
@Override
@@ -300,7 +300,6 @@ public class LuceneChangeIndex implements ChangeIndex {
@Override
public ResultSet<ChangeData> read() throws OrmException {
IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
@SuppressWarnings("deprecation")
Sort sort = new Sort(
new SortField(
ChangeField.SORTKEY.getName(),

44
gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.FieldType;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.index.RegexPredicate;
import com.google.gerrit.server.index.Schema;
import com.google.gerrit.server.index.TimestampRangePredicate;
import com.google.gerrit.server.query.AndPredicate;
import com.google.gerrit.server.query.NotPredicate;
@@ -54,26 +55,27 @@ public class QueryBuilder {
return intTerm(ID_FIELD, cd.getId().get());
}
public static Query toQuery(Predicate<ChangeData> p)
public static Query toQuery(Schema<ChangeData> schema, Predicate<ChangeData> p)
throws QueryParseException {
if (p instanceof AndPredicate) {
return and(p);
return and(schema, p);
} else if (p instanceof OrPredicate) {
return or(p);
return or(schema, p);
} else if (p instanceof NotPredicate) {
return not(p);
return not(schema, p);
} else if (p instanceof IndexPredicate) {
return fieldQuery((IndexPredicate<ChangeData>) p);
return fieldQuery(schema, (IndexPredicate<ChangeData>) p);
} else {
throw new QueryParseException("cannot create query for index: " + p);
}
}
private static Query or(Predicate<ChangeData> p) throws QueryParseException {
private static Query or(Schema<ChangeData> schema, Predicate<ChangeData> p)
throws QueryParseException {
try {
BooleanQuery q = new BooleanQuery();
for (int i = 0; i < p.getChildCount(); i++) {
q.add(toQuery(p.getChild(i)), SHOULD);
q.add(toQuery(schema, p.getChild(i)), SHOULD);
}
return q;
} catch (BooleanQuery.TooManyClauses e) {
@@ -81,7 +83,8 @@ public class QueryBuilder {
}
}
private static Query and(Predicate<ChangeData> p) throws QueryParseException {
private static Query and(Schema<ChangeData> schema, Predicate<ChangeData> p)
throws QueryParseException {
try {
BooleanQuery b = new BooleanQuery();
List<Query> not = Lists.newArrayListWithCapacity(p.getChildCount());
@@ -92,10 +95,10 @@ public class QueryBuilder {
if (n instanceof TimestampRangePredicate) {
b.add(notTimestamp((TimestampRangePredicate<ChangeData>) n), MUST);
} else {
not.add(toQuery(n));
not.add(toQuery(schema, n));
}
} else {
b.add(toQuery(c), MUST);
b.add(toQuery(schema, c), MUST);
}
}
for (Query q : not) {
@@ -107,7 +110,8 @@ public class QueryBuilder {
}
}
private static Query not(Predicate<ChangeData> p) throws QueryParseException {
private static Query not(Schema<ChangeData> schema, Predicate<ChangeData> p)
throws QueryParseException {
Predicate<ChangeData> n = p.getChild(0);
if (n instanceof TimestampRangePredicate) {
return notTimestamp((TimestampRangePredicate<ChangeData>) n);
@@ -116,12 +120,12 @@ public class QueryBuilder {
// Lucene does not support negation, start with all and subtract.
BooleanQuery q = new BooleanQuery();
q.add(new MatchAllDocsQuery(), MUST);
q.add(toQuery(n), MUST_NOT);
q.add(toQuery(schema, n), MUST_NOT);
return q;
}
private static Query fieldQuery(IndexPredicate<ChangeData> p)
throws QueryParseException {
private static Query fieldQuery(Schema<ChangeData> schema,
IndexPredicate<ChangeData> p) throws QueryParseException {
if (p.getType() == FieldType.INTEGER) {
return intQuery(p);
} else if (p.getType() == FieldType.TIMESTAMP) {
@@ -133,7 +137,7 @@ public class QueryBuilder {
} else if (p.getType() == FieldType.FULL_TEXT) {
return fullTextQuery(p);
} else if (p instanceof SortKeyPredicate) {
return sortKeyQuery((SortKeyPredicate) p);
return sortKeyQuery(schema, (SortKeyPredicate) p);
} else {
throw badFieldType(p.getType());
}
@@ -158,12 +162,14 @@ public class QueryBuilder {
return new TermQuery(intTerm(p.getField().getName(), value));
}
private static Query sortKeyQuery(SortKeyPredicate p) {
private static Query sortKeyQuery(Schema<ChangeData> schema, SortKeyPredicate p) {
long min = p.getMinValue(schema);
long max = p.getMaxValue(schema);
return NumericRangeQuery.newLongRange(
p.getField().getName(),
p.getMinValue() != Long.MIN_VALUE ? p.getMinValue() : null,
p.getMaxValue() != Long.MAX_VALUE ? p.getMaxValue() : null,
true, true);
min != Long.MIN_VALUE ? min : null,
max != Long.MAX_VALUE ? max : null,
false, false);
}
private static Query timestampQuery(IndexPredicate<ChangeData> p)