Fix racy read in ReindexAfterUpdate when merging
As soon as the GitReferenceUpdatedListener is fired, the GetChanges
task reads all open changes on the branch from the DB. There may be
many of these, and they may be stuck in the queue for a while, during
which time some of these changes may be submitted. Therefore we should
be rereading the changes from the DB when the index task starts, but
we weren't, we were using the previously-read Change entities.
Instead, pass a Change.Id into the Index task and reread it on task
execution time. We need another argument to pass the account ID for
the request context, but we choose this slight ugliness over holding a
tantalizing reference to a stale object.
Change-Id: I63f4b874d09d3e6fa35b7d3a82d0089982526f62
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
index 13d37fc..6bcd92d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
@@ -20,6 +20,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@@ -80,7 +81,8 @@
List<ListenableFuture<Void>> result =
Lists.newArrayListWithCapacity(changes.size());
for (Change c : changes) {
- result.add(executor.submit(new Index(event, c)));
+ result.add(executor.submit(
+ new Index(event, c.getOwner(), c.getId())));
}
return Futures.allAsList(result);
}
@@ -132,19 +134,21 @@
}
private class Index extends Task<Void> {
- private final Change change;
+ private final Account.Id user;
+ private final Change.Id id;
- Index(Event event, Change change) {
+ Index(Event event, Account.Id user, Change.Id id) {
super(event);
- this.change = change;
+ this.user = user;
+ this.id = id;
}
@Override
- protected Void impl() throws IOException {
+ protected Void impl() throws OrmException, IOException {
RequestContext context = new RequestContext() {
@Override
public CurrentUser getCurrentUser() {
- return userFactory.create(change.getOwner());
+ return userFactory.create(user);
}
@Override
@@ -154,7 +158,8 @@
};
RequestContext old = tl.setContext(context);
try {
- indexerFactory.create(executor, indexes).index(db, change);
+ Change c = db.changes().get(id);
+ indexerFactory.create(executor, indexes).index(db, c);
return null;
} finally {
tl.setContext(old);
