diff options
5 files changed, 18 insertions, 18 deletions
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java index 1db34538..0652897c 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java @@ -132,7 +132,7 @@ public class TargetLock { try (Connection conn = dataSource.getConnection(); PreparedStatement updateStatement = conn.prepareStatement( - "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = timestampadd(second, ?, now()) WHERE resourceId = ? AND (owner = ? OR expirationTime < now())"); + "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = timestampadd(second, ?, now()) WHERE resourceId = ? AND expirationTime < now()"); PreparedStatement insertStatement = conn.prepareStatement( "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, timestampadd(second, ?, now()))");) { @@ -142,7 +142,6 @@ public class TargetLock { updateStatement.setString(i++, this.owner); updateStatement.setInt(i++, holdSec); updateStatement.setString(i++, this.resourceId); - updateStatement.setString(i++, this.owner); // The lock was expired and we grabbed it. // return true diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java index 6e33f224..42c8a742 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java @@ -92,8 +92,10 @@ public class TargetLockTest { assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC)); - // extend the lock - assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC)); + // cannot re-lock + assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC)); + + assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1")); } @Test @@ -131,6 +133,8 @@ public class TargetLockTest { // refresh should work now assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC)); + assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1")); + // expire the lock try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = timestampadd(second, -1, now()) WHERE resourceId = ?");) { @@ -144,6 +148,8 @@ public class TargetLockTest { // refresh should fail now assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC)); + + assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1")); } diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java index ba6fe856..5aee490f 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockFeatureAPI.java @@ -71,8 +71,7 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService { } /** - * This method is called before a lock is acquired on a resource. It may be - * invoked repeatedly to extend the time that a lock is held. + * This method is called before a lock is acquired on a resource. * * @param resourceId * @param owner diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java index 081ad4c1..243ba3cb 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/SimpleLockManager.java @@ -74,7 +74,9 @@ public class SimpleLockManager { } /** - * Attempts to lock a resource, extending the lock if the owner already holds it. + * Attempts to lock a resource, rejecting the lock if it is already owned, even if + * it's the same owner; the same owner can use {@link #refresh(String, String, int) + * refresh()}, instead, to extend a lock on a resource. * * @param resourceId * @param owner @@ -93,25 +95,19 @@ public class SimpleLockManager { throw makeNullArgException(MSG_NULL_OWNER); } - Data existingLock; + boolean locked = false; synchronized(locker) { cleanUpLocks(); - if ((existingLock = resource2data.get(resourceId)) != null && existingLock.getOwner().equals(owner)) { - // replace the existing lock with an extended lock - locks.remove(existingLock); - existingLock = null; - } - - if (existingLock == null) { + if(!resource2data.containsKey(resourceId)) { Data data = new Data(owner, resourceId, currentTime.getMillis() + TimeUnit.SECONDS.toMillis(holdSec)); resource2data.put(resourceId, data); locks.add(data); + locked = true; } } - boolean locked = (existingLock == null); logger.info("lock {} for resource {} owner {}", locked, resourceId, owner); return locked; diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java index df6fb10a..feabdbc0 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/lock/SimpleLockManagerTest.java @@ -116,7 +116,7 @@ public class SimpleLockManagerTest { assertTrue(mgr.isLockedBy(RESOURCE_A, OWNER1)); // extend the lock - mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); + mgr.refresh(RESOURCE_A, OWNER1, MAX_AGE_SEC); // verify still locked after sleeping the other half of the cycle testTime.sleep(MAX_AGE_MS/2+1); @@ -132,7 +132,7 @@ public class SimpleLockManagerTest { mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC); // same owner - assertTrue(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); + assertFalse(mgr.lock(RESOURCE_A, OWNER1, MAX_AGE_SEC)); // different owner assertFalse(mgr.lock(RESOURCE_A, OWNER2, MAX_AGE_SEC)); |