diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt index 8f24a47e03..1ab67d87d5 100644 --- a/Documentation/cmd-stream-events.txt +++ b/Documentation/cmd-stream-events.txt @@ -112,7 +112,8 @@ patchSet:: link:json.html#patchSet[patchSet attribute] submitter:: link:json.html#account[account attribute] -newRev:: The resulting revision of the merge. +newRev:: The state (revision) of the target branch after the operation that +closed the change was completed. eventCreatedOn:: Time in seconds since the UNIX epoch when this event was created. diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index f29383e8e1..d8801eb07b 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -4254,7 +4254,8 @@ Number of threads to use when executing SSH command requests. If additional requests are received while all threads are busy they are queued and serviced in a first-come-first-served order. + -By default, 2x the number of CPUs available to the JVM. +By default, 2x the number of CPUs available to the JVM (but at least 4 +threads). + [NOTE] When SSH daemon is enabled then this setting also defines the max number of diff --git a/java/com/google/gerrit/acceptance/EventRecorder.java b/java/com/google/gerrit/acceptance/EventRecorder.java index 6c6e1bfe2e..cab6b58b37 100644 --- a/java/com/google/gerrit/acceptance/EventRecorder.java +++ b/java/com/google/gerrit/acceptance/EventRecorder.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance; import static com.google.common.truth.Truth.assertThat; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.LinkedListMultimap; @@ -109,7 +110,8 @@ public class EventRecorder { return events; } - private ImmutableList getChangeMergedEvents( + @VisibleForTesting + public ImmutableList getChangeMergedEvents( String project, String branch, int expectedSize) { String key = refEventKey(ChangeMergedEvent.TYPE, project, branch); if (expectedSize == 0) { diff --git a/java/com/google/gerrit/server/config/ThreadSettingsConfig.java b/java/com/google/gerrit/server/config/ThreadSettingsConfig.java index 6cb32cc0e5..c20e0a4ea4 100644 --- a/java/com/google/gerrit/server/config/ThreadSettingsConfig.java +++ b/java/com/google/gerrit/server/config/ThreadSettingsConfig.java @@ -28,7 +28,7 @@ public class ThreadSettingsConfig { @Inject ThreadSettingsConfig(@GerritServerConfig Config cfg) { int cores = Runtime.getRuntime().availableProcessors(); - sshdThreads = cfg.getInt("sshd", "threads", 2 * cores); + sshdThreads = cfg.getInt("sshd", "threads", Math.max(4, 2 * cores)); httpdMaxThreads = cfg.getInt("httpd", "maxThreads", 25); int defaultDatabasePoolLimit = sshdThreads + httpdMaxThreads + 2; databasePoolLimit = cfg.getInt("database", "poolLimit", defaultDatabasePoolLimit); diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java index e9b812d016..3d64b82542 100644 --- a/java/com/google/gerrit/server/git/MergedByPushOp.java +++ b/java/com/google/gerrit/server/git/MergedByPushOp.java @@ -52,7 +52,10 @@ public class MergedByPushOp implements BatchUpdateOp { public interface Factory { MergedByPushOp create( - RequestScopePropagator requestScopePropagator, PatchSet.Id psId, String refName); + RequestScopePropagator requestScopePropagator, + PatchSet.Id psId, + @Assisted("refName") String refName, + @Assisted("mergeResultRevId") String mergeResultRevId); } private final RequestScopePropagator requestScopePropagator; @@ -65,6 +68,7 @@ public class MergedByPushOp implements BatchUpdateOp { private final PatchSet.Id psId; private final String refName; + private final String mergeResultRevId; private Change change; private boolean correctBranch; @@ -82,7 +86,8 @@ public class MergedByPushOp implements BatchUpdateOp { ChangeMerged changeMerged, @Assisted RequestScopePropagator requestScopePropagator, @Assisted PatchSet.Id psId, - @Assisted String refName) { + @Assisted("refName") String refName, + @Assisted("mergeResultRevId") String mergeResultRevId) { this.patchSetInfoFactory = patchSetInfoFactory; this.cmUtil = cmUtil; this.mergedSenderFactory = mergedSenderFactory; @@ -92,6 +97,7 @@ public class MergedByPushOp implements BatchUpdateOp { this.requestScopePropagator = requestScopePropagator; this.psId = psId; this.refName = refName; + this.mergeResultRevId = mergeResultRevId; } public String getMergedIntoRef() { @@ -192,8 +198,7 @@ public class MergedByPushOp implements BatchUpdateOp { } })); - changeMerged.fire( - change, patchSet, ctx.getAccount(), patchSet.getRevision().get(), ctx.getWhen()); + changeMerged.fire(change, patchSet, ctx.getAccount(), mergeResultRevId, ctx.getWhen()); } private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException { diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 0bc5872a97..b8181b475f 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -2869,6 +2869,7 @@ class ReceiveCommits { projectState, notes.getChange().getDest(), checkMergedInto, + checkMergedInto ? inputCommand.getNewId().name() : null, priorPatchSet, priorCommit, psId, @@ -3099,6 +3100,9 @@ class ReceiveCommits { int limit = receiveConfig.maxBatchCommits; int n = 0; for (RevCommit c; (c = walk.next()) != null; ) { + // Even if skipValidation is set, we still get here when at least one plugin + // commit validator requires to validate all commits. In this case, however, + // we don't need to check the commit limit. if (++n > limit && !skipValidation) { logger.atFine().log("Number of new commits exceeds limit of %d", limit); reject( @@ -3169,7 +3173,8 @@ class ReceiveCommits { bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null)); bu.addOp( psId.getParentKey(), - mergedByPushOpFactory.create(requestScopePropagator, psId, refName)); + mergedByPushOpFactory.create( + requestScopePropagator, psId, refName, newTip.getId().getName())); continue COMMIT; } } @@ -3203,7 +3208,7 @@ class ReceiveCommits { bu.addOp( id, mergedByPushOpFactory - .create(requestScopePropagator, req.psId, refName) + .create(requestScopePropagator, req.psId, refName, newTip.getId().getName()) .setPatchSetProvider(req.replaceOp::getPatchSet)); bu.addOp(id, new ChangeProgressOp(progress)); ids.add(id); diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index b15035a17a..4d0d2c3559 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -101,6 +101,7 @@ public class ReplaceOp implements BatchUpdateOp { ProjectState projectState, Branch.NameKey dest, boolean checkMergedInto, + @Nullable String mergeResultRevId, @Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId, @Assisted("priorCommitId") ObjectId priorCommit, @Assisted("patchSetId") PatchSet.Id patchSetId, @@ -133,6 +134,7 @@ public class ReplaceOp implements BatchUpdateOp { private final ProjectState projectState; private final Branch.NameKey dest; private final boolean checkMergedInto; + private final String mergeResultRevId; private final PatchSet.Id priorPatchSetId; private final ObjectId priorCommitId; private final PatchSet.Id patchSetId; @@ -177,6 +179,7 @@ public class ReplaceOp implements BatchUpdateOp { @Assisted ProjectState projectState, @Assisted Branch.NameKey dest, @Assisted boolean checkMergedInto, + @Assisted @Nullable String mergeResultRevId, @Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId, @Assisted("priorCommitId") ObjectId priorCommitId, @Assisted("patchSetId") PatchSet.Id patchSetId, @@ -205,6 +208,7 @@ public class ReplaceOp implements BatchUpdateOp { this.projectState = projectState; this.dest = dest; this.checkMergedInto = checkMergedInto; + this.mergeResultRevId = mergeResultRevId; this.priorPatchSetId = priorPatchSetId; this.priorCommitId = priorCommitId.copy(); this.patchSetId = patchSetId; @@ -231,7 +235,8 @@ public class ReplaceOp implements BatchUpdateOp { String mergedInto = findMergedInto(ctx, dest.get(), commit); if (mergedInto != null) { mergedByPushOp = - mergedByPushOpFactory.create(requestScopePropagator, patchSetId, mergedInto); + mergedByPushOpFactory.create( + requestScopePropagator, patchSetId, mergedInto, mergeResultRevId); } } diff --git a/java/com/google/gerrit/sshd/commands/LsUserRefs.java b/java/com/google/gerrit/sshd/commands/LsUserRefs.java index 1f991e0224..db3e6494fd 100644 --- a/java/com/google/gerrit/sshd/commands/LsUserRefs.java +++ b/java/com/google/gerrit/sshd/commands/LsUserRefs.java @@ -91,7 +91,7 @@ public class LsUserRefs extends SshCommand { try { Map refsMap = permissionBackend - .user(user) + .user(ctx.getUser()) .project(projectName) .filter(repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults()); diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 34c154a61b..e2d2cf47e6 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -2277,6 +2277,34 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { @GerritConfig(name = "receive.maxBatchCommits", value = "2") @Test public void maxBatchCommits() throws Exception { + testMaxBatchCommits(); + } + + @GerritConfig(name = "receive.maxBatchCommits", value = "2") + @Test + public void maxBatchCommitsWithDefaultValidator() throws Exception { + TestValidator validator = new TestValidator(); + RegistrationHandle handle = commitValidators.add("test-validator", validator); + try { + testMaxBatchCommits(); + } finally { + handle.remove(); + } + } + + @GerritConfig(name = "receive.maxBatchCommits", value = "2") + @Test + public void maxBatchCommitsWithValidateAllCommitsValidator() throws Exception { + TestValidator validator = new TestValidator(true); + RegistrationHandle handle = commitValidators.add("test-validator", validator); + try { + testMaxBatchCommits(); + } finally { + handle.remove(); + } + } + + private void testMaxBatchCommits() throws Exception { List commits = new ArrayList<>(); commits.addAll(initChanges(2)); String master = "refs/heads/master"; diff --git a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 759a99ad73..2b10b17296 100644 --- a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -30,9 +30,11 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.ApprovalsUtil; +import com.google.gerrit.server.events.ChangeMergedEvent; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.query.change.ChangeData; import com.google.inject.Inject; +import java.util.List; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.InvalidRemoteException; import org.eclipse.jgit.api.errors.TransportException; @@ -167,6 +169,19 @@ public class SubmitOnPushIT extends AbstractDaemonTest { assertThat(cd.patchSet(psId).getRevision().get()).isEqualTo(c.name()); } + @Test + public void correctNewRevOnMergeByPushToBranch() throws Exception { + grant(project, "refs/heads/master", Permission.PUSH); + push("refs/for/master", PushOneCommit.SUBJECT, "one.txt", "One"); + PushOneCommit.Result r = push("refs/for/master", PushOneCommit.SUBJECT, "two.txt", "Two"); + startEventRecorder(); + git().push().setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master")).call(); + List changeMergedEvents = + eventRecorder.getChangeMergedEvents(project.get(), "refs/heads/master", 2); + assertThat(changeMergedEvents.get(0).newRev).isEqualTo(r.getPatchSet().getRevision().get()); + assertThat(changeMergedEvents.get(1).newRev).isEqualTo(r.getPatchSet().getRevision().get()); + } + @Test public void mergeOnPushToBranchWithChangeMergedInOther() throws Exception { enableCreateNewChangeForAllNotInTarget(); diff --git a/plugins/replication b/plugins/replication index 49597f862d..6038386d7e 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 49597f862d8449da38813c7f6dd8a64c26d95a56 +Subproject commit 6038386d7eb1fd0a29817f24cf40f550266748ee