Skip to content

Commit 1f582a0

Browse files
epeartreepeartree
authored andcommitted
5621 deadlock on caffeine cache when creating a resource with conditional create (#5622)
* Modifying the CacheProvider to avoid doing db I/O within the cache miss value supplier callback. * Setting the initial capacity of instantiated caches to the cache max size to avoid resizing during operations. * adding changelog and spotless. * Fixing typo. * Addressing comments from code review. --------- Co-authored-by: peartree <[email protected]>
1 parent c4d2af8 commit 1f582a0

File tree

3 files changed

+23
-4
lines changed

3 files changed

+23
-4
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
type: fix
3+
issue: 5621
4+
title: "Fixed a deadlock in resource conditional create."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,19 @@ public IIdType translatePidIdToForcedId(FhirContext theCtx, String theResourceTy
480480

481481
@Override
482482
public Optional<String> translatePidIdToForcedIdWithCache(JpaPid theId) {
483-
return myMemoryCacheService.get(
484-
MemoryCacheService.CacheEnum.PID_TO_FORCED_ID,
485-
theId.getId(),
486-
pid -> myResourceTableDao.findById(pid).map(ResourceTable::asTypedFhirResourceId));
483+
// do getIfPresent and then put to avoid doing I/O inside the cache.
484+
Optional<String> forcedId =
485+
myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.PID_TO_FORCED_ID, theId.getId());
486+
487+
if (forcedId == null) {
488+
// This is only called when we know the resource exists.
489+
// So this optional is only empty when there is no hfj_forced_id table
490+
// note: this is obsolete with the new fhir_id column, and will go away.
491+
forcedId = myResourceTableDao.findById(theId.getId()).map(ResourceTable::asTypedFhirResourceId);
492+
myMemoryCacheService.put(MemoryCacheService.CacheEnum.PID_TO_FORCED_ID, theId.getId(), forcedId);
493+
}
494+
495+
return forcedId;
487496
}
488497

489498
private ListMultimap<String, String> organizeIdsByResourceType(Collection<IIdType> theIds) {

hapi-fhir-serviceloaders/hapi-fhir-caching-caffeine/src/main/java/ca/uhn/fhir/sl/cache/caffeine/CacheProvider.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,19 @@ public LoadingCache<K, V> create(long timeoutMillis, CacheLoader<K, V> loading)
4444
public Cache<K, V> create(long timeoutMillis, long maximumSize) {
4545
return new CacheDelegator<K, V>(Caffeine.newBuilder()
4646
.expireAfterWrite(timeoutMillis, TimeUnit.MILLISECONDS)
47+
// Caffeine locks the whole array when growing the hash table.
48+
// Set initial capacity to max to avoid this. All our caches are <1M entries.
49+
.initialCapacity((int) maximumSize)
4750
.maximumSize(maximumSize)
4851
.build());
4952
}
5053

5154
public LoadingCache<K, V> create(long timeoutMillis, long maximumSize, CacheLoader<K, V> loading) {
5255
return new LoadingCacheDelegator<K, V>(Caffeine.newBuilder()
5356
.expireAfterWrite(timeoutMillis, TimeUnit.MILLISECONDS)
57+
// Caffeine locks the whole array when growing the hash table.
58+
// Set initial capacity to max to avoid this. All our caches are <1M entries.
59+
.initialCapacity((int) maximumSize)
5460
.maximumSize(maximumSize)
5561
.build(loading::load));
5662
}

0 commit comments

Comments
 (0)