diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 15752d850b..4faa53ebae 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -21,6 +21,7 @@ import uk.ac.ic.wlgitbridge.bridge.swap.job.SwapJobImpl; import uk.ac.ic.wlgitbridge.bridge.swap.store.S3SwapStore; import uk.ac.ic.wlgitbridge.bridge.swap.store.SwapStore; import uk.ac.ic.wlgitbridge.data.CandidateSnapshot; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.data.ProjectLockImpl; import uk.ac.ic.wlgitbridge.data.filestore.GitDirectoryContents; import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory; @@ -308,6 +309,8 @@ public class Bridge { projName, new Timestamp(dotGit.lastModified()) ); + } catch (CannotAcquireLockException e) { + throw new RuntimeException(e); } } } @@ -325,7 +328,7 @@ public class Bridge { public ProjectRepo getUpdatedRepo( Optional oauth2, String projectName - ) throws IOException, GitUserException { + ) throws IOException, GitUserException, CannotAcquireLockException { try (LockGuard __ = lock.lockGuard(projectName)) { Optional maybeDoc = snapshotAPI.getDoc(oauth2, projectName); if (!maybeDoc.isPresent()) { @@ -367,7 +370,7 @@ public class Bridge { Optional oauth2, String projectName, GetDocResult doc - ) throws IOException, GitUserException { + ) throws IOException, GitUserException, CannotAcquireLockException { ProjectRepo repo; ProjectState state = dbStore.getProjectState(projectName); switch (state) { @@ -450,9 +453,10 @@ public class Bridge { RawDirectory directoryContents, RawDirectory oldDirectoryContents, String hostname - ) throws SnapshotPostException, IOException, MissingRepositoryException, ForbiddenException, GitUserException { + ) throws SnapshotPostException, IOException, MissingRepositoryException, ForbiddenException, GitUserException, CannotAcquireLockException { Log.debug("[{}] pushing to Overleaf", projectName); try (LockGuard __ = lock.lockGuard(projectName)) { + Log.info("[{}] got project lock", projectName); pushCritical( oauth2, projectName, diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImpl.java index 4d72d61e73..37b49432c1 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImpl.java @@ -4,6 +4,7 @@ import uk.ac.ic.wlgitbridge.bridge.lock.LockGuard; import uk.ac.ic.wlgitbridge.bridge.lock.ProjectLock; import uk.ac.ic.wlgitbridge.bridge.repo.ProjectRepo; import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.util.Log; import uk.ac.ic.wlgitbridge.util.TimerUtils; @@ -126,6 +127,8 @@ public class GcJobImpl implements GcJob { } catch (IOException e) { Log.warn("[{}] Failed to GC project", proj); } + } catch (CannotAcquireLockException e) { + Log.warn("[{}] Cannot acquire project lock, skipping GC", proj); } } Log.info("GC job finished, num gcs: {}", numGcs); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/lock/ProjectLock.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/lock/ProjectLock.java index 98f6ae96c2..f4617d258c 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/lock/ProjectLock.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/lock/ProjectLock.java @@ -1,5 +1,7 @@ package uk.ac.ic.wlgitbridge.bridge.lock; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; + /** * Project Lock class. * @@ -10,12 +12,12 @@ public interface ProjectLock { void lockAll(); - void lockForProject(String projectName); + void lockForProject(String projectName) throws CannotAcquireLockException; void unlockForProject(String projectName); /* RAII hahaha */ - default LockGuard lockGuard(String projectName) { + default LockGuard lockGuard(String projectName) throws CannotAcquireLockException { lockForProject(projectName); return () -> unlockForProject(projectName); } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index 42585a66c6..6b42cac521 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -6,6 +6,7 @@ import uk.ac.ic.wlgitbridge.bridge.lock.LockGuard; import uk.ac.ic.wlgitbridge.bridge.lock.ProjectLock; import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore; import uk.ac.ic.wlgitbridge.bridge.swap.store.SwapStore; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.util.Log; import uk.ac.ic.wlgitbridge.util.TimerUtils; @@ -209,6 +210,9 @@ public class SwapJobImpl implements SwapJob { dbStore.swap(projName, compression); repoStore.remove(projName); } + } catch (CannotAcquireLockException e) { + Log.warn("[{}] Cannot acquire project lock, skipping swap", projName); + return; } Log.info("Evicted project: {}", projName); } @@ -256,6 +260,8 @@ public class SwapJobImpl implements SwapJob { swapStore.remove(projName); dbStore.restore(projName); } + } catch (CannotAcquireLockException e) { + throw new RuntimeException(e); } } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CannotAcquireLockException.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CannotAcquireLockException.java new file mode 100644 index 0000000000..79206dd639 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CannotAcquireLockException.java @@ -0,0 +1,9 @@ +package uk.ac.ic.wlgitbridge.data; + +public class CannotAcquireLockException extends Exception { + String projectName; + + public CannotAcquireLockException() { + super("Another operation is in progress. Please try again later."); + } +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ProjectLockImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ProjectLockImpl.java index ee6729e916..d9fa8018ca 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ProjectLockImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ProjectLockImpl.java @@ -4,6 +4,7 @@ import uk.ac.ic.wlgitbridge.bridge.lock.ProjectLock; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -35,9 +36,17 @@ public class ProjectLockImpl implements ProjectLock { } @Override - public void lockForProject(String projectName) { + public void lockForProject(String projectName) throws CannotAcquireLockException { Log.debug("[{}] taking project lock", projectName); - getLockForProjectName(projectName).lock(); + Lock projectLock = getLockForProjectName(projectName); + try { + if (!projectLock.tryLock(5, TimeUnit.SECONDS)) { + Log.debug("[{}] failed to acquire project lock", projectName); + throw new CannotAcquireLockException(); + } + } catch (InterruptedException e) { + throw new RuntimeException(e); + } Log.debug("[{}] taking reentrant lock", projectName); rlock.lock(); Log.debug("[{}] taken locks", projectName); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolver.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolver.java index 8057445f90..fdf80806a4 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolver.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolver.java @@ -7,6 +7,7 @@ import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.resolver.RepositoryResolver; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import uk.ac.ic.wlgitbridge.bridge.Bridge; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.git.handler.hook.WriteLatexPutHook; import uk.ac.ic.wlgitbridge.git.servlet.WLGitServlet; @@ -101,6 +102,8 @@ public class WLRepositoryResolver } catch (ServiceMayNotContinueException e) { /* Such as FailedConnectionException */ throw e; + } catch (CannotAcquireLockException e) { + throw new ServiceMayNotContinueException(e.getMessage()); } catch (RuntimeException e) { Log.warn( "Runtime exception when trying to open repo: " + projName, diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java index 1d0562d17c..9d2a783fc5 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java @@ -8,6 +8,7 @@ import org.eclipse.jgit.transport.ReceiveCommand.Result; import org.eclipse.jgit.transport.ReceivePack; import uk.ac.ic.wlgitbridge.bridge.Bridge; import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory; import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.git.handler.WLReceivePackFactory; @@ -86,6 +87,13 @@ public class WriteLatexPutHook implements PreReceiveHook { } catch (GitUserException e) { Log.error("GitUserException on pre receive", e); handleSnapshotPostException(receivePack, receiveCommand, e); + } catch (CannotAcquireLockException e) { + Log.info("CannotAcquireLockException on pre receive"); + receivePack.sendError(e.getMessage()); + receiveCommand.setResult( + Result.REJECTED_OTHER_REASON, + e.getMessage() + ); } catch (Throwable t) { Log.error("Throwable on pre receive", t); handleSnapshotPostException( @@ -126,7 +134,7 @@ public class WriteLatexPutHook implements PreReceiveHook { Optional oauth2, Repository repository, ReceiveCommand receiveCommand - ) throws IOException, GitUserException { + ) throws IOException, GitUserException, CannotAcquireLockException { checkBranch(receiveCommand); checkForcedPush(receiveCommand); bridge.push( diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/BridgeTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/BridgeTest.java index 9026973d49..cb94bdf2fb 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/BridgeTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/BridgeTest.java @@ -13,6 +13,7 @@ import uk.ac.ic.wlgitbridge.bridge.resource.ResourceCache; import uk.ac.ic.wlgitbridge.bridge.snapshot.SnapshotApiFacade; import uk.ac.ic.wlgitbridge.bridge.swap.job.SwapJob; import uk.ac.ic.wlgitbridge.bridge.swap.store.SwapStore; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.snapshot.getdoc.GetDocResult; @@ -88,7 +89,7 @@ public class BridgeTest { @Test public void updatingRepositorySetsLastAccessedTime( - ) throws IOException, GitUserException { + ) throws IOException, GitUserException, CannotAcquireLockException { ProjectRepo repo = mock(ProjectRepo.class); when(repoStore.getExistingRepo("asdf")).thenReturn(repo); when(dbStore.getProjectState("asdf")).thenReturn(ProjectState.PRESENT); diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImplTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImplTest.java index 091e6b5692..4db48f91d0 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImplTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/gc/GcJobImplTest.java @@ -8,6 +8,7 @@ import uk.ac.ic.wlgitbridge.bridge.lock.LockGuard; import uk.ac.ic.wlgitbridge.bridge.lock.ProjectLock; import uk.ac.ic.wlgitbridge.bridge.repo.ProjectRepo; import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore; +import uk.ac.ic.wlgitbridge.data.CannotAcquireLockException; import uk.ac.ic.wlgitbridge.data.ProjectLockImpl; import java.io.IOException; @@ -112,6 +113,8 @@ public class GcJobImplTest { assertFalse(fut.isDone()); Thread.sleep(1); } + } catch (CannotAcquireLockException e) { + throw new RuntimeException(e); } /* Now that we've released the lock, fut should complete */ fut.join();