Merge pull request #35948 from overleaf/dk-git-bridge-health-check-race

[git-bridge] Fix health check race on shared probe file

GitOrigin-RevId: 1463880cb4770a730e3e164bf9a5ff89366434b5

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

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<Future<Boolean>> results = new ArrayList<>();
+      for (int i = 0; i < 200; i++) {
+        results.add(executor.submit(bridge::healthCheck));
+      }
+      for (Future<Boolean> result : results) {
+        assertTrue(result.get(30, TimeUnit.SECONDS));
+      }
+    } finally {
+      executor.shutdownNow();
+    }
+  }
+
   @Test
   public void updatingRepositorySetsLastAccessedTime()
       throws IOException, GitUserException, CannotAcquireLockException {