force entry order

wellcomecollection · Jan 15, 2025 · ef0d512 · ef0d512
1 parent 1d7694b
commit ef0d512
Showing 1 changed file with 34 additions and 13 deletions.
diff --git a/...matcher_merger/matcher/src/test/scala/weco/pipeline/matcher/matcher/WorkMatcherTest.scala b/...matcher_merger/matcher/src/test/scala/weco/pipeline/matcher/matcher/WorkMatcherTest.scala
@@ -404,29 +404,43 @@ class WorkMatcherTest
                         _ == SubgraphId(idA, idB)
                       ) == 0
                     ) {
+                      // Lock before waiting for C, so that when the locks for C come in
+                      // at roughly the same time, the lock is already in place.
+                      // If we do not lock here, then the wait for A to unlock may
+                      // not take place.
+                      val lock = super.lock(id, contextId)
+                      createdLocksHistory = createdLocksHistory :+ id
+
                       eventually(timeout = timeout(5 seconds)) {
                         if (!cHasReadGraph) throw new Exception("not yet")
                       }
+                      lock
                     }
 
                     // (**) We don't let the update to 'C' start writing graph updates until
                     // we know the update to 'A' is finished
-                    if (
-                      (id == SubgraphId(idA, idB) || id == SubgraphId(
-                        idA,
-                        idB,
-                        idC
-                      )) &&
-                      createdLocksHistory.count(_ == SubgraphId(idA, idB)) == 1
-                    ) {
-                      eventually(timeout = timeout(5 seconds)) {
-                        if (locks.contains(idA.underlying))
-                          throw new Exception("not yet")
+                    else {
+                      if (
+                        (id == SubgraphId(idA, idB) || id == SubgraphId(
+                          idA,
+                          idB,
+                          idC
+                        )) &&
+                        createdLocksHistory.count(
+                          _ == SubgraphId(idA, idB)
+                        ) == 1
+                      ) {
+                        eventually(timeout = timeout(5 seconds)) {
+                          if (locks.contains(idA.underlying))
+                            throw new Exception("not yet")
+                        }
                       }
+                      createdLocksHistory = createdLocksHistory :+ id
+                      super.lock(id, contextId)
                     }
-                    createdLocksHistory = createdLocksHistory :+ id
+                  } else {
+                    super.lock(id, contextId)
                   }
-                  super.lock(id, contextId)
                 }
               }
 
@@ -458,6 +472,13 @@ class WorkMatcherTest
             //
             // We need the locking to ensure we don't try to apply both updates at once.
             val futureA = workMatcher.matchWork(workA)
+            // Sleep briefly to ensure that A is first through the gate.
+            // This is just to ensure that when we execute the artificial waits in the mock
+            // lock DAO above, A waits C to start, then C waits for A to finish.
+            // If we do not do this, then it is nondeterministic as to which one locks first
+            // if C goes through first, then it will not wait for anything and the test
+            // will fail without proving anything useful.
+            Thread.sleep(100)
             val futureC = workMatcher.matchWork(workC)
 
             val resultA = Try {