Fix pushing a first patchset does not trigger replication

As mentioned in the replies of the issue[1]:

Since change 71424[2], a call to setUpdateRef(false) was added on the
ChangeInserter. BatchUpdate.addRefUpdate is what causes the
batchRefUpdate member to be initialised, but when
ChangeInserter.setUpdateRef is false, the ChangeInserter.updateRepo
method does not call addRefUpdate. So when addRefUpdate is not called,
batchRefUpdate remains null. And in BatchUpdate.execute, the call to
gitRefUpdated.fire(project, batchRefUpdate) is only called when
batchRefUpdate is not null. Result is that ref-updated events are not
fired when a new change is uploaded.

To clarify, updateRef needs to be set to true to keep the same flow as
when the command is created on the spot. Or else we would need to get
access to the RepoContext created in executeRefUpdates somehow, which
would generate a special case flow.

Simply changing to setUpdateRef(true) would result in a LOCK_FAILURE
since it would create a duplicate to an already in progress command and
they would be trying to access the same ref simultaneously.

The idea of the fix is to set the command which was already completed
beforehand instead of creating a duplicate just so the event gets fired
properly when checking its status.

[1] https://code.google.com/p/gerrit/issues/detail?id=3767
[2] https://gerrit-review.googlesource.com/#/c/71424/2

Bug: Issue 3767
Change-Id: I5abd0da87058a4f5a70e44be284c4091aa04c8ec
This commit is contained in:
Alexandre Philbert
2016-01-19 15:19:02 -05:00
committed by David Pursehouse
parent dd41c53bd7
commit 72cf2fc65f
2 changed files with 14 additions and 3 deletions

View File

@@ -97,6 +97,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private Set<Account.Id> extraCC;
private Map<String, Short> approvals;
private RequestScopePropagator requestScopePropagator;
private ReceiveCommand updateRefCommand;
private boolean runHooks;
private boolean sendMail;
private boolean updateRef;
@@ -137,6 +138,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
this.reviewers = Collections.emptySet();
this.extraCC = Collections.emptySet();
this.approvals = Collections.emptyMap();
this.updateRefCommand = null;
this.runHooks = true;
this.sendMail = true;
this.updateRef = true;
@@ -204,6 +206,10 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
return this;
}
public void setUpdateRefCommand(ReceiveCommand cmd) {
updateRefCommand = cmd;
}
public PatchSet getPatchSet() {
return patchSet;
}
@@ -237,8 +243,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
if (!updateRef) {
return;
}
ctx.addRefUpdate(
new ReceiveCommand(ObjectId.zeroId(), commit, patchSet.getRefName()));
if (updateRefCommand == null) {
ctx.addRefUpdate(
new ReceiveCommand(ObjectId.zeroId(), commit, patchSet.getRefName()));
} else {
ctx.addRefUpdate(updateRefCommand);
}
}
@Override

View File

@@ -1734,6 +1734,7 @@ public class ReceiveCommits {
.setValidatePolicy(CommitValidators.Policy.NONE);
cmd = new ReceiveCommand(ObjectId.zeroId(), c,
ins.getPatchSet().getRefName());
ins.setUpdateRefCommand(cmd);
}
CheckedFuture<Void, RestApiException> insertChange() throws IOException {
@@ -1787,7 +1788,7 @@ public class ReceiveCommits {
.setMessage(msg)
.setRequestScopePropagator(requestScopePropagator)
.setSendMail(true)
.setUpdateRef(false));
.setUpdateRef(true));
if (magicBranch != null) {
bu.addOp(
ins.getChange().getId(),