From d437070115d2e7c151deb2dea680007e20651413 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 30 Oct 2019 16:28:55 -0400 Subject: Refactor duplicate code from lock managers Change-Id: I8910a1a4267d824f064b52c6ad08945590bd9617 Issue-ID: POLICY-2203 Signed-off-by: Jim Hahn --- .../drools/system/internal/FeatureLockImpl.java | 211 +++++++++++++++++++++ .../policy/drools/system/internal/LockManager.java | 197 +++++++++++++++++++ .../drools/system/internal/SimpleLockManager.java | 209 ++++---------------- 3 files changed, 449 insertions(+), 168 deletions(-) create mode 100644 policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java create mode 100644 policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java (limited to 'policy-management/src/main') diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java new file mode 100644 index 00000000..d4e4f5fc --- /dev/null +++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/FeatureLockImpl.java @@ -0,0 +1,211 @@ +/* + * ============LICENSE_START======================================================= + * ONAP + * ================================================================================ + * Copyright (C) 2019 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.system.internal; + +import java.util.concurrent.ScheduledExecutorService; +import org.onap.policy.drools.core.lock.LockCallback; +import org.onap.policy.drools.core.lock.LockImpl; +import org.onap.policy.drools.core.lock.LockState; +import org.onap.policy.drools.system.PolicyEngineConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Lock implementation used by locking features. + */ +public abstract class FeatureLockImpl extends LockImpl { + private static final long serialVersionUID = 1L; + private static final Logger logger = LoggerFactory.getLogger(FeatureLockImpl.class); + + public static final String LOCK_LOST_MSG = "lock lost"; + + /** + * {@code True} if this lock is attached to a feature, {@code false} if it is not. + */ + private transient boolean attached; + + /** + * Constructs the object. + */ + public FeatureLockImpl() { + this.attached = false; + } + + /** + * Constructs the object. + * + * @param state initial state of the lock + * @param resourceId identifier of the resource to be locked + * @param ownerKey information identifying the owner requesting the lock + * @param holdSec amount of time, in seconds, for which the lock should be held, after + * which it will automatically be released + * @param callback callback to be invoked once the lock is granted, or subsequently + * lost; must not be {@code null} + */ + public FeatureLockImpl(LockState state, String resourceId, String ownerKey, int holdSec, LockCallback callback) { + super(state, resourceId, ownerKey, holdSec, callback); + this.attached = true; + } + + /** + * Grants this lock. The notification is always invoked via a background + * thread. + * + * @param foreground {@code true} if to invoke the callback in the foreground thread, + * {@code false} otherwise + */ + protected synchronized void grant(boolean foreground) { + if (isUnavailable()) { + return; + } + + setState(LockState.ACTIVE); + updateGrant(); + + logger.info("lock granted: {}", this); + + if (foreground) { + notifyAvailable(); + } else { + getThreadPool().execute(this::notifyAvailable); + } + } + + /** + * Permanently denies this lock. + * + * @param reason the reason the lock was denied + * @param foreground {@code true} if to invoke the callback in the foreground thread, + * {@code false} otherwise + */ + public void deny(String reason, boolean foreground) { + synchronized (this) { + setState(LockState.UNAVAILABLE); + } + + logger.info("{}: {}", reason, this); + + if (foreground) { + notifyUnavailable(); + } else { + getThreadPool().execute(this::notifyUnavailable); + } + } + + /** + * The subclass should make use of {@link #freeAllowed()} in its implementation of + * {@link #free()}. + */ + @Override + public abstract boolean free(); + + /** + * Determines if the lock can be freed. + * + * @return {@code true} if the lock can be freed, {@code false} if the lock is + * unavailable + */ + protected boolean freeAllowed() { + // do a quick check of the state + if (isUnavailable()) { + return false; + } + + logger.info("releasing lock: {}", this); + + if (!attachFeature()) { + setState(LockState.UNAVAILABLE); + return false; + } + + return true; + } + + /** + * The subclass should make use of {@link #extendAllowed()} in its implementation of + * {@link #extend()}. + */ + @Override + public abstract void extend(int holdSec, LockCallback callback); + + /** + * Determines if the lock can be extended. + * + * @param holdSec the additional amount of time to hold the lock, in seconds + * @param callback callback to be invoked when the extension completes + * @return {@code true} if the lock can be extended, {@code false} if the lock is + * unavailable + */ + protected boolean extendAllowed(int holdSec, LockCallback callback) { + if (holdSec < 0) { + throw new IllegalArgumentException("holdSec is negative"); + } + + setHoldSec(holdSec); + setCallback(callback); + + // do a quick check of the state + if (isUnavailable() || !attachFeature()) { + deny(LOCK_LOST_MSG, true); + return false; + } + + return true; + } + + /** + * Attaches to the feature instance, if not already attached. + * + * @return {@code true} if the lock is now attached to a feature, {@code false} + * otherwise + */ + private synchronized boolean attachFeature() { + if (!attached) { + attached = addToFeature(); + } + + return attached; + } + + /** + * Updates a lock when it is granted. The default method does nothing. + */ + protected void updateGrant() { + // do nothing + } + + /** + * Adds the lock to the relevant feature. + * + * @return {@code true} if the lock was added, {@code false} if it could not be added + * (e.g., because there is no feature yet) + */ + protected abstract boolean addToFeature(); + + /** + * Gets the thread pool. + * + * @return the thread pool + */ + protected ScheduledExecutorService getThreadPool() { + return PolicyEngineConstants.getManager().getExecutorService(); + } +} diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java new file mode 100644 index 00000000..7e4505be --- /dev/null +++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/LockManager.java @@ -0,0 +1,197 @@ +/* + * ============LICENSE_START======================================================= + * ONAP + * ================================================================================ + * Copyright (C) 2019 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.system.internal; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import lombok.AccessLevel; +import lombok.Getter; +import org.onap.policy.drools.core.lock.AlwaysFailLock; +import org.onap.policy.drools.core.lock.Lock; +import org.onap.policy.drools.core.lock.LockCallback; +import org.onap.policy.drools.core.lock.LockState; +import org.onap.policy.drools.core.lock.PolicyResourceLockManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * Super class for Lock Features. + * + *

+ * Note: this implementation does not honor the waitForLocks={@code true} + * parameter. + * + *

+ * When a lock is deserialized, it will not initially appear in this feature's map; it + * will be added to the map once free() or extend() is invoked, provided there isn't + * already an entry. + */ +public abstract class LockManager implements PolicyResourceLockManager { + + private static final Logger logger = LoggerFactory.getLogger(LockManager.class); + + public static final String NOT_LOCKED_MSG = "not locked"; + + /** + * Maps a resource to the lock that owns it. + */ + @Getter(AccessLevel.PROTECTED) + private final Map resource2lock = new ConcurrentHashMap<>(); + + /** + * {@code True} if this feature is running, {@code false} otherwise. + */ + private boolean alive = false; + + /** + * {@code True} if this feature is locked, {@code false} otherwise. + */ + private boolean locked = false; + + + /** + * Constructs the object. + */ + public LockManager() { + super(); + } + + @Override + public boolean isAlive() { + return alive; + } + + @Override + public synchronized boolean start() { + if (alive) { + return false; + } + + alive = true; + return true; + } + + /** + * Stops the expiration checker. Does not invoke any lock call-backs. + */ + @Override + public synchronized boolean stop() { + if (!alive) { + return false; + } + + alive = false; + return true; + } + + @Override + public void shutdown() { + stop(); + } + + @Override + public boolean isLocked() { + return locked; + } + + @Override + public synchronized boolean lock() { + if (locked) { + return false; + } + + locked = true; + return true; + } + + @Override + public synchronized boolean unlock() { + if (!locked) { + return false; + } + + locked = false; + return true; + } + + /** + * After performing checks, this invokes + * {@link #makeLock(LockState, String, String, int, LockCallback)} to create a lock + * object, inserts it into the map, and then invokes {@link #finishLock(MgrLock)}. + */ + @Override + public Lock createLock(String resourceId, String ownerKey, int holdSec, LockCallback callback, + boolean waitForLock) { + + if (hasInstanceChanged()) { + AlwaysFailLock lock = new AlwaysFailLock(resourceId, ownerKey, holdSec, callback); + lock.notifyUnavailable(); + return lock; + } + + T lock = makeLock(LockState.WAITING, resourceId, ownerKey, holdSec, callback); + + T existingLock = resource2lock.putIfAbsent(resourceId, lock); + + if (existingLock == null) { + logger.debug("added lock to map {}", lock); + finishLock(lock); + } else { + lock.deny("resource is busy", true); + } + + return lock; + } + + /** + * Determines if this object is no longer the current instance of this feature type. + * + * @return {@code true} if this object is no longer the current instance, + * {@code false} otherwise + */ + protected abstract boolean hasInstanceChanged(); + + /** + * Finishes the steps required to establish a lock, changing its state to ACTIVE, when + * appropriate. + * + * @param lock the lock to be locked + */ + protected abstract void finishLock(T lock); + + // these may be overridden by junit tests + + /** + * Makes a lock of the appropriate type. + * + * @param state initial state of the lock + * @param resourceId identifier of the resource to be locked + * @param ownerKey information identifying the owner requesting the lock + * @param holdSec amount of time, in seconds, for which the lock should be held, after + * which it will automatically be released + * @param callback callback to be invoked once the lock is granted, or subsequently + * lost; must not be {@code null} + * @return a new lock + */ + protected abstract T makeLock(LockState waiting, String resourceId, String ownerKey, int holdSec, + LockCallback callback); +} diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java b/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java index f5163e9b..839c17dc 100644 --- a/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java +++ b/policy-management/src/main/java/org/onap/policy/drools/system/internal/SimpleLockManager.java @@ -23,8 +23,6 @@ package org.onap.policy.drools.system.internal; import java.util.Map; import java.util.Map.Entry; import java.util.Properties; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -34,13 +32,10 @@ import lombok.Getter; import lombok.Setter; import org.onap.policy.common.utils.properties.exception.PropertyException; import org.onap.policy.common.utils.time.CurrentTime; -import org.onap.policy.drools.core.lock.AlwaysFailLock; -import org.onap.policy.drools.core.lock.Lock; import org.onap.policy.drools.core.lock.LockCallback; -import org.onap.policy.drools.core.lock.LockImpl; import org.onap.policy.drools.core.lock.LockState; -import org.onap.policy.drools.core.lock.PolicyResourceLockManager; import org.onap.policy.drools.system.PolicyEngine; +import org.onap.policy.drools.system.PolicyEngineConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,13 +53,10 @@ import org.slf4j.LoggerFactory; * will be added to the map once free() or extend() is invoked, provided there isn't * already an entry. */ -public class SimpleLockManager implements PolicyResourceLockManager { +public class SimpleLockManager extends LockManager { private static final Logger logger = LoggerFactory.getLogger(SimpleLockManager.class); - private static final String NOT_LOCKED_MSG = "not locked"; - private static final String LOCK_LOST_MSG = "lock lost"; - /** * Provider of current time. May be overridden by junit tests. */ @@ -74,12 +66,6 @@ public class SimpleLockManager implements PolicyResourceLockManager { @Setter(AccessLevel.PROTECTED) private static SimpleLockManager latestInstance = null; - - /** - * Engine with which this manager is associated. - */ - private final PolicyEngine engine; - /** * Feature properties. */ @@ -88,13 +74,7 @@ public class SimpleLockManager implements PolicyResourceLockManager { /** * Maps a resource to the lock that owns it. */ - private final Map resource2lock = new ConcurrentHashMap<>(); - - /** - * Thread pool used to check for lock expiration and to notify owners when locks are - * lost. - */ - private ScheduledExecutorService exsvc = null; + private final Map resource2lock; /** * Used to cancel the expiration checker on shutdown. @@ -110,8 +90,8 @@ public class SimpleLockManager implements PolicyResourceLockManager { */ public SimpleLockManager(PolicyEngine engine, Properties properties) { try { - this.engine = engine; this.featProps = new SimpleLockProperties(properties); + this.resource2lock = getResource2lock(); } catch (PropertyException e) { throw new SimpleLockManagerException(e); @@ -119,24 +99,17 @@ public class SimpleLockManager implements PolicyResourceLockManager { } @Override - public boolean isAlive() { - return (checker != null); - } - - @Override - public boolean start() { - if (checker != null) { + public synchronized boolean start() { + if (isAlive()) { return false; } - exsvc = getThreadPool(); - - checker = exsvc.scheduleWithFixedDelay(this::checkExpired, featProps.getExpireCheckSec(), - featProps.getExpireCheckSec(), TimeUnit.SECONDS); + checker = PolicyEngineConstants.getManager().getExecutorService().scheduleWithFixedDelay(this::checkExpired, + featProps.getExpireCheckSec(), featProps.getExpireCheckSec(), TimeUnit.SECONDS); setLatestInstance(this); - return true; + return super.start(); } /** @@ -144,9 +117,7 @@ public class SimpleLockManager implements PolicyResourceLockManager { */ @Override public synchronized boolean stop() { - exsvc = null; - - if (checker == null) { + if (!super.stop()) { return false; } @@ -158,49 +129,6 @@ public class SimpleLockManager implements PolicyResourceLockManager { return true; } - @Override - public void shutdown() { - stop(); - } - - @Override - public boolean isLocked() { - return false; - } - - @Override - public boolean lock() { - return true; - } - - @Override - public boolean unlock() { - return true; - } - - @Override - public Lock createLock(String resourceId, String ownerKey, int holdSec, LockCallback callback, - boolean waitForLock) { - - if (latestInstance != this) { - AlwaysFailLock lock = new AlwaysFailLock(resourceId, ownerKey, holdSec, callback); - lock.notifyUnavailable(); - return lock; - } - - SimpleLock lock = makeLock(LockState.WAITING, resourceId, ownerKey, holdSec, callback); - - SimpleLock existingLock = resource2lock.putIfAbsent(resourceId, lock); - - if (existingLock == null) { - lock.grant(); - } else { - lock.deny("resource is busy"); - } - - return lock; - } - /** * Checks for expired locks. */ @@ -230,15 +158,25 @@ public class SimpleLockManager implements PolicyResourceLockManager { SimpleLock lock = lockref.get(); if (lock != null) { - lock.deny("lock expired"); + lock.deny("lock expired", false); } } } + @Override + protected void finishLock(SimpleLock lock) { + lock.grant(true); + } + + @Override + protected boolean hasInstanceChanged() { + return (getLatestInstance() != this); + } + /** * Simple Lock implementation. */ - public static class SimpleLock extends LockImpl { + public static class SimpleLock extends FeatureLockImpl { private static final long serialVersionUID = 1L; /** @@ -248,17 +186,16 @@ public class SimpleLockManager implements PolicyResourceLockManager { private long holdUntilMs; /** - * Feature containing this lock. May be {@code null} until the feature is - * identified. Note: this can only be null if the lock has been de-serialized. + * Map that should hold this lock. */ - private transient SimpleLockManager feature; + private transient Map resource2lock; /** * Constructs the object. */ public SimpleLock() { this.holdUntilMs = 0; - this.feature = null; + this.resource2lock = null; } /** @@ -276,7 +213,7 @@ public class SimpleLockManager implements PolicyResourceLockManager { public SimpleLock(LockState state, String resourceId, String ownerKey, int holdSec, LockCallback callback, SimpleLockManager feature) { super(state, resourceId, ownerKey, holdSec, callback); - this.feature = feature; + this.resource2lock = feature.resource2lock; } /** @@ -289,62 +226,15 @@ public class SimpleLockManager implements PolicyResourceLockManager { return (holdUntilMs <= currentMs); } - /** - * Grants this lock. The notification is always invoked via a background - * thread. - */ - protected synchronized void grant() { - if (isUnavailable()) { - return; - } - - setState(LockState.ACTIVE); - holdUntilMs = currentTime.getMillis() + TimeUnit.SECONDS.toMillis(getHoldSec()); - - logger.info("lock granted: {}", this); - - feature.exsvc.execute(this::notifyAvailable); - } - - /** - * Permanently denies this lock. The notification is invoked via a background - * thread, if a feature instance is attached, otherwise it uses the foreground - * thread. - * - * @param reason the reason the lock was denied - */ - protected void deny(String reason) { - synchronized (this) { - setState(LockState.UNAVAILABLE); - } - - logger.info("{}: {}", reason, this); - - if (feature == null) { - notifyUnavailable(); - - } else { - feature.exsvc.execute(this::notifyUnavailable); - } - } - @Override public boolean free() { - // do a quick check of the state - if (isUnavailable()) { - return false; - } - - logger.info("releasing lock: {}", this); - - if (!attachFeature()) { - setState(LockState.UNAVAILABLE); + if (!freeAllowed()) { return false; } AtomicBoolean result = new AtomicBoolean(false); - feature.resource2lock.computeIfPresent(getResourceId(), (resourceId, curlock) -> { + resource2lock.computeIfPresent(getResourceId(), (resourceId, curlock) -> { if (curlock == this) { // this lock was the owner - resource is now available @@ -362,46 +252,33 @@ public class SimpleLockManager implements PolicyResourceLockManager { @Override public void extend(int holdSec, LockCallback callback) { - if (holdSec < 0) { - throw new IllegalArgumentException("holdSec is negative"); - } - - setHoldSec(holdSec); - setCallback(callback); - - // do a quick check of the state - if (isUnavailable() || !attachFeature()) { - deny(LOCK_LOST_MSG); + if (!extendAllowed(holdSec, callback)) { return; } - if (feature.resource2lock.get(getResourceId()) == this) { - grant(); + if (resource2lock.get(getResourceId()) == this) { + grant(true); } else { - deny(NOT_LOCKED_MSG); + deny(NOT_LOCKED_MSG, true); } } - /** - * Attaches to the feature instance, if not already attached. - * - * @return {@code true} if the lock is now attached to a feature, {@code false} - * otherwise - */ - private synchronized boolean attachFeature() { - if (feature != null) { - // already attached - return true; - } + @Override + protected void updateGrant() { + holdUntilMs = currentTime.getMillis() + TimeUnit.SECONDS.toMillis(getHoldSec()); + } - feature = latestInstance; + @Override + protected boolean addToFeature() { + SimpleLockManager feature = getLatestInstance(); if (feature == null) { logger.warn("no feature yet for {}", this); return false; } // put this lock into the map - feature.resource2lock.putIfAbsent(getResourceId(), this); + resource2lock = feature.resource2lock; + resource2lock.putIfAbsent(getResourceId(), this); return true; } @@ -415,10 +292,6 @@ public class SimpleLockManager implements PolicyResourceLockManager { // these may be overridden by junit tests - protected ScheduledExecutorService getThreadPool() { - return engine.getExecutorService(); - } - protected SimpleLock makeLock(LockState waiting, String resourceId, String ownerKey, int holdSec, LockCallback callback) { return new SimpleLock(waiting, resourceId, ownerKey, holdSec, callback, this); -- cgit 1.2.3-korg