[Named min timestamp leases] Post merge review #7385

palantir · Oct 31, 2024 · 4826b68 · 4826b68
1 parent 9183a9d
commit 4826b68
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 8 deletions.
diff --git a/...c/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java b/...c/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java
@@ -81,7 +81,7 @@ private AsyncTimelockService getServiceForNamespace(Namespace namespace, @Nullab
     }
 
     private static ListenableFuture<TimestampLeaseResponses> acquireTimestampLease(
-            AsyncTimelockService service, RequestId requestsId, Map<TimestampLeaseName, Integer> numFreshTimestamps) {
-        return service.acquireTimestampLease(requestsId.get(), numFreshTimestamps);
+            AsyncTimelockService service, RequestId requestId, Map<TimestampLeaseName, Integer> numFreshTimestamps) {
+        return service.acquireTimestampLease(requestId.get(), numFreshTimestamps);
     }
 }
diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/lock/AsyncLockService.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/lock/AsyncLockService.java
@@ -183,6 +183,8 @@ private AsyncResult<HeldLocks> acquireNamedTimestampLocksInternal(
         List<AsyncLock> locks = timestampNames.stream()
                 .map(name -> lockManager.getNamedTimestampLock(name, timestamp))
                 .collect(Collectors.toList());
+        // for these locks, each descriptor has a unique timestamp appended to it which means
+        // the individual locks do not contend with another thus eliminating the risk of deadlock
         return lockAcquirer.acquireLocks(requestId, OrderedLocks.fromOrderedList(locks), TimeLimit.zero());
     }
 

diff --git a/timelock-impl/src/test/java/com/palantir/atlasdb/timelock/AsyncTimelockServiceImplTest.java b/timelock-impl/src/test/java/com/palantir/atlasdb/timelock/AsyncTimelockServiceImplTest.java
@@ -71,7 +71,8 @@ public void unlockingWithRequestIdFromAcquireTimestampLeaseResponseUnlocksForAll
 
         TimestampedInvocation<TimestampLeaseResponses> responses =
                 acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 10, TIMESTAMP_NAME_2, 20);
-        assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(responses.result), responses);
+        assertThatTimestampsAreStrictlyWithinInvocationInterval(
+                getMinLeasedTimestampsFrom(responses.result), responses);
 
         unlockForResponse(responses.result);
         assertThatTimestampIsStrictlyWithinInvocationInterval(timestamp1MinLeased1.result, timestamp1MinLeased1);
@@ -90,21 +91,25 @@ public void acquireTimestampLeaseReturnsLeaseGuaranteeIdentifierWithGivenRequest
     public void acquireTimestampLeaseReturnsMinLeasedAllThroughout() {
         TimestampedInvocation<TimestampLeaseResponses> response1 =
                 acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 15);
-        assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response1.result), response1);
+        assertThatTimestampsAreStrictlyWithinInvocationInterval(
+                getMinLeasedTimestampsFrom(response1.result), response1);
 
         TimestampedInvocation<TimestampLeaseResponses> response2 =
                 acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 5);
-        assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response2.result), response1);
+        assertThatTimestampsAreStrictlyWithinInvocationInterval(
+                getMinLeasedTimestampsFrom(response2.result), response1);
 
         TimestampedInvocation<TimestampLeaseResponses> response3 =
                 acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 7);
-        assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response3.result), response1);
+        assertThatTimestampsAreStrictlyWithinInvocationInterval(
+                getMinLeasedTimestampsFrom(response3.result), response1);
 
         unlockForResponse(response1.result);
         unlockForResponse(response3.result);
         TimestampedInvocation<TimestampLeaseResponses> response4 =
                 acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 19);
-        assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response4.result), response2);
+        assertThatTimestampsAreStrictlyWithinInvocationInterval(
+                getMinLeasedTimestampsFrom(response4.result), response2);
     }
 
     @Test
@@ -241,7 +246,7 @@ private static List<Long> getMinLeasedTimestampsFrom(TimestampLeaseResponses res
                 .collect(Collectors.toList());
     }
 
-    private static void assertThatTimestampsIsStrictlyWithinInvocationInterval(
+    private static void assertThatTimestampsAreStrictlyWithinInvocationInterval(
             List<Long> timestamps, TimestampedInvocation<?> invocation) {
         assertThat(timestamps)
                 .allSatisfy(timestamp -> assertThatTimestampIsStrictlyWithinInvocationInterval(timestamp, invocation));