Add limit to number of commits that ReceiveCommits will validate

Compared with simply walking the commits with JGit, running Gerrit
commit validators is quite expensive. When pushing a large number of
commits directly to a branch, validation may exceed the timeout allowed
by AsyncReceiveCommits. For example, pushing the full Linux kernel
history of 650k commits allows only 370 microseconds of validation time
per commit, if validation is allowed to take up the full 4 minute
default AsyncReceiveCommits limit. Gerrit's validators have never been
particularly optimized, so it wouldn't be entirely surprising to see a
timeout in this case, particularly if the Gerrit server is under
moderate to heavy load.

Add a limit configured with receive.maxBatchCommits, analogous to the
existing receive.maxBatchChanges. The options are still separate:
maxBatchChanges is about creating changes, which is a far more
heavyweight operation as it needs to write change metadata, and
accidentally pushing too many changes is a bigger mess to clean up.

Change-Id: I4b81b1f99d9dafdc365ff66e0fb812877355e3b9

Documentation/config-gerrit.txt

@@ -3541,6 +3541,15 @@ number of changes for review by mistake.
 +
 Default is zero, no limit.
 
+[[receive.maxBatchCommits]]receive.maxBatchCommits::
++
+The maximum number of commits that Gerrit allows to be pushed in a batch
+directly to a branch when link:user-upload.html#bypass_review[bypassing review].
+This limit can be bypassed if a user link:user-upload.html#skip_validation[skips
+validation].
++
+Default is 10000.
+
 [[receive.maxObjectSizeLimit]]receive.maxObjectSizeLimit::
 +
 Maximum allowed Git object size that 'receive-pack' will accept.

Documentation/error-messages.txt

@@ -32,6 +32,7 @@ occurring and what can be done to solve it.
 * link:error-prohibited-by-gerrit.html[prohibited by Gerrit]
 * link:error-project-not-found.html[Project not found: ...]
 * link:error-same-change-id-in-multiple-changes.html[same Change-Id in multiple changes]
+* link:error-too-many-commits.html[too many commits]
 * link:error-upload-denied.html[Upload denied for project \'...']
 * link:error-not-allowed-to-upload-merges.html[you are not allowed to upload merges]
 

Documentation/error-too-many-commits.txt

@@ -0,0 +1,20 @@
+= too many commits
+
+This error occurs when a push directly to a branch
+link:user-upload.html#bypass_review[bypassing review] contains more commits than
+the server is able to validate in a single batch.
+
+The recommended way to avoid this message is to use the
+link:user-upload.html#skip_validation[`skip-validation` push option]. Depending
+on the number of commits, it may also be feasible to split the push into smaller
+batches.
+
+The actual limit is controlled by a
+link:config-gerrit.html#receive.maxBatchCommits[server config option].
+
+GERRIT
+------
+Part of link:error-messages.html[Gerrit Error Messages]
+
+SEARCHBOX
+---------

Documentation/user-upload.txt

@@ -549,6 +549,11 @@ This option only applies when pushing directly to a branch bypassing review.
 Validation also occurs when pushing new changes for review, and that type of
 validation cannot be skipped.
 
+The `skip-validation` option is always required when pushing
+link:error-too-many-commits.html[more than a certain number of commits]. This is
+the recommended approach when pushing lots of old history, since some validators
+would require rewriting history in order to make them pass.
+
 
 [[auto_merge]]
 === Auto-Merge during Push

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -23,10 +23,12 @@ import static com.google.gerrit.acceptance.GitUtil.pushHead;
 import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
 import static com.google.gerrit.extensions.common.EditInfoSubject.assertThat;
+import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.project.Util.category;
 import static com.google.gerrit.server.project.Util.value;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.collect.ImmutableList;
@@ -35,6 +37,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Streams;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
@@ -62,10 +65,12 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.git.receive.ReceiveConstants;
+import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.project.Util;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -1766,6 +1771,29 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     assertThat(getPublishedComments(r.getChangeId())).isEmpty();
   }
 
+  @GerritConfig(name = "receive.maxBatchCommits", value = "2")
+  @Test
+  public void maxBatchCommits() throws Exception {
+    List<RevCommit> commits = new ArrayList<>();
+    commits.addAll(initChanges(2));
+    String master = "refs/heads/master";
+    assertPushOk(pushHead(testRepo, master), master);
+
+    commits.addAll(initChanges(3));
+    assertPushRejected(pushHead(testRepo, master), master, "too many commits");
+
+    grantSkipValidation(project, master, SystemGroupBackend.REGISTERED_USERS);
+    PushResult r =
+        pushHead(testRepo, master, false, false, ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+    assertPushOk(r, master);
+
+    // No open changes; branch was advanced.
+    String q = commits.stream().map(ObjectId::name).collect(joining(" OR commit:", "commit:", ""));
+    assertThat(gApi.changes().query(q).get()).isEmpty();
+    assertThat(gApi.projects().name(project.get()).branch(master).get().revision)
+        .isEqualTo(Iterables.getLast(commits).name());
+  }
+
   private DraftInput newDraft(String path, int line, String message) {
     DraftInput d = new DraftInput();
     d.path = path;
@@ -1839,11 +1867,21 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
   }
 
   private List<RevCommit> createChanges(int n, String refsFor) throws Exception {
-    return createChanges(n, refsFor, ImmutableList.<String>of());
+    return createChanges(n, refsFor, ImmutableList.of());
   }
 
   private List<RevCommit> createChanges(int n, String refsFor, List<String> footerLines)
       throws Exception {
+    List<RevCommit> commits = initChanges(n, footerLines);
+    assertPushOk(pushHead(testRepo, refsFor, false), refsFor);
+    return commits;
+  }
+
+  private List<RevCommit> initChanges(int n) throws Exception {
+    return initChanges(n, ImmutableList.of());
+  }
+
+  private List<RevCommit> initChanges(int n, List<String> footerLines) throws Exception {
     List<RevCommit> commits = new ArrayList<>(n);
     for (int i = 1; i <= n; i++) {
       String msg = "Change " + i;
@@ -1863,7 +1901,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
       testRepo.getRevWalk().parseBody(c);
       commits.add(c);
     }
-    assertPushOk(pushHead(testRepo, refsFor, false), refsFor);
     return commits;
   }
 
@@ -1934,4 +1971,15 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     config.getProject().setCreateNewChangeForAllNotInTarget(InheritableBoolean.TRUE);
     saveProjectConfig(project, config);
   }
+
+  private void grantSkipValidation(Project.NameKey project, String ref, AccountGroup.UUID groupUuid)
+      throws Exception {
+    // See SKIP_VALIDATION implementation in default permission backend.
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    Util.allow(config, Permission.FORGE_AUTHOR, groupUuid, ref);
+    Util.allow(config, Permission.FORGE_COMMITTER, groupUuid, ref);
+    Util.allow(config, Permission.FORGE_SERVER, groupUuid, ref);
+    Util.allow(config, Permission.PUSH_MERGE, groupUuid, "refs/for/" + ref);
+    saveProjectConfig(project, config);
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -2710,9 +2710,22 @@ class ReceiveCommits {
       ListMultimap<ObjectId, Ref> existing = changeRefsById();
       walk.markStart((RevCommit) parsedObject);
       markHeadsAsUninteresting(walk, cmd.getRefName());
-      int i = 0;
+      int limit = receiveConfig.maxBatchCommits;
+      int n = 0;
       for (RevCommit c; (c = walk.next()) != null; ) {
-        i++;
+        if (++n > limit) {
+          logDebug("Number of new commits exceeds limit of {}", limit);
+          addMessage(
+              "Cannot push more than "
+                  + limit
+                  + " commits to "
+                  + branch.get()
+                  + " without "
+                  + PUSH_OPTION_SKIP_VALIDATION
+                  + " option");
+          reject(cmd, "too many commits");
+          return;
+        }
         if (existing.keySet().contains(c)) {
           continue;
         } else if (!validCommit(walk, perm, branch, cmd, c)) {
@@ -2725,7 +2738,7 @@ class ReceiveCommits {
           missingFullName = false;
         }
       }
-      logDebug("Validated {} new commits", i);
+      logDebug("Validated {} new commits", n);
     } catch (IOException err) {
       cmd.setResult(REJECTED_MISSING_OBJECT);
       logError("Invalid pack upload; one or more objects weren't sent", err);

gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveConfig.java

@@ -28,6 +28,7 @@ class ReceiveConfig {
   final boolean checkMagicRefs;
   final boolean checkReferencedObjectsAreReachable;
   final boolean allowDrafts;
+  final int maxBatchCommits;
   private final int systemMaxBatchChanges;
   private final AccountLimits.Factory limitsFactory;
 
@@ -37,6 +38,7 @@ class ReceiveConfig {
     checkReferencedObjectsAreReachable =
         config.getBoolean("receive", null, "checkReferencedObjectsAreReachable", true);
     allowDrafts = config.getBoolean("change", null, "allowDrafts", true);
+    maxBatchCommits = config.getInt("receive", null, "maxBatchCommits", 10000);
     systemMaxBatchChanges = config.getInt("receive", "maxBatchChanges", 0);
     this.limitsFactory = limitsFactory;
   }