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 9cea2258bd..d5e45a2ce5 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 @@ -244,7 +244,9 @@ public class Bridge { gcJob.start(); } - public boolean healthCheck() { + // synchronized so concurrent health checks (e.g. liveness and readiness + // probes) don't race on the shared probe file and observe partial writes + public synchronized boolean healthCheck() { try { dbStore.getNumProjects(); // Check the repo store volume, not the container root filesystem, so a 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 d63a0a7b01..4fb517c50f 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 @@ -1,13 +1,21 @@ package uk.ac.ic.wlgitbridge.bridge; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.*; import java.io.File; import java.io.IOException; import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -100,6 +108,39 @@ public class BridgeTest { assertFalse(bridge.healthCheck()); } + @Test + public void healthCheckLeavesOnlyTheFixedProbeFile() throws IOException { + File rootDirectory = tempFolder.newFolder("repostore"); + File wlgbDirectory = new File(rootDirectory, ".wlgb"); + wlgbDirectory.mkdir(); + when(repoStore.getRootDirectory()).thenReturn(rootDirectory); + assertTrue(bridge.healthCheck()); + assertTrue(bridge.healthCheck()); + String[] probeFiles = wlgbDirectory.list(); + assertNotNull(probeFiles); + assertEquals(1, probeFiles.length); + assertEquals(".health_check", probeFiles[0]); + } + + @Test + public void concurrentHealthChecksAllPass() throws Exception { + File rootDirectory = tempFolder.newFolder("repostore"); + new File(rootDirectory, ".wlgb").mkdir(); + when(repoStore.getRootDirectory()).thenReturn(rootDirectory); + ExecutorService executor = Executors.newFixedThreadPool(8); + try { + List> results = new ArrayList<>(); + for (int i = 0; i < 200; i++) { + results.add(executor.submit(bridge::healthCheck)); + } + for (Future result : results) { + assertTrue(result.get(30, TimeUnit.SECONDS)); + } + } finally { + executor.shutdownNow(); + } + } + @Test public void updatingRepositorySetsLastAccessedTime() throws IOException, GitUserException, CannotAcquireLockException {