From 9e14e86880450b19698280ba7dbf0a508ebe25e0 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 26 Jun 2018 20:46:46 -0400 Subject: [PATCH] Fix DynamoDbLockRegistry.tryLock for fair time When we try to lock the DynamoDB item we need to iterate at least the lease time, but we still don't need to sleep too much - maximum the time requested for `tryLock()` --- .../aws/lock/DynamoDbLockRegistry.java | 5 +-- ...amoDbLockRegistryLeaderInitiatorTests.java | 2 ++ .../aws/lock/DynamoDbLockRegistryTests.java | 31 ++++++++++++++++--- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/integration/aws/lock/DynamoDbLockRegistry.java b/src/main/java/org/springframework/integration/aws/lock/DynamoDbLockRegistry.java index 009ebde..0892e64 100644 --- a/src/main/java/org/springframework/integration/aws/lock/DynamoDbLockRegistry.java +++ b/src/main/java/org/springframework/integration/aws/lock/DynamoDbLockRegistry.java @@ -445,9 +445,10 @@ public class DynamoDbLockRegistry implements ExpirableLockRegistry, Initializing return false; } + long timeToWaitForLock = System.currentTimeMillis() - start + TimeUnit.MILLISECONDS.convert(time, unit); this.acquireLockOptionsBuilder - .withAdditionalTimeToWaitForLock( - System.currentTimeMillis() - start + TimeUnit.MILLISECONDS.convert(time, unit)); + .withAdditionalTimeToWaitForLock(timeToWaitForLock) + .withRefreshPeriod(timeToWaitForLock); boolean acquired = false; try { diff --git a/src/test/java/org/springframework/integration/aws/leader/DynamoDbLockRegistryLeaderInitiatorTests.java b/src/test/java/org/springframework/integration/aws/leader/DynamoDbLockRegistryLeaderInitiatorTests.java index dd6b5ed..eca2520 100644 --- a/src/test/java/org/springframework/integration/aws/leader/DynamoDbLockRegistryLeaderInitiatorTests.java +++ b/src/test/java/org/springframework/integration/aws/leader/DynamoDbLockRegistryLeaderInitiatorTests.java @@ -169,6 +169,8 @@ public class DynamoDbLockRegistryLeaderInitiatorTests { assertThat(revoked11.await(10, TimeUnit.SECONDS)).isTrue(); assertThat(initiator1.getContext().isLeader()).isFalse(); + initiator1.stop(); + for (DynamoDbLockRegistry registry : registries) { registry.destroy(); } diff --git a/src/test/java/org/springframework/integration/aws/lock/DynamoDbLockRegistryTests.java b/src/test/java/org/springframework/integration/aws/lock/DynamoDbLockRegistryTests.java index f0ff009..e07c6a1 100644 --- a/src/test/java/org/springframework/integration/aws/lock/DynamoDbLockRegistryTests.java +++ b/src/test/java/org/springframework/integration/aws/lock/DynamoDbLockRegistryTests.java @@ -179,8 +179,13 @@ public class DynamoDbLockRegistryTests { final AtomicBoolean locked = new AtomicBoolean(); final CountDownLatch latch = new CountDownLatch(1); Future result = this.taskExecutor.submit(() -> { - Lock lock2 = this.dynamoDbLockRegistry.obtain("foo"); - locked.set(lock2.tryLock(200, TimeUnit.MILLISECONDS)); + DynamoDbLockRegistry registry2 = new DynamoDbLockRegistry(DYNAMO_DB_RUNNING.getDynamoDB()); + registry2.setHeartbeatPeriod(1); + registry2.setRefreshPeriod(10); + registry2.setLeaseDuration(2); + registry2.afterPropertiesSet(); + Lock lock2 = registry2.obtain("foo"); + locked.set(lock2.tryLock()); latch.countDown(); try { lock2.unlock(); @@ -188,6 +193,9 @@ public class DynamoDbLockRegistryTests { catch (Exception e) { return e; } + finally { + registry2.destroy(); + } return null; }); assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue(); @@ -206,8 +214,13 @@ public class DynamoDbLockRegistryTests { final CountDownLatch latch2 = new CountDownLatch(1); final CountDownLatch latch3 = new CountDownLatch(1); lock1.lockInterruptibly(); - this.taskExecutor.execute(() -> { - Lock lock2 = this.dynamoDbLockRegistry.obtain("foo"); + this.taskExecutor.submit(() -> { + DynamoDbLockRegistry registry2 = new DynamoDbLockRegistry(DYNAMO_DB_RUNNING.getDynamoDB()); + registry2.setHeartbeatPeriod(1); + registry2.setRefreshPeriod(10); + registry2.setLeaseDuration(2); + registry2.afterPropertiesSet(); + Lock lock2 = registry2.obtain("foo"); try { latch1.countDown(); lock2.lockInterruptibly(); @@ -220,7 +233,9 @@ public class DynamoDbLockRegistryTests { finally { lock2.unlock(); latch3.countDown(); + registry2.destroy(); } + return null; }); assertThat(latch1.await(10, TimeUnit.SECONDS)).isTrue(); @@ -236,8 +251,15 @@ public class DynamoDbLockRegistryTests { @Test public void testTwoThreadsDifferentRegistries() throws Exception { final DynamoDbLockRegistry registry1 = new DynamoDbLockRegistry(DYNAMO_DB_RUNNING.getDynamoDB()); + registry1.setHeartbeatPeriod(1); + registry1.setRefreshPeriod(10); + registry1.setLeaseDuration(2); registry1.afterPropertiesSet(); + final DynamoDbLockRegistry registry2 = new DynamoDbLockRegistry(DYNAMO_DB_RUNNING.getDynamoDB()); + registry2.setHeartbeatPeriod(1); + registry2.setRefreshPeriod(10); + registry2.setLeaseDuration(2); registry2.afterPropertiesSet(); final Lock lock1 = registry1.obtain("foo"); @@ -311,6 +333,7 @@ public class DynamoDbLockRegistryTests { DynamoDbLockRegistry dynamoDbLockRegistry = new DynamoDbLockRegistry(DYNAMO_DB_RUNNING.getDynamoDB()); dynamoDbLockRegistry.setHeartbeatPeriod(1); dynamoDbLockRegistry.setRefreshPeriod(10); + dynamoDbLockRegistry.setLeaseDuration(2); return dynamoDbLockRegistry; }