From c59626ad7a06d47dcf6c223d0c8ceaec2e63e276 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 24 May 2021 09:53:08 -0700 Subject: [PATCH] RepoCommand: Retry commit on LockFailure When the target repository is receiving commits from other sources, the repo command commit can fail with a LOCK_FAILURE. We could let callers retry, but then the command needs to redo all the work (opening all subrepos to recreate the tree). Retry the commit in LOCK_FAILURE inside the command. The commit rewrites the whole tree, so it shouldn't have merge errors. Use an exponential delay with jitter for the retries. Change-Id: I517b6f2afd16a4b695e6cf471b5d6cf492024ec4 Signed-off-by: Ivan Frade --- .../org/eclipse/jgit/gitrepo/RepoCommand.java | 110 +++++++++++------- 1 file changed, 67 insertions(+), 43 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java index 5bf95def9..552315d43 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java @@ -80,6 +80,13 @@ * @since 3.4 */ public class RepoCommand extends GitCommand { + private static final int LOCK_FAILURE_MAX_RETRIES = 5; + + // Retry exponentially with delays in this range + private static final int LOCK_FAILURE_MIN_RETRY_DELAY_MILLIS = 50; + + private static final int LOCK_FAILURE_MAX_RETRY_DELAY_MILLIS = 5000; + private String manifestPath; private String baseUri; private URI targetUri; @@ -686,50 +693,22 @@ public RevCommit call() throws GitAPIException { builder.finish(); ObjectId treeId = index.writeTree(inserter); - // Create a Commit object, populate it and write it - ObjectId headId = repo.resolve(targetBranch + "^{commit}"); //$NON-NLS-1$ - if (headId != null && rw.parseCommit(headId).getTree().getId().equals(treeId)) { - // No change. Do nothing. - return rw.parseCommit(headId); + long prevDelay = 0; + for (int i = 0; i < LOCK_FAILURE_MAX_RETRIES - 1; i++) { + try { + return commitTreeOnCurrentTip( + inserter, rw, treeId); + } catch (ConcurrentRefUpdateException e) { + prevDelay = FileUtils.delay(prevDelay, + LOCK_FAILURE_MIN_RETRY_DELAY_MILLIS, + LOCK_FAILURE_MAX_RETRY_DELAY_MILLIS); + Thread.sleep(prevDelay); + repo.getRefDatabase().refresh(); + } } - - CommitBuilder commit = new CommitBuilder(); - commit.setTreeId(treeId); - if (headId != null) - commit.setParentIds(headId); - commit.setAuthor(author); - commit.setCommitter(author); - commit.setMessage(RepoText.get().repoCommitMessage); - - ObjectId commitId = inserter.insert(commit); - inserter.flush(); - - RefUpdate ru = repo.updateRef(targetBranch); - ru.setNewObjectId(commitId); - ru.setExpectedOldObjectId(headId != null ? headId : ObjectId.zeroId()); - Result rc = ru.update(rw); - - switch (rc) { - case NEW: - case FORCED: - case FAST_FORWARD: - // Successful. Do nothing. - break; - case REJECTED: - case LOCK_FAILURE: - throw new ConcurrentRefUpdateException( - MessageFormat.format( - JGitText.get().cannotLock, targetBranch), - ru.getRef(), - rc); - default: - throw new JGitInternalException(MessageFormat.format( - JGitText.get().updatingRefFailed, - targetBranch, commitId.name(), rc)); - } - - return rw.parseCommit(commitId); - } catch (GitAPIException | IOException e) { + // In the last try, just propagate the exceptions + return commitTreeOnCurrentTip(inserter, rw, treeId); + } catch (GitAPIException | IOException | InterruptedException e) { throw new ManifestErrorException(e); } } @@ -746,6 +725,51 @@ public RevCommit call() throws GitAPIException { } } + + private RevCommit commitTreeOnCurrentTip(ObjectInserter inserter, + RevWalk rw, ObjectId treeId) + throws IOException, ConcurrentRefUpdateException { + ObjectId headId = repo.resolve(targetBranch + "^{commit}"); //$NON-NLS-1$ + if (headId != null && rw.parseCommit(headId).getTree().getId().equals(treeId)) { + // No change. Do nothing. + return rw.parseCommit(headId); + } + + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(treeId); + if (headId != null) + commit.setParentIds(headId); + commit.setAuthor(author); + commit.setCommitter(author); + commit.setMessage(RepoText.get().repoCommitMessage); + + ObjectId commitId = inserter.insert(commit); + inserter.flush(); + + RefUpdate ru = repo.updateRef(targetBranch); + ru.setNewObjectId(commitId); + ru.setExpectedOldObjectId(headId != null ? headId : ObjectId.zeroId()); + Result rc = ru.update(rw); + switch (rc) { + case NEW: + case FORCED: + case FAST_FORWARD: + // Successful. Do nothing. + break; + case REJECTED: + case LOCK_FAILURE: + throw new ConcurrentRefUpdateException(MessageFormat + .format(JGitText.get().cannotLock, targetBranch), + ru.getRef(), rc); + default: + throw new JGitInternalException(MessageFormat.format( + JGitText.get().updatingRefFailed, + targetBranch, commitId.name(), rc)); + } + + return rw.parseCommit(commitId); + } + private void addSubmodule(String name, String url, String path, String revision, List copyfiles, List linkfiles, Git git) throws GitAPIException, IOException {