opendev/gerrit
Code Issues Proposed changes

Fix racy read in ReindexAfterUpdate when merging

Browse Source 
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
This commit is contained in:
Dave Borowitz
2014-11-17 16:16:40 -08:00
parent 99c6c1fd32
commit d84a373ea2
1 changed files with 12 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
View File

@@ -20,6 +20,7 @@ import com.google.common.util.concurrent.Futures;
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 @@ public class ReindexAfterUpdate implements GitReferenceUpdatedListener {
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 @@ public class ReindexAfterUpdate implements GitReferenceUpdatedListener {
}
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 @@ public class ReindexAfterUpdate implements GitReferenceUpdatedListener {
};
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);