Skip to content

Commit fb92029

Browse files
committed
[fix](fe) Fence metadata object cache invalidation
### What problem does this PR solve? Issue Number: None Related PR: #65126 Problem Summary: External metadata object loading previously ran inside a Caffeine same-key computation. A database reset could wait on Caffeine invalidation while the loader waited on the database monitor, causing the FE unit-test deadlock. Moving the loader outside that computation also requires an explicit publication fence so bulk invalidation cannot split an event object from its name and ID routes or acknowledge an event against a retired database object. Use exact per-name locks for same-object operations and a short lifecycle generation fence for cache publication. Bulk invalidation atomically swaps in a new object cache, then runs removal callbacks on the retired cache outside the lifecycle lock. Loads validate their generation before publishing, and event actions revalidate the object identity afterwards. ### Release note Prevent external catalog reset deadlocks and inconsistent metadata object publication during concurrent HMS events. ### Check List (For Author) - Test: Unit Test and FE build - MetaCacheTest - MetaCacheDeadlockTest - ExternalCatalogDeadlockTest - ExternalTableNameComparedLowercaseMetaCacheTrueTest - ExternalDatabaseNameComparedLowercaseMetaCacheTrueTest - FE Checkstyle - ./build.sh --fe - Behavior changed: Yes. External metadata reset and event publication are serialized by cache generation without holding lifecycle locks during connector loads or removal callbacks. - Does this need documentation: No
1 parent e1c0955 commit fb92029

2 files changed

Lines changed: 258 additions & 61 deletions

File tree

‎fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCache.java‎

Lines changed: 166 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,18 @@
3737
import java.util.OptionalLong;
3838
import java.util.concurrent.CompletableFuture;
3939
import java.util.concurrent.CompletionException;
40+
import java.util.concurrent.ConcurrentMap;
4041
import java.util.concurrent.ExecutionException;
4142
import java.util.concurrent.ExecutorService;
4243
import java.util.concurrent.TimeUnit;
43-
import java.util.concurrent.atomic.AtomicBoolean;
4444
import java.util.concurrent.atomic.AtomicLong;
45+
import java.util.concurrent.locks.ReentrantReadWriteLock;
4546
import java.util.function.BiConsumer;
4647
import java.util.function.BooleanSupplier;
4748
import java.util.function.Consumer;
4849
import java.util.function.LongPredicate;
4950
import java.util.function.LongSupplier;
51+
import java.util.function.Supplier;
5052
import java.util.stream.Collectors;
5153

5254
public class MetaCache<T> {
@@ -74,7 +76,16 @@ public class MetaCache<T> {
7476
//Pair<String, String> : <Remote name, Local name>
7577
private Map<Long, String> idToName = Maps.newConcurrentMap();
7678
private final CacheLoader<String, Optional<T>> metaObjCacheLoader;
79+
private final CacheFactory metaObjCacheFactory;
80+
private final RemovalListener<String, Optional<T>> metaObjRemovalListener;
7781
private LoadingCache<String, Optional<T>> metaObjCache;
82+
// Bulk invalidation retires the current cache generation before running removal callbacks.
83+
// Loads may run without this lock, but may publish only into the generation they started in.
84+
private final ReentrantReadWriteLock metaObjLifecycleLock = new ReentrantReadWriteLock();
85+
private final AtomicLong metaObjGeneration = new AtomicLong();
86+
// Object loads and event mutations for the same name share one lock. Locks are reference-counted
87+
// so a blocked connector call never serializes unrelated names or leaves an unbounded lock map.
88+
private final ConcurrentMap<String, MetaObjKeyLock> metaObjKeyLocks = Maps.newConcurrentMap();
7889

7990
private String name;
8091

@@ -145,6 +156,7 @@ public MetaCache(String name,
145156
this.namesRefreshAfterWriteNanos = refreshAfterWriteSec.isPresent()
146157
? TimeUnit.SECONDS.toNanos(refreshAfterWriteSec.getAsLong()) : Long.MAX_VALUE;
147158
this.metaObjCacheLoader = metaObjCacheLoader;
159+
this.metaObjRemovalListener = removalListener;
148160

149161
// ATTN:
150162
// The refreshAfterWriteSec is only used for metaObjCache, not for namesCache.
@@ -158,7 +170,7 @@ public MetaCache(String name,
158170
1, // names cache has one and only one entry
159171
true,
160172
null);
161-
CacheFactory objCacheFactory = new CacheFactory(
173+
metaObjCacheFactory = new CacheFactory(
162174
expireAfterAccessSec,
163175
OptionalLong.empty(),
164176
maxSize,
@@ -167,7 +179,7 @@ public MetaCache(String name,
167179
namesCache = namesCacheFactory.buildCache();
168180
// Use sync removal listener to prevent deadlock (removal listener calls invalidateAll)
169181
// NOTE: This cache should NOT use refreshAfterWrite, as it would become synchronous
170-
metaObjCache = objCacheFactory.buildCacheWithSyncRemovalListener(metaObjCacheLoader, removalListener);
182+
metaObjCache = buildMetaObjCache();
171183
}
172184

173185
public List<String> listNames() {
@@ -452,20 +464,41 @@ private void publishNames(NamesCacheValue value) {
452464
}
453465

454466
public Optional<T> getMetaObj(String name, long id) {
455-
Optional<T> val = metaObjCache.getIfPresent(name);
467+
Optional<T> val = withMetaObjLifecycleReadLock(() -> metaObjCache.getIfPresent(name));
456468
if (val != null && val.isPresent()) {
457469
return val;
458470
}
459-
if (LOG.isDebugEnabled()) {
460-
LOG.debug("trigger getMetaObj in metacache {}, obj name: {}, id: {}",
461-
this.name, name, id, new Exception());
462-
}
463-
// Use Caffeine's per-key computation so an Optional.empty() entry is retried once for
464-
// concurrent callers without invalidating a value published by another same-key load.
465-
val = metaObjCache.asMap().compute(name,
466-
(key, current) -> current != null && current.isPresent() ? current : loadMetaObj(key));
467-
idToName.put(id, name);
468-
return val;
471+
return withMetaObjKeyLock(name, () -> {
472+
while (true) {
473+
long generation;
474+
metaObjLifecycleLock.readLock().lock();
475+
try {
476+
Optional<T> current = metaObjCache.getIfPresent(name);
477+
if (current != null && current.isPresent()) {
478+
return current;
479+
}
480+
generation = metaObjGeneration.get();
481+
} finally {
482+
metaObjLifecycleLock.readLock().unlock();
483+
}
484+
if (LOG.isDebugEnabled()) {
485+
LOG.debug("trigger getMetaObj in metacache {}, obj name: {}, id: {}",
486+
this.name, name, id, new Exception());
487+
}
488+
Optional<T> loaded = loadMetaObj(name);
489+
metaObjLifecycleLock.readLock().lock();
490+
try {
491+
if (generation != metaObjGeneration.get()) {
492+
continue;
493+
}
494+
metaObjCache.put(name, loaded);
495+
idToName.put(id, name);
496+
return loaded;
497+
} finally {
498+
metaObjLifecycleLock.readLock().unlock();
499+
}
500+
}
501+
});
469502
}
470503

471504
private Optional<T> loadMetaObj(String key) {
@@ -482,15 +515,15 @@ private Optional<T> loadMetaObj(String key) {
482515
}
483516

484517
public Optional<T> tryGetMetaObj(String name) {
485-
Optional<T> val = metaObjCache.getIfPresent(name);
518+
Optional<T> val = withMetaObjLifecycleReadLock(() -> metaObjCache.getIfPresent(name));
486519
if (val == null || !val.isPresent()) {
487520
return Optional.empty();
488521
}
489522
return val;
490523
}
491524

492525
public Optional<T> getMetaObjById(long id) {
493-
String name = idToName.get(id);
526+
String name = withMetaObjLifecycleReadLock(() -> idToName.get(id));
494527
return name == null ? Optional.empty() : getMetaObj(name, id);
495528
}
496529

@@ -499,57 +532,73 @@ public void updateCache(String remoteName, String localName, T obj, long id) {
499532
}
500533

501534
public boolean updateCache(String remoteName, String localName, T obj, long id, long expectedEpoch) {
502-
AtomicBoolean updated = new AtomicBoolean();
503-
metaObjCache.asMap().compute(localName, (key, currentObj) -> {
504-
synchronized (namesMutationLock) {
505-
if (!namesLoadEpochValidator.test(expectedEpoch)) {
506-
return currentObj;
535+
return withMetaObjKeyLock(localName, () -> {
536+
return withMetaObjLifecycleReadLock(() -> {
537+
synchronized (namesMutationLock) {
538+
if (!namesLoadEpochValidator.test(expectedEpoch)) {
539+
return false;
540+
}
541+
long generation = advanceNamesGeneration();
542+
NamesCacheValue currentNames = namesCache.getIfPresent("");
543+
Map<String, Pair<String, String>> names = currentNames == null
544+
? Maps.newLinkedHashMap() : currentNames.names;
545+
names.put(localName, Pair.of(remoteName, localName));
546+
namesCache.put("", new NamesCacheValue(
547+
generation, names, currentNames != null && currentNames.complete));
548+
nameUpdateAction.accept(remoteName, localName);
549+
metaObjCache.put(localName, Optional.of(obj));
550+
idToName.put(id, localName);
507551
}
508-
long generation = advanceNamesGeneration();
509-
NamesCacheValue currentNames = namesCache.getIfPresent("");
510-
Map<String, Pair<String, String>> names = currentNames == null
511-
? Maps.newLinkedHashMap() : currentNames.names;
512-
names.put(localName, Pair.of(remoteName, localName));
513-
namesCache.put("", new NamesCacheValue(
514-
generation, names, currentNames != null && currentNames.complete));
515-
nameUpdateAction.accept(remoteName, localName);
516-
idToName.put(id, localName);
517-
updated.set(true);
518-
}
519-
return Optional.of(obj);
552+
return true;
553+
});
520554
});
521-
return updated.get();
522555
}
523556

524-
// The action runs inside Caffeine's same-key computation and must not recursively mutate this MetaCache.
557+
// The action runs under the same-name mutation lock and must not mutate another name in this MetaCache.
558+
// A bulk invalidation may retire the cache while the action runs, so verify the generation afterwards.
525559
public boolean executeIfMetaObjCurrent(String localName, T expectedObj, BooleanSupplier action) {
526-
AtomicBoolean result = new AtomicBoolean();
527-
metaObjCache.asMap().computeIfPresent(localName, (key, cachedObj) -> {
528-
if (cachedObj.isPresent() && cachedObj.get() == expectedObj) {
529-
result.set(action.getAsBoolean());
560+
return withMetaObjKeyLock(localName, () -> {
561+
long generation;
562+
metaObjLifecycleLock.readLock().lock();
563+
try {
564+
Optional<T> cachedObj = metaObjCache.getIfPresent(localName);
565+
if (cachedObj == null || !cachedObj.isPresent() || cachedObj.get() != expectedObj) {
566+
return false;
567+
}
568+
generation = metaObjGeneration.get();
569+
} finally {
570+
metaObjLifecycleLock.readLock().unlock();
530571
}
531-
return cachedObj;
572+
boolean result = action.getAsBoolean();
573+
return result && withMetaObjLifecycleReadLock(() -> {
574+
Optional<T> cachedObj = metaObjCache.getIfPresent(localName);
575+
return generation == metaObjGeneration.get()
576+
&& cachedObj != null && cachedObj.isPresent() && cachedObj.get() == expectedObj;
577+
});
532578
});
533-
return result.get();
534579
}
535580

536581
public void invalidate(String localName, long id) {
537-
metaObjCache.asMap().compute(localName, (key, currentObj) -> {
538-
synchronized (namesMutationLock) {
539-
long generation = advanceNamesGeneration();
540-
NamesCacheValue currentNames = namesCache.getIfPresent("");
541-
if (currentNames != null) {
542-
currentNames.names.remove(localName);
543-
namesCache.put("", new NamesCacheValue(
544-
generation, currentNames.names, currentNames.complete));
582+
withMetaObjKeyLock(localName, () -> {
583+
withMetaObjLifecycleReadLock(() -> {
584+
synchronized (namesMutationLock) {
585+
long generation = advanceNamesGeneration();
586+
NamesCacheValue currentNames = namesCache.getIfPresent("");
587+
if (currentNames != null) {
588+
currentNames.names.remove(localName);
589+
namesCache.put("", new NamesCacheValue(
590+
generation, currentNames.names, currentNames.complete));
591+
}
592+
nameInvalidationAction.accept(localName);
593+
idToName.remove(id);
545594
}
546-
nameInvalidationAction.accept(localName);
547-
idToName.remove(id);
548-
}
549-
if (LOG.isDebugEnabled()) {
550-
LOG.debug("invalidate obj in metacache {}, obj name: {}, id: {}",
551-
name, localName, id, new Exception());
552-
}
595+
if (LOG.isDebugEnabled()) {
596+
LOG.debug("invalidate obj in metacache {}, obj name: {}, id: {}",
597+
name, localName, id, new Exception());
598+
}
599+
metaObjCache.invalidate(localName);
600+
return null;
601+
});
553602
return null;
554603
});
555604
}
@@ -566,8 +615,17 @@ public void invalidateObjects() {
566615
if (LOG.isDebugEnabled()) {
567616
LOG.debug("invalidate objects in metacache {}", name, new Exception());
568617
}
569-
idToName.clear();
570-
metaObjCache.invalidateAll();
618+
LoadingCache<String, Optional<T>> retiredCache;
619+
metaObjLifecycleLock.writeLock().lock();
620+
try {
621+
metaObjGeneration.incrementAndGet();
622+
idToName.clear();
623+
retiredCache = metaObjCache;
624+
metaObjCache = buildMetaObjCache();
625+
} finally {
626+
metaObjLifecycleLock.writeLock().unlock();
627+
}
628+
retiredCache.invalidateAll();
571629
}
572630

573631
public void invalidateAll() {
@@ -577,7 +635,7 @@ public void invalidateAll() {
577635

578636
@VisibleForTesting
579637
public LoadingCache<String, Optional<T>> getMetaObjCache() {
580-
return metaObjCache;
638+
return withMetaObjLifecycleReadLock(() -> metaObjCache);
581639
}
582640

583641
@VisibleForTesting
@@ -587,8 +645,51 @@ public void refreshNamesForTest() {
587645

588646
@VisibleForTesting
589647
public void addObjForTest(long id, String name, T db) {
590-
idToName.put(id, name);
591-
metaObjCache.put(name, Optional.of(db));
648+
withMetaObjLifecycleReadLock(() -> {
649+
metaObjCache.put(name, Optional.of(db));
650+
idToName.put(id, name);
651+
return null;
652+
});
653+
}
654+
655+
private LoadingCache<String, Optional<T>> buildMetaObjCache() {
656+
return metaObjCacheFactory.buildCacheWithSyncRemovalListener(
657+
metaObjCacheLoader, metaObjRemovalListener);
658+
}
659+
660+
private <R> R withMetaObjLifecycleReadLock(Supplier<R> action) {
661+
metaObjLifecycleLock.readLock().lock();
662+
try {
663+
return action.get();
664+
} finally {
665+
metaObjLifecycleLock.readLock().unlock();
666+
}
667+
}
668+
669+
private <R> R withMetaObjKeyLock(String key, Supplier<R> action) {
670+
MetaObjKeyLock keyLock = acquireMetaObjKeyLock(key);
671+
try {
672+
synchronized (keyLock) {
673+
return action.get();
674+
}
675+
} finally {
676+
releaseMetaObjKeyLock(key);
677+
}
678+
}
679+
680+
private MetaObjKeyLock acquireMetaObjKeyLock(String key) {
681+
return metaObjKeyLocks.compute(key, (ignored, current) -> {
682+
MetaObjKeyLock result = current == null ? new MetaObjKeyLock() : current;
683+
result.users++;
684+
return result;
685+
});
686+
}
687+
688+
private void releaseMetaObjKeyLock(String key) {
689+
metaObjKeyLocks.compute(key, (ignored, current) -> {
690+
current.users--;
691+
return current.users == 0 ? null : current;
692+
});
592693
}
593694

594695
/**
@@ -638,6 +739,10 @@ private List<Pair<String, String>> snapshot() {
638739
}
639740
}
640741

742+
private static class MetaObjKeyLock {
743+
private int users;
744+
}
745+
641746
private static class NamesLoad {
642747
private final long generation;
643748
private final long loadEpoch;

0 commit comments

Comments
 (0)