Store project field in change index

Sometimes we need to lookup a change from the index in order to be
able to load it from the database or notedb. This is e.g. the case
when we only have a Change-Id. In this case we want to reload the
change after the index lookup because the index can be stale. This is
why we don't request any fields from the index when the change is
looked up, so that a rereading is enforced. So far the change ID was
sufficient for rereading because we always loaded the change from the
database, but for rereading the change from notedb, we need the
project name in addition. This is why the project field should be
stored in the index. In the case described above we explicitly don't
want to request the change field which contains the project, since the
index may be stale and requesting the change field wouldn't enforce
rereading the change. To have the project name available for
rereading the change the project field should be always included in
the set of requested fields, when the change field is not requested.

Because the project field was changed we need a new schema version
although it contains the same fields as the current schema version.

Change-Id: I14eefae0ef90accab6972d11babddf0402ce01b5
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-02-04 11:46:55 +01:00
parent 02417753a9
commit 25b6287364
4 changed files with 21 additions and 1 deletions

View File

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE; import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
import static com.google.gerrit.server.index.ChangeField.LEGACY_ID; import static com.google.gerrit.server.index.ChangeField.LEGACY_ID;
import static com.google.gerrit.server.index.ChangeField.PROJECT;
import static com.google.gerrit.server.index.IndexRewriter.CLOSED_STATUSES; import static com.google.gerrit.server.index.IndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.IndexRewriter.OPEN_STATUSES; import static com.google.gerrit.server.index.IndexRewriter.OPEN_STATUSES;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -34,6 +35,7 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
@@ -454,6 +456,10 @@ public class LuceneChangeIndex implements ChangeIndex {
ChangeProtoField.CODEC.decode(cb.bytes, cb.offset, cb.length)); ChangeProtoField.CODEC.decode(cb.bytes, cb.offset, cb.length));
} else { } else {
int id = doc.getField(idFieldName).numericValue().intValue(); int id = doc.getField(idFieldName).numericValue().intValue();
// TODO(ekempin): Pass project to changeDataFactory
@SuppressWarnings("unused")
Project.NameKey project =
new Project.NameKey(doc.getField(PROJECT.getName()).stringValue());
cd = changeDataFactory.create(db.get(), new Change.Id(id)); cd = changeDataFactory.create(db.get(), new Change.Id(id));
} }

View File

@@ -108,7 +108,7 @@ public class ChangeField {
/** Project containing the change. */ /** Project containing the change. */
public static final FieldDef<ChangeData, String> PROJECT = public static final FieldDef<ChangeData, String> PROJECT =
new FieldDef.Single<ChangeData, String>( new FieldDef.Single<ChangeData, String>(
ChangeQueryBuilder.FIELD_PROJECT, FieldType.EXACT, false) { ChangeQueryBuilder.FIELD_PROJECT, FieldType.EXACT, true) {
@Override @Override
public String get(ChangeData input, FillArgs args) public String get(ChangeData input, FillArgs args)
throws OrmException { throws OrmException {

View File

@@ -67,6 +67,7 @@ public class ChangeSchemas {
ChangeField.AUTHOR, ChangeField.AUTHOR,
ChangeField.COMMITTER); ChangeField.COMMITTER);
@Deprecated
static final Schema<ChangeData> V26 = schema( static final Schema<ChangeData> V26 = schema(
ChangeField.LEGACY_ID, ChangeField.LEGACY_ID,
ChangeField.ID, ChangeField.ID,
@@ -104,6 +105,8 @@ public class ChangeSchemas {
ChangeField.COMMITTER, ChangeField.COMMITTER,
ChangeField.DRAFTBY); ChangeField.DRAFTBY);
static final Schema<ChangeData> V27 = schema(V26.getFields().values());
private static Schema<ChangeData> schema(Collection<FieldDef<ChangeData, ?>> fields) { private static Schema<ChangeData> schema(Collection<FieldDef<ChangeData, ?>> fields) {
return new Schema<>(ImmutableList.copyOf(fields)); return new Schema<>(ImmutableList.copyOf(fields));
} }

View File

@@ -15,11 +15,14 @@
package com.google.gerrit.server.query.change; package com.google.gerrit.server.query.change;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.server.index.ChangeField.CHANGE;
import static com.google.gerrit.server.index.ChangeField.PROJECT;
import com.google.auto.value.AutoValue; import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexConfig;
import java.util.HashSet;
import java.util.Set; import java.util.Set;
@AutoValue @AutoValue
@@ -28,6 +31,14 @@ public abstract class QueryOptions {
Set<String> fields) { Set<String> fields) {
checkArgument(start >= 0, "start must be nonnegative: %s", start); checkArgument(start >= 0, "start must be nonnegative: %s", start);
checkArgument(limit > 0, "limit must be positive: %s", limit); checkArgument(limit > 0, "limit must be positive: %s", limit);
// Always include project since it is needed to load the change from notedb.
if (!fields.contains(CHANGE.getName())
&& !fields.contains(PROJECT.getName())) {
fields = new HashSet<>(fields);
fields.add(PROJECT.getName());
}
return new AutoValue_QueryOptions(config, start, limit, return new AutoValue_QueryOptions(config, start, limit,
ImmutableSet.copyOf(fields)); ImmutableSet.copyOf(fields));
} }