-
Notifications
You must be signed in to change notification settings - Fork 80
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix bug in CachingCatalogFacade's cascade eviction
When a `CatalogInfo` is explicitly evicted in `CachingCatalogFacade`, due to a local mutating operation, or in reaction to remote event, any oher `CatalogInfo` that directly (e.g. `StyleInfo->WorkspaceInfo`), or indirecly (e.g. `LayerInfo->ResourceInfo->StoreInfo->WorkspaceInfo`) references the evicted object needs to also be evicted, in order to prevent stale object references being cached. Previously, a multi-map of such references was maintained as a reverse index. But it had a bug where for example, renaming a workspace wouldn't evict the styles for that workspace. Additionally, it could lead to memory leaks. This patch replaces it by partial traversal of the cache entries (see CachedReferenceCleaner), and fixes both issues. The performance penalty is sub-millisecond to < 10ms on a large catalog (100k layers, 100k styles, 20k stores, 20k workspaces).
- Loading branch information
Showing
3 changed files
with
222 additions
and
143 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
215 changes: 215 additions & 0 deletions
215
...catalog/cache/src/main/java/org/geoserver/cloud/catalog/cache/CachedReferenceCleaner.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,215 @@ | ||
/* | ||
* (c) 2024 Open Source Geospatial Foundation - all rights reserved This code is licensed under the | ||
* GPL 2.0 license, available at the root application directory. | ||
*/ | ||
package org.geoserver.cloud.catalog.cache; | ||
|
||
import com.github.benmanes.caffeine.cache.Cache; | ||
import com.google.common.base.Stopwatch; | ||
|
||
import lombok.Getter; | ||
import lombok.NonNull; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
|
||
import org.geoserver.catalog.CatalogInfo; | ||
import org.geoserver.catalog.LayerGroupInfo; | ||
import org.geoserver.catalog.LayerInfo; | ||
import org.geoserver.catalog.NamespaceInfo; | ||
import org.geoserver.catalog.PublishedInfo; | ||
import org.geoserver.catalog.ResourceInfo; | ||
import org.geoserver.catalog.StoreInfo; | ||
import org.geoserver.catalog.StyleInfo; | ||
import org.geoserver.catalog.WorkspaceInfo; | ||
import org.geoserver.catalog.plugin.AbstractCatalogVisitor; | ||
import org.springframework.lang.Nullable; | ||
|
||
import java.util.concurrent.ConcurrentMap; | ||
import java.util.concurrent.atomic.AtomicInteger; | ||
|
||
@RequiredArgsConstructor | ||
@Slf4j(topic = "org.geoserver.cloud.catalog.cache") | ||
class CachedReferenceCleaner extends AbstractCatalogVisitor { | ||
|
||
@NonNull private final Cache<?, ?> caffeine; | ||
|
||
/** | ||
* Evicts ant {@link CatalogInfo} that has a reference to the evicted {@code InfoKey}, to avoid | ||
* having cached entries with references to stale objects that have been modified | ||
* | ||
* <p>For example, a when {@code InfoIdKey} targets a {@link WorkspaceInfo}, it'll also evict | ||
* all {@link StyleInfo}s, {@link LayerGroupInfo}s, etc that reference the workspace. | ||
* | ||
* @param evicted | ||
*/ | ||
public void cascadeEvict(InfoIdKey evicted) { | ||
var cache = caffeine.asMap(); | ||
var initialSize = cache.size(); | ||
var sw = Stopwatch.createStarted(); | ||
final AtomicInteger visited = new AtomicInteger(); | ||
int cascadedEvictCount; | ||
try { | ||
cascadedEvictCount = cascadeEvict(evicted, cache, visited); | ||
} catch (RuntimeException e) { | ||
log.warn( | ||
"Error cascade-evicting cached entries referencing {}, clearing out the whole cache", | ||
evicted, | ||
e); | ||
caffeine.invalidateAll(); | ||
return; | ||
} | ||
sw.stop(); | ||
var finalSize = cache.size(); | ||
if (cascadedEvictCount > 0 || visited.intValue() > 0) | ||
log.debug( | ||
"cascade evicted {} entries referencing {} in {}. Size pre: {}, after: {}, visited: {}", | ||
cascadedEvictCount, | ||
evicted.id(), | ||
sw, | ||
initialSize, | ||
finalSize, | ||
visited); | ||
} | ||
|
||
@SuppressWarnings("java:S3864") // Stream.peek | ||
private int cascadeEvict(InfoIdKey idKey, ConcurrentMap<?, ?> cache, AtomicInteger visited) { | ||
return cache.values().parallelStream() | ||
.filter(CatalogInfo.class::isInstance) | ||
.map(CatalogInfo.class::cast) | ||
.filter(info -> this.canReference(info, idKey)) | ||
.peek(i -> visited.incrementAndGet()) | ||
.map(info -> CachedReferenceCleanerVisitor.cascadeEvict(cache, idKey, info)) | ||
.reduce(0, (c1, c2) -> c1 + c2); | ||
} | ||
|
||
/** | ||
* @return whether {@code cached} can directly or indirectly reference {@code evicted} | ||
*/ | ||
private boolean canReference(CatalogInfo cached, InfoIdKey evicted) { | ||
return switch (evicted.type()) { | ||
// evicted a workspace/namespace, any object by a workspace or namespace can be | ||
// referencing it | ||
case WORKSPACE, NAMESPACE -> !(cached instanceof WorkspaceInfo | ||
|| cached instanceof NamespaceInfo); | ||
// evicted a StoreInfo, any ResourceInfo or PublishedInfo can be referencing it | ||
case COVERAGESTORE, DATASTORE, WMSSTORE, WMTSSTORE -> (cached instanceof ResourceInfo | ||
|| cached instanceof PublishedInfo); | ||
// evicted a ResourceInfo, only PublishedInfos may be referencing it | ||
case COVERAGE, FEATURETYPE, WMSLAYER, WMTSLAYER -> cached instanceof PublishedInfo; | ||
// evicted a LayerInfo, only LayerGroupInfos may reference it | ||
case LAYER -> cached instanceof LayerGroupInfo; | ||
// evicted a LayerGroupInfo, only other LayerGroupInfos may reference it | ||
case LAYERGROUP -> cached instanceof LayerGroupInfo; | ||
// evicted a StyleInfo, only PublishedInfos may reference it | ||
case STYLE -> cached instanceof PublishedInfo; | ||
default -> false; | ||
}; | ||
} | ||
|
||
static CachedReferenceCleaner valueOf(org.springframework.cache.@NonNull Cache cache) { | ||
Object nativeCache = cache.getNativeCache(); | ||
if (nativeCache instanceof com.github.benmanes.caffeine.cache.Cache<?, ?> caffeineCache) { | ||
return new CachedReferenceCleaner(caffeineCache); | ||
} | ||
throw new UnsupportedOperationException( | ||
"Expected Caffeine cache, got unsupported cache implementation: %s" | ||
.formatted(nativeCache.getClass().getCanonicalName())); | ||
} | ||
|
||
@RequiredArgsConstructor | ||
private static class CachedReferenceCleanerVisitor extends AbstractCatalogVisitor { | ||
|
||
@NonNull private final ConcurrentMap<?, ?> cache; | ||
|
||
/** key for the evicted oject. Will evict any cached object that has a reference to it */ | ||
@NonNull private final InfoIdKey evictedKey; | ||
|
||
/** | ||
* The cached object being traversed, to be evicted if it has any nested reference to {@link | ||
* #evictedKey} | ||
*/ | ||
@NonNull private final CatalogInfo cached; | ||
|
||
/** | ||
* Number of cascaded evictions (0, 1 or 2 for {@link #cached}'s InfoIdKey and/or | ||
* InfoNameKey) | ||
*/ | ||
@Getter private int count; | ||
|
||
public static int cascadeEvict( | ||
ConcurrentMap<?, ?> cache, InfoIdKey evicted, CatalogInfo cached) { | ||
var visitor = new CachedReferenceCleanerVisitor(cache, evicted, cached); | ||
cached.accept(visitor); | ||
return visitor.getCount(); | ||
} | ||
|
||
private boolean accept(@Nullable CatalogInfo ref) { | ||
if (null != ref && evictedKey.id().equals(ref.getId())) { | ||
InfoIdKey idKey = InfoIdKey.valueOf(cached); | ||
InfoNameKey nameKey = InfoNameKey.valueOf(cached); | ||
evict(idKey); | ||
evict(nameKey); | ||
} | ||
return count > 0; | ||
} | ||
|
||
private void evict(Object key) { | ||
Object removed = cache.remove(key); | ||
if (null != removed) { | ||
++count; | ||
log.trace("cascade evicted {} referencing {}", key, evictedKey.id()); | ||
} | ||
} | ||
|
||
private void traverse(@Nullable CatalogInfo info) { | ||
if (null != info) info.accept(this); | ||
} | ||
|
||
public @Override void visit(WorkspaceInfo ws) { | ||
accept(ws); | ||
} | ||
|
||
public @Override void visit(NamespaceInfo ns) { | ||
accept(ns); | ||
} | ||
|
||
public @Override void visit(StoreInfo store) { | ||
accept(store); | ||
if (0 == count) traverse(store.getWorkspace()); | ||
} | ||
|
||
public @Override void visit(ResourceInfo r) { | ||
accept(r); | ||
if (0 == count) traverse(r.getNamespace()); | ||
if (0 == count) traverse(r.getStore()); | ||
} | ||
|
||
public @Override void visit(StyleInfo style) { | ||
accept(style); | ||
if (0 == count) traverse(style.getWorkspace()); | ||
} | ||
|
||
public @Override void visit(LayerInfo l) { | ||
accept(l); | ||
if (0 == count) traverse(l.getResource()); | ||
if (0 == count) traverse(l.getDefaultStyle()); | ||
if (0 == count) l.getStyles().forEach(this::traverse); | ||
} | ||
|
||
public @Override void visit(LayerGroupInfo lg) { | ||
accept(lg); | ||
if (0 == count) traverse(lg.getWorkspace()); | ||
if (0 == count) traverse(lg.getRootLayer()); | ||
if (0 == count) traverse(lg.getRootLayerStyle()); | ||
if (0 == count) lg.getLayers().forEach(this::traverse); | ||
if (0 == count) lg.getStyles().forEach(this::traverse); | ||
if (0 == count) | ||
lg.getLayerGroupStyles() | ||
.forEach( | ||
lgs -> { | ||
lgs.getStyles().forEach(this::traverse); | ||
lgs.getLayers().forEach(this::traverse); | ||
}); | ||
} | ||
} | ||
} |
Oops, something went wrong.