From 1732f75709d987710b220a2807b1bc54a5b324d7 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 26 Jul 2012 11:36:58 -0700 Subject: [PATCH] Use BatchRefUpdate to execute reference changes Some storage backends for JGit are able to update multiple references in a single pass efficiently. Take advantage of this by pushing any normal reference updates (such as direct push or branch create) into a single BatchRefUpdate object. Change-Id: Iadd7c5a3271f5ab8eaf31881392c85dd5b0a709d --- .../server/git/MultiProgressMonitor.java | 21 +++++++++- .../gerrit/server/git/ReceiveCommits.java | 38 +++++++++++++------ 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MultiProgressMonitor.java index dbb849c61d..23d8dada7b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MultiProgressMonitor.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MultiProgressMonitor.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.git; import static java.util.concurrent.TimeUnit.NANOSECONDS; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ProgressMonitor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,7 +59,7 @@ public class MultiProgressMonitor { private static final char NO_SPINNER = ' '; /** Handle for a sub-task. */ - public class Task { + public class Task implements ProgressMonitor { private final String name; private final int total; private volatile int count; @@ -76,6 +77,7 @@ public class MultiProgressMonitor { * * @param completed number of work units completed. */ + @Override public void update(final int completed) { count += completed; if (total != UNKNOWN) { @@ -97,6 +99,23 @@ public class MultiProgressMonitor { wakeUp(); } } + + @Override + public void start(int totalTasks) { + } + + @Override + public void beginTask(String title, int totalWork) { + } + + @Override + public void endTask() { + } + + @Override + public boolean isCancelled() { + return false; + } } private final OutputStream out; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index e7150bb9a1..35658f6758 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -68,6 +68,7 @@ import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.AbbreviatedObjectId; +import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; @@ -253,6 +254,7 @@ public class ReceiveCommits { private Task closeProgress; private Task commandProgress; private MessageSender messageSender; + private BatchRefUpdate batch; @Inject ReceiveCommits(final ReviewDb db, @@ -460,6 +462,10 @@ public class ReceiveCommits { closeProgress = progress.beginSubTask("closed", UNKNOWN); commandProgress = progress.beginSubTask("refs", UNKNOWN); + batch = repo.getRefDatabase().newBatchUpdate(); + batch.setRefLogIdent(rp.getRefLogIdent()); + batch.setRefLogMessage("push", true); + parseCommands(commands); if (newChange != null && newChange.getResult() == NOT_ATTEMPTED) { createNewChanges(); @@ -469,6 +475,22 @@ public class ReceiveCommits { doReplaces(); replaceProgress.end(); + if (!batch.getCommands().isEmpty()) { + try { + batch.execute(rp.getRevWalk(), commandProgress); + } catch (IOException err) { + int cnt = 0; + for (ReceiveCommand cmd : batch.getCommands()) { + if (cmd.getResult() == NOT_ATTEMPTED) { + cmd.setResult(REJECTED_OTHER_REASON, "internal server error"); + cnt++; + } + } + log.error(String.format( + "Failed to store %d refs in %s", cnt, project.getName()), err); + } + } + if (!errors.isEmpty()) { for (Error error : errors.keySet()) { rp.sendMessage(buildError(error, errors.get(error))); @@ -691,9 +713,7 @@ public class ReceiveCommits { RefControl ctl = projectControl.controlForRef(cmd.getRefName()); if (ctl.canCreate(rp.getRevWalk(), obj)) { validateNewCommits(ctl, cmd); - if (cmd.getResult() == NOT_ATTEMPTED) { - cmd.execute(rp); - } + batch.addCommand(cmd); } else { errors.put(Error.CREATE, ctl.getRefName()); reject(cmd, "can not create new references"); @@ -708,9 +728,7 @@ public class ReceiveCommits { } validateNewCommits(ctl, cmd); - if (cmd.getResult() == NOT_ATTEMPTED) { - cmd.execute(rp); - } + batch.addCommand(cmd); } else { if (GitRepositoryManager.REF_CONFIG.equals(ctl.getRefName())) { errors.put(Error.CONFIG_UPDATE, GitRepositoryManager.REF_CONFIG); @@ -743,9 +761,7 @@ public class ReceiveCommits { private void parseDelete(final ReceiveCommand cmd) { RefControl ctl = projectControl.controlForRef(cmd.getRefName()); if (ctl.canDelete()) { - if (cmd.getResult() == NOT_ATTEMPTED) { - cmd.execute(rp); - } + batch.addCommand(cmd); } else { if (GitRepositoryManager.REF_CONFIG.equals(ctl.getRefName())) { reject(cmd, "cannot delete project configuration"); @@ -778,9 +794,7 @@ public class ReceiveCommits { } if (ctl.canForceUpdate()) { - if (cmd.getResult() == NOT_ATTEMPTED) { - cmd.execute(rp); - } + batch.setAllowNonFastForwards(true).addCommand(cmd); } else { cmd.setResult(REJECTED_NONFASTFORWARD, " need '" + PermissionRule.FORCE_PUSH + "' privilege.");