From ee2313141a40222a83d6bf39a15c3f9d4c3a239e Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 27 Jun 2018 10:12:45 -0400 Subject: Add time limit to locking facility Modified the locking facility to add a time limit and remove the callback parameter. This affected both the default facility as well as the distributed locking feature. It will also require a change to the rules for Closed Loop. Changed testUnlock() to try locking with a different owner. Default feature API should be OPER_UNHANDLED. Put a few things back so-as not to break the drools-applications build. They can be removed once drools-applications is updated. Fix newlines in API java. Change-Id: I3ed7835cac6a582493a9bc8f6d1d4f3e6cb6289e Issue-ID: POLICY-872 Signed-off-by: Jim Hahn --- .../org/onap/policy/drools/core/lock/Lock.java | 2 +- .../policy/drools/core/lock/LockRequestFuture.java | 261 --------------------- .../core/lock/PolicyResourceLockFeatureAPI.java | 37 +-- .../core/lock/PolicyResourceLockManager.java | 33 ++- .../policy/drools/core/lock/SimpleLockManager.java | 65 +++-- 5 files changed, 58 insertions(+), 340 deletions(-) delete mode 100644 policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java (limited to 'policy-core/src/main') diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java index ea5e2521..d3764910 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/Lock.java @@ -102,7 +102,7 @@ public class Lock { */ public boolean add(String requester, T item) { if (item == null) { - throw LockRequestFuture.makeNullArgException("lock requester item is null"); + throw SimpleLockManager.makeNullArgException("lock requester item is null"); } if (requester.equals(owner)) { diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java deleted file mode 100644 index a2e9e62e..00000000 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/LockRequestFuture.java +++ /dev/null @@ -1,261 +0,0 @@ -/* - * ============LICENSE_START======================================================= - * ONAP - * ================================================================================ - * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. - * ================================================================================ - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * ============LICENSE_END========================================================= - */ - -package org.onap.policy.drools.core.lock; - -import java.util.concurrent.CancellationException; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicReference; -import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Future associated with a lock request. - */ -public class LockRequestFuture implements Future { - - // messages used in exceptions - public static final String MSG_NULL_RESOURCE_ID = "null resourceId"; - public static final String MSG_NULL_OWNER = "null owner"; - - private static Logger logger = LoggerFactory.getLogger(LockRequestFuture.class); - - /** - * The resource on which the lock was requested. - */ - private final String resourceId; - - /** - * The owner for which the lock was requested. - */ - private final String owner; - - /** - * Possible states for this future. - */ - private enum State { - WAITING, CANCELLED, ACQUIRED, DENIED - }; - - private AtomicReference state; - - /** - * Used to wait for the lock request to complete. - */ - private CountDownLatch waiter = new CountDownLatch(1); - - /** - * Callback to invoke once the lock is acquired (or denied). This is set to - * {@code null} once the callback has been invoked. - */ - private final AtomicReference callback; - - /** - * Constructs a future that has already been completed. - * - * @param resourceId - * @param owner owner for which the lock was requested - * @param locked {@code true} if the lock has been acquired, {@code false} if the lock - * request has been denied - * @throws IllegalArgumentException if any of the arguments are {@code null} - */ - public LockRequestFuture(String resourceId, String owner, boolean locked) { - if (resourceId == null) { - throw makeNullArgException(MSG_NULL_RESOURCE_ID); - } - - if (owner == null) { - throw makeNullArgException(MSG_NULL_OWNER); - } - - this.resourceId = resourceId; - this.owner = owner; - this.callback = new AtomicReference<>(null); - this.state = new AtomicReference<>(locked ? State.ACQUIRED : State.DENIED); - - // indicate that it's already done - this.waiter.countDown(); - } - - /** - * Constructs a future that has not yet been completed. - * - * @param resourceId - * @param owner owner for which the lock was requested - * @param callback item to be wrapped - * @throws IllegalArgumentException if the resourceId or owner is {@code null} - */ - public LockRequestFuture(String resourceId, String owner, Callback callback) { - if (resourceId == null) { - throw makeNullArgException(MSG_NULL_RESOURCE_ID); - } - - if (owner == null) { - throw makeNullArgException(MSG_NULL_OWNER); - } - - this.resourceId = resourceId; - this.owner = owner; - this.callback = new AtomicReference<>(callback); - this.state = new AtomicReference<>(State.WAITING); - } - - public String getResourceId() { - return resourceId; - } - - public String getOwner() { - return owner; - } - - @Override - public boolean cancel(boolean mayInterruptIfRunning) { - boolean cancelled = state.compareAndSet(State.WAITING, State.CANCELLED); - - if (cancelled) { - logger.info("resource {} owner {} cancelled lock request", resourceId, owner); - waiter.countDown(); - } - - return cancelled; - } - - /** - * Indicates that the lock has been acquired or denied. - * - * @param locked {@code true} if the lock has been acquired, {@code false} if the lock - * request has been denied - * - * @return {@code true} if it was not already completed, {@code false} otherwise - */ - protected boolean setLocked(boolean locked) { - State newState = (locked ? State.ACQUIRED : State.DENIED); - if (state.compareAndSet(State.WAITING, newState)) { - waiter.countDown(); - return true; - - } else { - return false; - } - } - - @Override - public boolean isCancelled() { - return (state.get() == State.CANCELLED); - } - - @Override - public boolean isDone() { - return (state.get() != State.WAITING); - } - - /** - * Gets the current status of the lock. - * - * @return {@code true} if the lock has been acquired, {@code false} otherwise - */ - public boolean isLocked() { - return (state.get() == State.ACQUIRED); - } - - /** - * @return {@code true} if the lock was acquired, {@code false} if it was denied - */ - @Override - public Boolean get() throws InterruptedException { - waiter.await(); - - switch (state.get()) { - case CANCELLED: - throw new CancellationException("lock request was cancelled"); - case ACQUIRED: - return true; - default: - // should only be DENIED at this point - return false; - } - } - - /** - * @return {@code true} if the lock was acquired, {@code false} if it was denied - */ - @Override - public Boolean get(long timeout, TimeUnit unit) - throws InterruptedException, TimeoutException { - - if (!waiter.await(timeout, unit)) { - throw new TimeoutException("lock request did not complete in time"); - } - - return get(); - } - - /** - * Invokes the callback, indicating whether or not the lock was acquired. - * - * @throws IllegalStateException if the request was previously cancelled, has not yet - * completed, or if the callback has already been invoked - */ - protected void invokeCallback() { - boolean locked; - - switch (state.get()) { - case ACQUIRED: - locked = true; - break; - case DENIED: - locked = false; - break; - case CANCELLED: - throw new IllegalStateException("cancelled lock request callback"); - default: - // only other choice is WAITING - throw new IllegalStateException("incomplete lock request callback"); - } - - Callback cb = callback.get(); - if (cb == null || !callback.compareAndSet(cb, null)) { - throw new IllegalStateException("already invoked lock request callback"); - } - - - // notify the callback - try { - cb.set(locked); - - } catch (RuntimeException e) { - logger.info("lock request callback for resource {} owner {} threw an exception", resourceId, owner, e); - } - } - - /** - * Makes an exception for when an argument is {@code null}. - * - * @param msg exception message - * @return a new Exception - */ - public static IllegalArgumentException makeNullArgException(String msg) { - return new IllegalArgumentException(msg); - } -} 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 d4e7bee9..1184b017 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 @@ -20,7 +20,6 @@ package org.onap.policy.drools.core.lock; -import java.util.concurrent.Future; import org.onap.policy.drools.utils.OrderedService; import org.onap.policy.drools.utils.OrderedServiceImpl; @@ -62,6 +61,7 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService { } /** + * * Result of a requested operation. */ public enum OperResult { @@ -87,40 +87,21 @@ public interface PolicyResourceLockFeatureAPI extends OrderedService { } /** - * This method is called before a lock is acquired on a resource. If a callback is - * provided, and the implementer is unable to acquire the lock immediately, then the - * implementer will invoke the callback once the lock is acquired. If the implementer - * handled the request, then it will return a future, which may be in one of three - * states: - *
- *
isDone()=true and get()=true
- *
the lock has been acquired; the callback may or may not have been invoked
- *
isDone()=true and get()=false
- *
the lock request has been denied; the callback may or may not have been - * invoked
- *
isDone()=false
- *
the lock was not immediately available and a callback was provided. The - * callback will be invoked once the lock is acquired (or denied). In this case, the - * future may be used to cancel the request
- *
+ * 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. * * @param resourceId * @param owner - * @param callback function to invoke, if the requester wishes to wait for the lock to - * come available, {@code null} to provide immediate replies - * @return a future for the lock, if the implementer handled the request, {@code null} - * if additional locking logic should be performed - * @throws IllegalStateException if the owner already holds the lock or is already in - * the queue to get the lock + * @param holdSec the amount of time, in seconds, that the lock should be held + * @return the result, where OPER_DENIED indicates that the lock is currently + * held by another owner */ - public default Future beforeLock(String resourceId, String owner, Callback callback) { - return null; + public default OperResult beforeLock(String resourceId, String owner, int holdSec) { + return OperResult.OPER_UNHANDLED; } /** - * This method is called after a lock for a resource has been acquired or denied. This - * may be invoked immediately, if the status can be determined immediately, or it may - * be invoked asynchronously, once the status has been determined. + * This method is called after a lock for a resource has been acquired or denied. * * @param resourceId * @param owner diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java index 97e7242d..afe1cabe 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/lock/PolicyResourceLockManager.java @@ -20,9 +20,6 @@ package org.onap.policy.drools.core.lock; -import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_OWNER; -import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_RESOURCE_ID; -import static org.onap.policy.drools.core.lock.LockRequestFuture.makeNullArgException; import java.util.List; import java.util.concurrent.Future; import java.util.function.Function; @@ -71,9 +68,22 @@ public class PolicyResourceLockManager extends SimpleLockManager { protected static void setFactory(Factory factory) { PolicyResourceLockManager.factory = factory; } + + /** + * This method is here temporarily so-as not to break the drools-applications + * build. It will be removed once drools-applications has been updated. + * @param resourceId + * @param owner + * @param callback + * @return nothing; always throws an exception + * @throws UnsupportedOperationException + */ + public Future lock(String resourceId, String owner, Callback callback) { + throw new UnsupportedOperationException("lock with callback"); + } @Override - public Future lock(String resourceId, String owner, Callback callback) { + public boolean lock(String resourceId, String owner, int holdSec) { if (resourceId == null) { throw makeNullArgException(MSG_NULL_RESOURCE_ID); } @@ -82,19 +92,16 @@ public class PolicyResourceLockManager extends SimpleLockManager { throw makeNullArgException(MSG_NULL_OWNER); } - Future result = doIntercept(null, impl -> impl.beforeLock(resourceId, owner, callback)); - if (result != null) { - return result; - } - // implementer didn't do the work - use superclass - result = super.lock(resourceId, owner, callback); + return doBoolIntercept(impl -> impl.beforeLock(resourceId, owner, holdSec), () -> { - boolean locked = ((LockRequestFuture) result).isLocked(); + // implementer didn't do the work - defer to the superclass + boolean locked = super.lock(resourceId, owner, holdSec); - doIntercept(false, impl -> impl.afterLock(resourceId, owner, locked)); + doIntercept(false, impl -> impl.afterLock(resourceId, owner, locked)); - return result; + return locked; + }); } @Override 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 9bcf2598..79fa0a7d 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 @@ -20,18 +20,13 @@ package org.onap.policy.drools.core.lock; -import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_OWNER; -import static org.onap.policy.drools.core.lock.LockRequestFuture.MSG_NULL_RESOURCE_ID; -import static org.onap.policy.drools.core.lock.LockRequestFuture.makeNullArgException; import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.SortedSet; import java.util.TreeSet; -import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import org.onap.policy.common.utils.time.CurrentTime; -import org.onap.policy.drools.core.lock.PolicyResourceLockFeatureAPI.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,10 +38,9 @@ public class SimpleLockManager { protected static Logger logger = LoggerFactory.getLogger(SimpleLockManager.class); - /** - * Maximum age, in milliseconds, before a lock is considered stale and released. - */ - protected static final long MAX_AGE_MS = TimeUnit.MINUTES.toMillis(15L); + // messages used in exceptions + public static final String MSG_NULL_RESOURCE_ID = "null resourceId"; + public static final String MSG_NULL_OWNER = "null owner"; /** * Used to access the current time. May be overridden by junit tests. @@ -80,31 +74,16 @@ public class SimpleLockManager { } /** - * Attempts to lock a resource. This method ignores the callback and always returns a - * {@link CompletedLockRequest}. + * Attempts to lock a resource, extending the lock if the owner already holds it. * * @param resourceId * @param owner - * @param callback function to invoke, if the requester wishes to wait for the lock to - * be acquired, {@code null} to provide immediate replies - * @return a future for the lock request. The future will be in one of three states: - *
- *
isDone()=true and get()=true
- *
the lock has been acquired; the callback may or may not have been - * invoked
- *
isDone()=true and get()=false
- *
the lock request has been denied; the callback may or may not have been - * invoked
- *
isDone()=false
- *
the lock was not immediately available and a callback was provided. The - * callback will be invoked once the lock is acquired (or denied). In this - * case, the future may be used to cancel the request
- *
+ * @param holdSec the amount of time, in seconds, that the lock should be held + * @return {@code true} if locked, {@code false} if the resource is already locked by + * a different owner * @throws IllegalArgumentException if the resourceId or owner is {@code null} - * @throws IllegalStateException if the owner already holds the lock or is already in - * the queue to get the lock */ - public Future lock(String resourceId, String owner, Callback callback) { + public boolean lock(String resourceId, String owner, int holdSec) { if (resourceId == null) { throw makeNullArgException(MSG_NULL_RESOURCE_ID); @@ -118,22 +97,24 @@ public class SimpleLockManager { synchronized(locker) { cleanUpLocks(); - - if((existingLock = resource2data.get(resourceId)) == null) { - Data data = new Data(owner, resourceId, currentTime.getMillis() + MAX_AGE_MS); + + 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) { + Data data = new Data(owner, resourceId, currentTime.getMillis() + TimeUnit.SECONDS.toMillis(holdSec)); resource2data.put(resourceId, data); locks.add(data); } } boolean locked = (existingLock == null); - if (existingLock != null && owner.equals(existingLock.getOwner())) { - throw new IllegalStateException("lock for resource " + resourceId + " already owned by " + owner); - } - logger.info("lock {} for resource {} owner {}", locked, resourceId, owner); - return new LockRequestFuture(resourceId, owner, locked); + return locked; } /** @@ -254,6 +235,16 @@ public class SimpleLockManager { } } } + + /** + * Makes an exception for when an argument is {@code null}. + * + * @param msg exception message + * @return a new Exception + */ + public static IllegalArgumentException makeNullArgException(String msg) { + return new IllegalArgumentException(msg); + } /** * Data for a single Lock. Sorts by expiration time, then resource, and -- cgit 1.2.3-korg