copy-approvals: do not lock loose refs when executing BatchRefUpdate When running copy-approvals as standalone program we can further optimize performance by performing BatchRefUpdate on project level without locking all loose refs. This is only supported for the RefDirectory which uses PackedBatchRefUpdate. There is no need to lock all loose refs in the PackedBatchRefUpdate if we know that the copy-approvals program is the only one accessing and modifying the Git repositories. For large Gerrit sites this can save creation of millions of .lock files. Release-Notes: skip Change-Id: Id71cadd910b844011bb3423c2e581bdbe7c28b05
diff --git a/java/com/google/gerrit/pgm/CopyApprovals.java b/java/com/google/gerrit/pgm/CopyApprovals.java index 73b0da6..f62d60c 100644 --- a/java/com/google/gerrit/pgm/CopyApprovals.java +++ b/java/com/google/gerrit/pgm/CopyApprovals.java
@@ -72,7 +72,7 @@ sysInjector.injectMembers(this); try { - recursiveApprovalCopier.persist(); + recursiveApprovalCopier.persistStandalone(); return 0; } catch (Exception e) { throw die(e.getMessage(), e);
diff --git a/java/com/google/gerrit/server/approval/RecursiveApprovalCopier.java b/java/com/google/gerrit/server/approval/RecursiveApprovalCopier.java index 3e5504f..d02b64a 100644 --- a/java/com/google/gerrit/server/approval/RecursiveApprovalCopier.java +++ b/java/com/google/gerrit/server/approval/RecursiveApprovalCopier.java
@@ -50,6 +50,7 @@ import java.util.concurrent.ExecutorService; import java.util.function.Consumer; import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.internal.storage.file.RefDirectory; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; @@ -89,9 +90,14 @@ this.executor = MoreExecutors.listeningDecorator(executor); } - public void persist() + /** + * This method assumes it is used as a standalone program having exclusive access to the Git + * repositories. Therefore, it will (safely) skip locking of the loose refs when performing batch + * ref-updates. + */ + public void persistStandalone() throws RepositoryNotFoundException, IOException, InterruptedException, ExecutionException { - persist(repositoryManager.list(), null); + persist(repositoryManager.list(), null, false); if (failedForAtLeastOneProject) { throw new RuntimeException("There were errors, check the logs for details"); @@ -100,11 +106,13 @@ public void persist(Project.NameKey project, @Nullable Consumer<Change> labelsCopiedListener) throws IOException, RepositoryNotFoundException, InterruptedException, ExecutionException { - persist(ImmutableList.of(project), labelsCopiedListener); + persist(ImmutableList.of(project), labelsCopiedListener, true); } private void persist( - Collection<Project.NameKey> projects, @Nullable Consumer<Change> labelsCopiedListener) + Collection<Project.NameKey> projects, + @Nullable Consumer<Change> labelsCopiedListener, + boolean shouldLockLooseRefs) throws InterruptedException, ExecutionException, RepositoryNotFoundException, IOException { List<ListenableFuture<Void>> copyApprovalsTasks = new LinkedList<>(); for (Project.NameKey project : projects) { @@ -112,7 +120,8 @@ } Futures.successfulAsList(copyApprovalsTasks).get(); - List<ListenableFuture<Void>> batchRefUpdateTasks = submitBatchRefUpdateTasks(); + List<ListenableFuture<Void>> batchRefUpdateTasks = + submitBatchRefUpdateTasks(shouldLockLooseRefs); Futures.successfulAsList(batchRefUpdateTasks).get(); } @@ -187,7 +196,7 @@ } } - private List<ListenableFuture<Void>> submitBatchRefUpdateTasks() { + private List<ListenableFuture<Void>> submitBatchRefUpdateTasks(boolean shouldLockLooseRefs) { logger.atInfo().log("submitting batch ref-update tasks"); List<ListenableFuture<Void>> futures = new LinkedList<>(); for (Map.Entry<Project.NameKey, List<ReceiveCommand>> e : pendingRefUpdates.entrySet()) { @@ -196,20 +205,26 @@ futures.add( executor.submit( () -> { - executeRefUpdates(project, updates); + executeRefUpdates(project, updates, shouldLockLooseRefs); return null; })); } return futures; } - private void executeRefUpdates(Project.NameKey project, List<ReceiveCommand> updates) + private void executeRefUpdates( + Project.NameKey project, List<ReceiveCommand> updates, boolean shouldLockLooseRefs) throws RepositoryNotFoundException, IOException { logger.atInfo().log( "executing batch ref-update for project %s, size %d", project, updates.size()); try (Repository repository = repositoryManager.openRepository(project)) { RefDatabase refdb = repository.getRefDatabase(); - BatchRefUpdate bu = refdb.newBatchUpdate(); + BatchRefUpdate bu; + if (refdb instanceof RefDirectory) { + bu = ((RefDirectory) refdb).newBatchUpdate(shouldLockLooseRefs); + } else { + bu = refdb.newBatchUpdate(); + } bu.addCommand(updates); RefUpdateUtil.executeChecked(bu, repository); }
diff --git a/javatests/com/google/gerrit/acceptance/api/change/CopyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/CopyApprovalsIT.java index f97802e..7acf957 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/CopyApprovalsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/CopyApprovalsIT.java
@@ -77,7 +77,7 @@ u.save(); } - recursiveApprovalCopier.persist(); + recursiveApprovalCopier.persistStandalone(); ApprovalInfo vote1 = Iterables.getOnlyElement( @@ -127,7 +127,7 @@ correctMetaRefObjectId = ru.getOldObjectId(); try { - recursiveApprovalCopier.persist(); + recursiveApprovalCopier.persistStandalone(); fail("Expected exception when a project contains corrupt change"); } catch (Exception e) { assertThat(e.getMessage()).contains("check the logs");
diff --git a/modules/jgit b/modules/jgit index d013761..cb90ed0 160000 --- a/modules/jgit +++ b/modules/jgit
@@ -1 +1 @@ -Subproject commit d01376106af8800017ac3c08d7c7ac1fd5ccc9ee +Subproject commit cb90ed08526bd51f04e5d72e3ba3cf5bd30c11e4