The lock class is made more greedy so all locks are granted in the requested order.
Previously, an exclusive lock could be discriminated.
Issue-ID: CCSDK-3560
Signed-off-by: PatrikBuhr <patrik.buhr@est.tech>
Change-Id: If2dd171409c58eacbccce9569b2f6694e09992a2
},
"/a1-policy/v2/services/{service_id}/keepalive": {"put": {
"summary": "Heartbeat indicates that the service is running",
- "description": "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that teh service will be deregisterred and all its policies are removed.",
+ "description": "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that the service will be deregisterred and all its policies are removed.",
"operationId": "keepAliveService",
"responses": {
"200": {
- Service Registry and Supervision
summary: Heartbeat indicates that the service is running
description: A registerred service must call this in regular intervals to indicate
- that it is in operation. Absence of this call will lead to that teh service
+ that it is in operation. Absence of this call will lead to that the service
will be deregisterred and all its policies are removed.
operationId: keepAliveService
parameters:
import org.onap.ccsdk.oran.a1policymanagementservice.controllers.VoidResponse;
import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.EntityNotFoundException;
import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.ServiceException;
+import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policies;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policy;
@PathVariable(Consts.POLICY_ID_PARAM) String policyId) throws EntityNotFoundException {
Policy policy = policies.getPolicy(policyId);
keepServiceAlive(policy.getOwnerServiceId());
- Ric ric = policy.getRic();
- return ric.getLock().lock(LockType.SHARED) //
- .flatMap(notUsed -> assertRicStateIdle(ric)) //
+ return policy.getRic().getLock().lock(LockType.SHARED, "deletePolicy") //
+ .flatMap(grant -> deletePolicy(grant, policy));
+ }
+
+ Mono<ResponseEntity<Object>> deletePolicy(Lock.Grant grant, Policy policy) {
+ return assertRicStateIdle(policy.getRic()) //
.flatMap(notUsed -> a1ClientFactory.createA1Client(policy.getRic())) //
.doOnNext(notUsed -> policies.remove(policy)) //
+ .doFinally(x -> grant.unlockBlocking()) //
.flatMap(client -> client.deletePolicy(policy)) //
- .doOnNext(notUsed -> ric.getLock().unlockBlocking()) //
- .doOnError(notUsed -> ric.getLock().unlockBlocking()) //
.map(notUsed -> new ResponseEntity<>(HttpStatus.NO_CONTENT)) //
.onErrorResume(this::handleException);
}
.statusNotificationUri(policyInfo.statusNotificationUri == null ? "" : policyInfo.statusNotificationUri) //
.build();
+ return ric.getLock().lock(LockType.SHARED, "putPolicy") //
+ .flatMap(grant -> putPolicy(grant, policy));
+ }
+
+ private Mono<ResponseEntity<Object>> putPolicy(Lock.Grant grant, Policy policy) {
final boolean isCreate = this.policies.get(policy.getId()) == null;
+ final Ric ric = policy.getRic();
- return ric.getLock().lock(LockType.SHARED) //
- .flatMap(notUsed -> assertRicStateIdle(ric)) //
- .flatMap(notUsed -> checkSupportedType(ric, type)) //
+ return assertRicStateIdle(ric) //
+ .flatMap(notUsed -> checkSupportedType(ric, policy.getType())) //
.flatMap(notUsed -> validateModifiedPolicy(policy)) //
.flatMap(notUsed -> a1ClientFactory.createA1Client(ric)) //
.flatMap(client -> client.putPolicy(policy)) //
.doOnNext(notUsed -> policies.put(policy)) //
- .doOnNext(notUsed -> ric.getLock().unlockBlocking()) //
- .doOnError(trowable -> ric.getLock().unlockBlocking()) //
+ .doFinally(x -> grant.unlockBlocking()) //
.flatMap(notUsed -> Mono.just(new ResponseEntity<>(isCreate ? HttpStatus.CREATED : HttpStatus.OK))) //
.onErrorResume(this::handleException);
+
}
private Mono<ResponseEntity<Object>> handleException(Throwable throwable) {
}
@Operation(summary = "Heartbeat indicates that the service is running",
- description = "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that teh service will be deregisterred and all its policies are removed.")
+ description = "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that the service will be deregisterred and all its policies are removed.")
@ApiResponses(value = { //
@ApiResponse(responseCode = "200", description = "Service supervision timer refreshed, OK"), //
@ApiResponse(responseCode = "404", description = "The service is not found, needs re-registration",
package org.onap.ccsdk.oran.a1policymanagementservice.repository;
import java.util.ArrayList;
-import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
+import java.util.Queue;
+
+import lombok.Getter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class Lock {
private static final Logger logger = LoggerFactory.getLogger(Lock.class);
- private boolean isExclusive = false;
+ boolean isExclusive = false;
private int lockCounter = 0;
- private final List<LockRequest> lockRequestQueue = new LinkedList<>();
+ final Queue<LockRequest> lockRequestQueue = new LinkedList<>();
private static AsynchCallbackExecutor callbackProcessor = new AsynchCallbackExecutor();
public enum LockType {
EXCLUSIVE, SHARED
}
- /** The caller thread will be blocked util the lock is granted. */
- public synchronized void lockBlocking(LockType locktype) {
- while (!tryLock(locktype)) {
- this.waitForUnlock();
+ /**
+ * A grant is achieved when the lock is granted.
+ * It can be used for unlocking.
+ */
+ public static class Grant {
+ private final Lock lock;
+ private boolean unlocked = false;
+ @Getter
+ private final String label;
+
+ Grant(Lock lock, String label) {
+ this.lock = lock;
+ this.label = label;
+ }
+
+ /**
+ * reactive unlocking. Submits the lock.
+ *
+ * @return the lock
+ */
+ public Mono<Lock> unlock() {
+ check();
+ return this.lock.unlock();
+ }
+
+ /**
+ * Synchronuous unlocking
+ */
+ public void unlockBlocking() {
+ check();
+ this.lock.unlockBlocking();
+ }
+
+ private void check() {
+ if (unlocked) {
+ logger.error("Lock already unlocked");
+ }
+ unlocked = true;
}
}
- /** Reactive version. The Lock will be emitted when the lock is granted */
- public synchronized Mono<Lock> lock(LockType lockType) {
+ /**
+ * Reactive lock. The Lock will be emitted when the lock is granted
+ *
+ * @param lockType type of lock (exclusive/shared)
+ * @param label a label that will be attached to the request. Will be passed
+ * back in the Grant
+ * @return a Grant that cane be used only to unlock.
+ */
+ public synchronized Mono<Grant> lock(LockType lockType, String label) {
if (tryLock(lockType)) {
- return Mono.just(this);
+ return Mono.just(new Grant(this, label));
} else {
- return Mono.create(monoSink -> addToQueue(monoSink, lockType));
+ return Mono.create(monoSink -> addToQueue(monoSink, lockType, label));
+ }
+ }
+
+ /**
+ * A synchronuous variant of locking. The caller thread will be blocked util the
+ * lock is granted.
+ */
+ public synchronized Grant lockBlocking(LockType locktype, String label) {
+ while (!tryLock(locktype)) {
+ this.waitForUnlock();
}
+ return new Grant(this, label);
}
public Mono<Lock> unlock() {
private void processQueuedEntries() {
List<LockRequest> granted = new ArrayList<>();
- for (Iterator<LockRequest> i = lockRequestQueue.iterator(); i.hasNext();) {
- LockRequest request = i.next();
+ while (!lockRequestQueue.isEmpty()) {
+ LockRequest request = lockRequestQueue.element();
if (tryLock(request.lockType)) {
- i.remove();
+ lockRequestQueue.remove();
granted.add(request);
+ } else {
+ break; // Avoid starvation
}
}
callbackProcessor.addAll(granted);
}
- private synchronized void addToQueue(MonoSink<Lock> callback, LockType lockType) {
- lockRequestQueue.add(new LockRequest(callback, lockType, this));
- processQueuedEntries();
+ private synchronized void addToQueue(MonoSink<Grant> callback, LockType lockType, String label) {
+ lockRequestQueue.add(new LockRequest(callback, lockType, this, label));
}
@SuppressWarnings("java:S2274") // Always invoke wait() and await() methods inside a loop
* Represents a queued lock request
*/
private static class LockRequest {
- final MonoSink<Lock> callback;
+ final MonoSink<Grant> callback;
final LockType lockType;
final Lock lock;
+ final String label;
- LockRequest(MonoSink<Lock> callback, LockType lockType, Lock lock) {
+ LockRequest(MonoSink<Grant> callback, LockType lockType, Lock lock, String label) {
this.callback = callback;
this.lockType = lockType;
this.lock = lock;
+ this.label = label;
}
}
try {
while (true) {
for (LockRequest request : consume()) {
- request.callback.success(request.lock);
+ Grant g = new Grant(request.lock, request.label);
+ request.callback.success(g);
}
waitForNewEntries();
}
private Mono<RicData> checkOneRic(RicData ricData) {
return checkRicState(ricData) //
- .flatMap(x -> ricData.ric.getLock().lock(LockType.EXCLUSIVE)) //
+ .flatMap(x -> ricData.ric.getLock().lock(LockType.EXCLUSIVE, "checkOneRic")) //
.flatMap(notUsed -> setRicState(ricData)) //
.flatMap(x -> checkRicPolicies(ricData)) //
.flatMap(x -> checkRicPolicyTypes(ricData)) //
return;
}
- ric.getLock().lock(LockType.EXCLUSIVE) //
+ ric.getLock().lock(LockType.EXCLUSIVE, "RicSynchronizationTask") //
.flatMap(notUsed -> synchronizeRic(ric)) //
.subscribe(new BaseSubscriber<Object>() {
@SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally
private Flux<Policy> deletePolicy(Policy policy) {
Lock lock = policy.getRic().getLock();
- return lock.lock(LockType.SHARED) //
+ return lock.lock(LockType.SHARED, "ServiceSupervision") //
.doOnNext(notUsed -> policies.remove(policy)) //
.flatMap(notUsed -> deletePolicyInRic(policy))
.doOnNext(notUsed -> logger.debug("Policy deleted due to service inactivity: {}, service: {}",
policy.getId(), policy.getOwnerServiceId())) //
- .doOnNext(notUsed -> lock.unlockBlocking()) //
- .doOnError(throwable -> lock.unlockBlocking()) //
+ .doFinally(notUsed -> lock.unlockBlocking()) //
.doOnError(throwable -> logger.debug("Failed to delete inactive policy: {}, reason: {}", policy.getId(),
throwable.getMessage())) //
.flatMapMany(notUsed -> Flux.just(policy)) //
import org.onap.ccsdk.oran.a1policymanagementservice.configuration.WebClientConfig;
import org.onap.ccsdk.oran.a1policymanagementservice.controllers.ServiceCallbackInfo;
import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.ServiceException;
+import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policies;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policy;
@AfterEach
void verifyNoRicLocks() {
for (Ric ric : this.rics.getRics()) {
- ric.getLock().lockBlocking(LockType.EXCLUSIVE);
- ric.getLock().unlockBlocking();
+ Lock.Grant grant = ric.getLock().lockBlocking(LockType.EXCLUSIVE, "");
+ grant.unlockBlocking();
assertThat(ric.getLock().getLockCounter()).isZero();
assertThat(ric.getState()).isEqualTo(Ric.RicState.AVAILABLE);
}
// Check that a service callback for the AVAILABLE RIC is invoked
final RappSimulatorController.TestResults receivedCallbacks = rAppSimulator.getTestResults();
- await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+ await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
ServiceCallbackInfo callbackInfo = receivedCallbacks.getReceivedInfo().get(0);
assertThat(callbackInfo.ricId).isEqualTo(RIC);
assertThat(callbackInfo.eventType).isEqualTo(ServiceCallbackInfo.EventType.AVAILABLE);
supervision.checkAllRics();
waitForRicState(RIC, RicState.AVAILABLE);
- await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+ await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
}
@Test
waitForRicState("ric1", RicState.AVAILABLE);
final RappSimulatorController.TestResults receivedCallbacks = rAppSimulator.getTestResults();
- await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo().size()).isEqualTo(1));
+ await().untilAsserted(() -> assertThat(receivedCallbacks.getReceivedInfo()).hasSize(1));
ServiceCallbackInfo callbackInfo = receivedCallbacks.getReceivedInfo().get(0);
assertThat(callbackInfo.ricId).isEqualTo("ric1");
assertThat(callbackInfo.eventType).isEqualTo(ServiceCallbackInfo.EventType.AVAILABLE);
package org.onap.ccsdk.oran.a1policymanagementservice.repository;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
import java.io.IOException;
+import java.lang.invoke.MethodHandles;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.onap.ccsdk.oran.a1policymanagementservice.exceptions.ServiceException;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
@ExtendWith(MockitoExtension.class)
class LockTest {
+ private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
@SuppressWarnings("squid:S2925") // "Thread.sleep" should not be used in tests.
private void sleep() {
try {
}
}
- private void asynchUnlock(Lock lock) {
+ private void asynchUnlock(Lock.Grant grant, Lock lock) {
+ logger.info("Lock {} cnt: {}, exclusive: {}, queue: {}", grant.getLabel(), lock.getLockCounter(),
+ lock.isExclusive, lock.lockRequestQueue.size());
+
Thread thread = new Thread(() -> {
sleep();
- lock.unlockBlocking();
+ grant.unlockBlocking();
});
thread.start();
}
@Test
void testLock() throws IOException, ServiceException {
Lock lock = new Lock();
- lock.lockBlocking(LockType.SHARED);
- lock.unlockBlocking();
+ Lock.Grant grant = lock.lockBlocking(LockType.SHARED, "test");
+ grant.unlockBlocking();
+ assertThat(grant.getLabel()).isEqualTo("test");
- lock.lockBlocking(LockType.EXCLUSIVE);
- asynchUnlock(lock);
+ grant = lock.lockBlocking(LockType.EXCLUSIVE, "");
+ asynchUnlock(grant, lock);
- lock.lockBlocking(LockType.SHARED);
- lock.unlockBlocking();
+ grant = lock.lockBlocking(LockType.SHARED, "");
+ grant.unlockBlocking();
assertThat(lock.getLockCounter()).isZero();
}
void testReactiveLock() {
Lock lock = new Lock();
- Mono<Lock> seq = lock.lock(LockType.EXCLUSIVE) //
- .flatMap(l -> lock.lock(LockType.EXCLUSIVE)) //
- .flatMap(l -> lock.unlock());
+ Mono<?> l0 = lock.lock(LockType.EXCLUSIVE, "1").doOnNext(grant -> asynchUnlock(grant, lock));
+ Mono<?> l1 = lock.lock(LockType.SHARED, "2").doOnNext(grant -> asynchUnlock(grant, lock));
+ Mono<?> l2 = lock.lock(LockType.SHARED, "3").doOnNext(grant -> asynchUnlock(grant, lock));
+ Mono<?> l3 = lock.lock(LockType.EXCLUSIVE, "4").doOnNext(grant -> asynchUnlock(grant, lock));
+ Mono<?> l4 = lock.lock(LockType.SHARED, "5").doOnNext(grant -> asynchUnlock(grant, lock));
- asynchUnlock(lock);
- StepVerifier.create(seq) //
+ StepVerifier.create(Flux.zip(l0, l1, l2, l3, l4)) //
.expectSubscription() //
- .expectNext(lock) //
+ .expectNextCount(1) //
.verifyComplete();
- assertThat(lock.getLockCounter()).isZero();
-
+ await().untilAsserted(() -> assertThat(lock.getLockCounter()).isZero());
}
-
}
import org.onap.ccsdk.oran.a1policymanagementservice.clients.A1ClientFactory;
import org.onap.ccsdk.oran.a1policymanagementservice.configuration.ApplicationConfig;
import org.onap.ccsdk.oran.a1policymanagementservice.configuration.ImmutableRicConfig;
+import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Lock.LockType;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policies;
import org.onap.ccsdk.oran.a1policymanagementservice.repository.Policy;
@AfterEach
void verifyNoRicLocks() {
for (Ric ric : this.rics.getRics()) {
- ric.getLock().lockBlocking(LockType.EXCLUSIVE);
- ric.getLock().unlockBlocking();
+ Lock.Grant grant = ric.getLock().lockBlocking(LockType.EXCLUSIVE, "");
+ grant.unlockBlocking();
assertThat(ric.getLock().getLockCounter()).isZero();
}
}
},
"/a1-policy/v2/services/{service_id}/keepalive": {"put": {
"summary": "Heartbeat indicates that the service is running",
- "description": "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that teh service will be deregisterred and all its policies are removed.",
+ "description": "A registerred service must call this in regular intervals to indicate that it is in operation. Absence of this call will lead to that the service will be deregisterred and all its policies are removed.",
"operationId": "keepAliveService",
"responses": {
"200": {
- Service Registry and Supervision
summary: Heartbeat indicates that the service is running
description: A registerred service must call this in regular intervals to indicate
- that it is in operation. Absence of this call will lead to that teh service
+ that it is in operation. Absence of this call will lead to that the service
will be deregisterred and all its policies are removed.
operationId: keepAliveService
parameters:
Code Review / ccsdk / oran.git / commitdiff