github-actions[bot] commented on code in PR #67417:
URL: https://github.com/apache/doris/pull/67417#discussion_r3934598803
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCache.java:
##########
@@ -126,27 +460,37 @@ public Optional<T> getMetaObjById(long id) {
}
public void updateCache(String remoteName, String localName, T obj, long
id) {
- metaObjCache.put(localName, Optional.of(obj));
- namesCache.asMap().compute("", (k, v) -> {
- if (v == null) {
- return Lists.newArrayList(Pair.of(remoteName, localName));
- } else {
- v.add(Pair.of(remoteName, localName));
- return v;
+ updateCache(remoteName, localName, obj, id,
namesLoadEpochSupplier.getAsLong());
+ }
+
+ public boolean updateCache(String remoteName, String localName, T obj,
long id, long expectedEpoch) {
+ synchronized (namesMutationLock) {
+ if (!namesLoadEpochValidator.test(expectedEpoch)) {
+ return false;
}
- });
- idToName.put(id, localName);
+ metaObjCache.put(localName, Optional.of(obj));
Review Comment:
[P1] Keep the object-cache put out of the names lock
`metaObjCache.get(localName)` runs its loader inside Caffeine's same-key map
computation. Both production loaders re-enter `listNames()` (through
`buildDbForInit(..., true)` or `buildTableForInit(..., true)`) before
completing. If that loader pauses just before the re-entry while an HMS create
event reaches this line, the event owns `namesMutationLock` and blocks in this
put waiting for the computation, while the loader blocks acquiring
`namesMutationLock`, so neither can finish. This is distinct from the existing
reset/catalog-monitor deadlock thread because this cycle is introduced by event
publication here. Please avoid holding `namesMutationLock` across the Caffeine
mutation while preserving the epoch publication fence, and add same-key
object-load/create-event regressions for catalog and table caches.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCache.java:
##########
@@ -74,24 +160,272 @@ public MetaCache(String name,
maxSize,
true,
null);
- namesCache = namesCacheFactory.buildCache(namesCacheLoader, executor);
+ namesCache = namesCacheFactory.buildCache();
// Use sync removal listener to prevent deadlock (removal listener
calls invalidateAll)
// NOTE: This cache should NOT use refreshAfterWrite, as it would
become synchronous
metaObjCache =
objCacheFactory.buildCacheWithSyncRemovalListener(metaObjCacheLoader,
removalListener);
}
public List<String> listNames() {
- return
Objects.requireNonNull(namesCache.get("")).stream().map(Pair::value).collect(Collectors.toList());
+ return
getNames(false).stream().map(Pair::value).collect(Collectors.toList());
+ }
+
+ public List<String> refreshNames() {
+ NamesLoad loadInProgress;
+ synchronized (namesMutationLock) {
+ loadInProgress = activeNamesLoad;
+ }
+ if (loadInProgress != null) {
+ try {
+ awaitNamesLoad(loadInProgress);
+ } catch (RuntimeException ignored) {
Review Comment:
[P2] Do not swallow a forced-refresh waiter's interrupt
`awaitNamesLoad()` restores the flag and wraps `InterruptedException`, but
this catch discards that exception. If the old owner clears `activeNamesLoad`
before this thread enters `getNames(true)`, the interrupted query becomes the
next owner and runs `namesCacheLoader.load()` synchronously; socket-based
connector I/O may ignore a pre-set interrupt and remain blocked after
cancellation. This is distinct from the existing loader-interrupt thread: the
loader did not throw here, because the new wait path consumed the caller's
interruption. Please propagate waiter interruption instead of treating it as an
old-load failure, and test that interrupting a forced-refresh waiter cannot
start a replacement load.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
