Skip to content

Commit

Permalink
Improve thread safety of ResourceManager
Browse files Browse the repository at this point in the history 
This is the first step in the process of making ResourceManager
and ConnectorManager more thread-safe (MID-5954).

Here we only change ResourceCache to implement Cacheable, in order
to receive invalidation requests. Also some minor refactorings in
ResourceManager were done, mainly regarding (im)mutability of resource
objects being worked with.

(cherry picked from commit 4a773f1)
  • Loading branch information
@mederly
mederly committed Jan 23, 2020
1 parent dc084b8 commit d5dc643
Show file tree
Hide file tree
Showing 4 changed files with 364 additions and 314 deletions.
23 changes: 8 additions & 15 deletions infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismObjectImpl.java
View file
Expand Up @@ -241,20 +241,13 @@ public PrismObjectImpl<O> cloneComplex(CloneStrategy strategy) {
prismContext.getMonitor().beforeObjectClone(this);
}

// MethodInvocationRecord record = MethodInvocationRecord.create(PrismObjectImpl.class.getName() + ".cloneComplex", new Object[] { strategy });
// try {
PrismObjectImpl<O> clone = new PrismObjectImpl<>(getElementName(), getDefinition(), prismContext);
copyValues(strategy, clone);
PrismObjectImpl<O> clone = new PrismObjectImpl<>(getElementName(), getDefinition(), prismContext);
copyValues(strategy, clone);

if (prismContext != null && prismContext.getMonitor() != null) {
prismContext.getMonitor().afterObjectClone(this, clone);
}
return clone;
// } catch (RuntimeException t) {
// throw record.processException(t);
// } finally {
// record.afterCall();
// }
if (prismContext != null && prismContext.getMonitor() != null) {
prismContext.getMonitor().afterObjectClone(this, clone);
}
return clone;
}

protected void copyValues(CloneStrategy strategy, PrismObjectImpl<O> clone) {
Expand Down Expand Up @@ -460,11 +453,11 @@ public void setImmutable(boolean immutable) {
}

public PrismObject<O> cloneIfImmutable() {
return isImmutable() ? ((PrismObject) this).clone() : this;
return isImmutable() ? clone() : this;
}

public PrismObject<O> createImmutableClone() {
PrismObject<O> clone = ((PrismObject) this).clone();
PrismObject<O> clone = clone();
clone.setImmutable(true);
return clone;
}
Expand Down
10 changes: 4 additions & 6 deletions ...g-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java
View file
Expand Up @@ -556,10 +556,9 @@ private <T extends ObjectType> PrismObject<T> completeObject(Class<T> type, Pris
Collection<SelectorOptions<GetOperationOptions>> options, Task task, OperationResult result) throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException, ExpressionEvaluationException {

if (ResourceType.class.equals(type)) {

PrismObject<ResourceType> completeResource = resourceManager.getResource((PrismObject<ResourceType>) inObject,
SelectorOptions.findRootOptions(options), task, result);
return (PrismObject<T>) completeResource;
//noinspection unchecked
return (PrismObject<T>) resourceManager.completeResource((PrismObject<ResourceType>) inObject,
SelectorOptions.findRootOptions(options), task, result);
} else if (ShadowType.class.equals(type)) {
// should not happen, the shadow-related code is already in the ShadowCache
throw new IllegalStateException("BOOOM!");
Expand All @@ -568,7 +567,6 @@ private <T extends ObjectType> PrismObject<T> completeObject(Class<T> type, Pris

}
return inObject;

}

public <T extends ObjectType> Integer countObjects(Class<T> type, ObjectQuery query, Collection<SelectorOptions<GetOperationOptions>> options, Task task, OperationResult parentResult)
Expand Down Expand Up @@ -749,7 +747,7 @@ public <T extends ObjectType> PrismObject<T> deleteObject(Class<T> type, String

} else if (object.canRepresent(ResourceType.class)) {

resourceManager.deleteResource(oid, options, task, result);
resourceManager.deleteResource(oid, result);

} else {

Expand Down
183 changes: 127 additions & 56 deletions ...rovisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ResourceCache.java
View file
Expand Up @@ -6,36 +6,73 @@
*/
package com.evolveum.midpoint.provisioning.impl;

import java.util.HashMap;
import java.util.Map;

import com.evolveum.midpoint.CacheInvalidationContext;
import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.repo.api.Cacheable;
import com.evolveum.midpoint.repo.api.RepositoryService;
import com.evolveum.midpoint.repo.cache.CacheRegistry;
import com.evolveum.midpoint.schema.internals.InternalMonitor;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.caching.CachePerformanceCollector;
import com.evolveum.midpoint.util.exception.ObjectNotFoundException;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ResourceType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.SingleCacheStateInformationType;
import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;

import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ResourceType;
import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import static com.evolveum.midpoint.util.caching.CacheConfiguration.StatisticsLevel.PER_CACHE;

/**
* Class for caching ResourceType instances with a parsed schemas.
* Caches ResourceType instances with a parsed schemas.
*
* @author Radovan Semancik
* Resource cache is similar to repository cache. One of the differences is that it does not expire its entries.
* It relies on versions and on invalidation events instead. So we have to use resource object versions when querying it.
* (This could be perhaps changed in the future. But not now.)
*
* @author Radovan Semancik
*/
@Component
public class ResourceCache {
public class ResourceCache implements Cacheable {

private static final Trace LOGGER = TraceManager.getTrace(ResourceCache.class);

private Map<String,PrismObject<ResourceType>> cache;
@Autowired private PrismContext prismContext;
@Autowired private CacheRegistry cacheRegistry;
@Autowired @Qualifier("cacheRepositoryService")
private RepositoryService repositoryService;

@PostConstruct
public void register() {
cacheRegistry.registerCacheableService(this);
}

ResourceCache() {
cache = new HashMap<>();
@PreDestroy
public void unregister() {
cacheRegistry.unregisterCacheableService(this);
}

public synchronized void put(PrismObject<ResourceType> resource) throws SchemaException {
/**
* Note that prism objects in this map are always immutable. And they must remain immutable after getting them
* from the cache. So no "modifyUnfrozen" and similar tricks!
*
* As for ConcurrentHashMap: Although we use synchronization whenever possible, let's be extra cautious here.
*/
private final Map<String, PrismObject<ResourceType>> cache = new ConcurrentHashMap<>();

synchronized void put(PrismObject<ResourceType> resource) throws SchemaException {
String oid = resource.getOid();
if (oid == null) {
throw new SchemaException("Attempt to cache "+resource+" without an OID");
Expand All @@ -50,70 +87,84 @@ public synchronized void put(PrismObject<ResourceType> resource) throws SchemaEx
if (cachedResource == null) {
LOGGER.debug("Caching(new): {}", resource);
cache.put(oid, resource.createImmutableClone());
} else if (compareVersion(resource.getVersion(), cachedResource.getVersion())) {
LOGGER.debug("Caching fizzle, resource already cached: {}", resource);
// We already have equivalent resource, nothing to do
// TODO is this correct? What if the resource being put here is newer than the existing one (although having the same version)?
} else {
if (compareVersion(resource.getVersion(), cachedResource.getVersion())) {
LOGGER.debug("Caching fizzle, resource already cached: {}", resource);
// We already have equivalent resource, nothing to do
} else {
LOGGER.debug("Caching(replace): {}", resource);
cache.put(oid, resource.createImmutableClone());
}
LOGGER.debug("Caching(replace): {}", resource);
cache.put(oid, resource.createImmutableClone());
}
}

private boolean compareVersion(String version1, String version2) {
if (version1 == null && version2 == null) {
return true;
}
if (version1 == null || version2 == null) {
return false;
}
return version1.equals(version2);
}

public synchronized PrismObject<ResourceType> get(PrismObject<ResourceType> resource, GetOperationOptions options) throws SchemaException {
return get(resource.getOid(), resource.getVersion(), options);
return version1 == null && version2 == null || version1 != null && version1.equals(version2);
}

public synchronized PrismObject<ResourceType> get(String oid, String requestedVersion, GetOperationOptions options) throws SchemaException {
if (oid == null) {
return null;
}
/**
* Gets a resource if it has specified version. If it has not, purges it from the cache (even if it exists there).
*/
synchronized PrismObject<ResourceType> get(@NotNull String oid, String requestedVersion, boolean readOnly) {
InternalMonitor.getResourceCacheStats().recordRequest();

PrismObject<ResourceType> resourceToReturn;
PrismObject<ResourceType> cachedResource = cache.get(oid);
if (cachedResource == null) {
LOGGER.debug("MISS(not cached) for {}", oid);
return null;
}

if (!compareVersion(requestedVersion, cachedResource.getVersion())) {
resourceToReturn = null;
} else if (!compareVersion(requestedVersion, cachedResource.getVersion())) {
LOGGER.debug("MISS(wrong version) for {}", oid);
LOGGER.trace("Cached resource version {} does not match requested resource version {}, purging from cache", cachedResource.getVersion(), requestedVersion);
LOGGER.trace("Cached resource version {} does not match requested resource version {}, purging from cache",
cachedResource.getVersion(), requestedVersion);
cache.remove(oid);
return null;
resourceToReturn = null;
} else if (readOnly) {
cachedResource.checkImmutability();
LOGGER.trace("HIT(read only) for {}", cachedResource);
resourceToReturn = cachedResource;
} else {
LOGGER.debug("HIT(returning clone) for {}", cachedResource);
resourceToReturn = cachedResource.clone();
}

if (GetOperationOptions.isReadOnly(options)) {
try { // MID-4574
cachedResource.checkImmutability();
} catch (IllegalStateException ex) {
LOGGER.error("Failed immutability test", ex);
cache.remove(oid);
if (resourceToReturn != null) {
CachePerformanceCollector.INSTANCE.registerHit(ResourceCache.class, ResourceType.class, PER_CACHE);
InternalMonitor.getResourceCacheStats().recordHit();
} else {
CachePerformanceCollector.INSTANCE.registerMiss(ResourceCache.class, ResourceType.class, PER_CACHE);
InternalMonitor.getResourceCacheStats().recordMiss();
}
return resourceToReturn;
}

return null;
}
LOGGER.trace("HIT(read only) for {}", cachedResource);
return cachedResource;
/**
* Gets a resource without specifying requested version: returns one only if it has the same version as in the repo.
*
* This requires a cooperation with the repository cache. Therefore this method is NOT synchronized
* and has operation result as its parameter.
*/
PrismObject<ResourceType> getIfLatest(@NotNull String oid, boolean readonly, OperationResult parentResult)
throws SchemaException, ObjectNotFoundException {
// First let's check if the cache contains given resource. If not, we can avoid getting version from the repo.
if (contains(oid)) {
String version = repositoryService.getVersion(ResourceType.class, oid, parentResult);
return get(oid, version, readonly);
} else {
LOGGER.debug("HIT(returning clone) for {}", cachedResource);
return cachedResource.clone();
LOGGER.debug("MISS(not cached) for {}", oid);
CachePerformanceCollector.INSTANCE.registerMiss(ResourceCache.class, ResourceType.class, PER_CACHE);
InternalMonitor.getResourceCacheStats().recordMiss();
return null;
}
}

private synchronized boolean contains(@NotNull String oid) {
return cache.containsKey(oid);
}

/**
* Returns currently cached version. FOR DIAGNOSTICS ONLY.
*/
public synchronized String getVersion(String oid) {
synchronized String getVersion(String oid) {
if (oid == null) {
return null;
}
Expand All @@ -124,8 +175,28 @@ public synchronized String getVersion(String oid) {
return cachedResource.getVersion();
}

public synchronized void remove(String oid) {
synchronized void remove(String oid) {
cache.remove(oid);
}

@Override
public synchronized void invalidate(Class<?> type, String oid, CacheInvalidationContext context) {
if (type == null || type.isAssignableFrom(ResourceType.class)) {
if (oid != null) {
remove(oid);
} else {
cache.clear();
}
}
}

@NotNull
@Override
public synchronized Collection<SingleCacheStateInformationType> getStateInformation() {
return Collections.singleton(
new SingleCacheStateInformationType(prismContext)
.name(ResourceCache.class.getName())
.size(cache.size())
);
}
}

0 comments on commit d5dc643

Please sign in to comment.