RefDirectory: Retry acquiring ref locks with backoff

If a repo frequently uses PackedBatchRefUpdates, there is likely to be
contention on the packed-refs file, so it's not appropriate to fail
immediately the first time we fail to acquire a lock. Add some logic to
RefDirectory to support general retrying of lock acquisition.

Currently, there is a hard-coded wait starting at 100ms and backing off
exponentially to 1600ms, for about 3s of total wait. This is no worse
than the hard-coded backoff that JGit does elsewhere, e.g. in
FileUtils#delete. One can imagine a scheme that uses per-repository
configuration of backoff, and the current interface would support this
without changing any callers.

Change-Id: I4764e11270d9336882483eb698f67a78a401c251

2 files changed, 106 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
index 5a40907229..ead9a5dad2 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
@@ -54,6 +54,7 @@ import static org.eclipse.jgit.transport.ReceiveCommand.Type.DELETE;
 import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE;
 import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE_NONFASTFORWARD;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -96,6 +97,7 @@ import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+@SuppressWarnings("boxing")
 @RunWith(Parameterized.class)
 public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase {
 	@Parameter
@@ -125,6 +127,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase {
 		cfg.save();
 
 		refdir = (RefDirectory) diskRepo.getRefDatabase();
+		refdir.setRetrySleepMs(Arrays.asList(0, 0));
 
 		repo = new TestRepository<>(diskRepo);
 		A = repo.commit().create();
@@ -584,6 +587,85 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase {
 				getLastReflog("refs/heads/branch"));
 	}
 
+	@Test
+	public void packedRefsLockFailure() throws Exception {
+		writeLooseRef("refs/heads/master", A);
+
+		List<ReceiveCommand> cmds = Arrays.asList(
+				new ReceiveCommand(A, B, "refs/heads/master", UPDATE),
+				new ReceiveCommand(zeroId(), B, "refs/heads/branch", CREATE));
+
+		LockFile myLock = refdir.lockPackedRefs();
+		try {
+			execute(newBatchUpdate(cmds).setAllowNonFastForwards(true));
+
+			assertFalse(getLockFile("refs/heads/master").exists());
+			assertFalse(getLockFile("refs/heads/branch").exists());
+
+			if (atomic) {
+				assertResults(cmds, LOCK_FAILURE, TRANSACTION_ABORTED);
+				assertRefs("refs/heads/master", A);
+			} else {
+				// Only operates on loose refs, doesn't care that packed-refs is locked.
+				assertResults(cmds, OK, OK);
+				assertRefs(
+						"refs/heads/master", B,
+						"refs/heads/branch", B);
+			}
+		} finally {
+			myLock.unlock();
+		}
+	}
+
+	@Test
+	public void oneRefLockFailure() throws Exception {
+		writeLooseRef("refs/heads/master", A);
+
+		List<ReceiveCommand> cmds = Arrays.asList(
+				new ReceiveCommand(zeroId(), B, "refs/heads/branch", CREATE),
+				new ReceiveCommand(A, B, "refs/heads/master", UPDATE));
+
+		LockFile myLock = new LockFile(refdir.fileFor("refs/heads/master"));
+		assertTrue(myLock.lock());
+		try {
+			execute(newBatchUpdate(cmds).setAllowNonFastForwards(true));
+
+			assertFalse(LockFile.getLockFile(refdir.packedRefsFile).exists());
+			assertFalse(getLockFile("refs/heads/branch").exists());
+
+			if (atomic) {
+				assertResults(cmds, TRANSACTION_ABORTED, LOCK_FAILURE);
+				assertRefs("refs/heads/master", A);
+			} else {
+				assertResults(cmds, OK, LOCK_FAILURE);
+				assertRefs(
+						"refs/heads/branch", B,
+						"refs/heads/master", A);
+			}
+		} finally {
+			myLock.unlock();
+		}
+	}
+
+	@Test
+	public void singleRefUpdateDoesNotRequirePackedRefsLock() throws Exception {
+		writeLooseRef("refs/heads/master", A);
+
+		List<ReceiveCommand> cmds = Arrays.asList(
+				new ReceiveCommand(A, B, "refs/heads/master", UPDATE));
+
+		LockFile myLock = refdir.lockPackedRefs();
+		try {
+			execute(newBatchUpdate(cmds).setAllowNonFastForwards(true));
+
+			assertFalse(getLockFile("refs/heads/master").exists());
+			assertResults(cmds, OK);
+			assertRefs("refs/heads/master", B);
+		} finally {
+			myLock.unlock();
+		}
+	}
+
 	private void writeLooseRef(String name, AnyObjectId id) throws IOException {
 		write(new File(diskRepo.getDirectory(), name), id.name() + "\n");
 	}
@@ -712,6 +794,10 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase {
 		return r.getLastEntry();
 	}
 
+	private File getLockFile(String refName) {
+		return LockFile.getLockFile(refdir.fileFor(refName));
+	}
+
 	private void assertReflogUnchanged(
 			Map<String, ReflogEntry> old, String name) throws IOException {
 		assertReflogEquals(old.get(name), getLastReflog(name), true);
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
index 145fed6b1b..fefccf314f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
@@ -60,10 +60,12 @@ import static org.junit.Assert.fail;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.eclipse.jgit.errors.LockFailedException;
 import org.eclipse.jgit.events.ListenerHandle;
 import org.eclipse.jgit.events.RefsChangedEvent;
 import org.eclipse.jgit.events.RefsChangedListener;
@@ -79,6 +81,7 @@ import org.eclipse.jgit.revwalk.RevTag;
 import org.junit.Before;
 import org.junit.Test;
 
+@SuppressWarnings("boxing")
 public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 	private Repository diskRepo;
 
@@ -1284,6 +1287,23 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		assertEquals(1, changeCount.get());
 	}
 
+	@Test
+	public void testPackedRefsLockFailure() throws Exception {
+		writeLooseRef("refs/heads/master", A);
+		refdir.setRetrySleepMs(Arrays.asList(0, 0));
+		LockFile myLock = refdir.lockPackedRefs();
+		try {
+			refdir.pack(Arrays.asList("refs/heads/master"));
+			fail("expected LockFailedException");
+		} catch (LockFailedException e) {
+			assertEquals(refdir.packedRefsFile.getPath(), e.getFile().getPath());
+		} finally {
+			myLock.unlock();
+		}
+		Ref ref = refdir.getRef("refs/heads/master");
+		assertEquals(Storage.LOOSE, ref.getStorage());
+	}
+
 	private void writeLooseRef(String name, AnyObjectId id) throws IOException {
 		writeLooseRef(name, id.name() + "\n");
 	}