From 4826b682aea76dbf9f8e86376faaa9bdd9ae3b2c Mon Sep 17 00:00:00 2001 From: Ahmed Alouane Date: Thu, 31 Oct 2024 17:19:49 +0000 Subject: [PATCH] [Named min timestamp leases] Post merge review #7385 --- .../RemotingTimestampLeaseServiceAdapter.java | 4 ++-- .../atlasdb/timelock/lock/AsyncLockService.java | 2 ++ .../timelock/AsyncTimelockServiceImplTest.java | 17 +++++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java index 779878f487..06ed0bd411 100644 --- a/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java +++ b/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/batch/RemotingTimestampLeaseServiceAdapter.java @@ -81,7 +81,7 @@ private AsyncTimelockService getServiceForNamespace(Namespace namespace, @Nullab } private static ListenableFuture acquireTimestampLease( - AsyncTimelockService service, RequestId requestsId, Map numFreshTimestamps) { - return service.acquireTimestampLease(requestsId.get(), numFreshTimestamps); + AsyncTimelockService service, RequestId requestId, Map 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 index 36906c1604..73754f6958 100644 --- 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 acquireNamedTimestampLocksInternal( List 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 index 7dcf9e9673..298fafe3d9 100644 --- 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 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 response1 = acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 15); - assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response1.result), response1); + assertThatTimestampsAreStrictlyWithinInvocationInterval( + getMinLeasedTimestampsFrom(response1.result), response1); TimestampedInvocation response2 = acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 5); - assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response2.result), response1); + assertThatTimestampsAreStrictlyWithinInvocationInterval( + getMinLeasedTimestampsFrom(response2.result), response1); TimestampedInvocation response3 = acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 7); - assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response3.result), response1); + assertThatTimestampsAreStrictlyWithinInvocationInterval( + getMinLeasedTimestampsFrom(response3.result), response1); unlockForResponse(response1.result); unlockForResponse(response3.result); TimestampedInvocation response4 = acquireTimestampLeaseTimestamped(TIMESTAMP_NAME_1, 19); - assertThatTimestampsIsStrictlyWithinInvocationInterval(getMinLeasedTimestampsFrom(response4.result), response2); + assertThatTimestampsAreStrictlyWithinInvocationInterval( + getMinLeasedTimestampsFrom(response4.result), response2); } @Test @@ -241,7 +246,7 @@ private static List getMinLeasedTimestampsFrom(TimestampLeaseResponses res .collect(Collectors.toList()); } - private static void assertThatTimestampsIsStrictlyWithinInvocationInterval( + private static void assertThatTimestampsAreStrictlyWithinInvocationInterval( List timestamps, TimestampedInvocation invocation) { assertThat(timestamps) .allSatisfy(timestamp -> assertThatTimestampIsStrictlyWithinInvocationInterval(timestamp, invocation));