github-actions[bot] commented on code in PR #67417:
URL: https://github.com/apache/doris/pull/67417#discussion_r3930706701
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -634,9 +640,13 @@ public void resetToUninitialized(boolean invalidCache) {
this.cachedConf = null;
}
this.lowerCaseToDatabaseName.clear();
+ refreshMetaCacheOnly();
Review Comment:
[P1] Fence callers that passed initialization before reset
This invalidation still leaves a post-check lifecycle window. `getDbNames()`
releases the catalog monitor after `makeSureInitialized()` and only then enters
`metaCache.listNames()`. A caller paused in that gap can resume after this
generation bump but while `onClose()` is still retiring the old client; its
load is created at the new generation, so both `MetaCache` publication guards
accept names read through that retiring client. This is durable during property
changes because the later reinitialization does not invalidate the accepted
snapshot again. Please tie load admission/publication to the catalog lifecycle
epoch (or hold a read fence from the initialization check through the cache
operation), and add a regression that pauses immediately after the check,
advances reset through invalidation, then resumes while close is paused.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -634,9 +640,13 @@ public void resetToUninitialized(boolean invalidCache) {
this.cachedConf = null;
}
this.lowerCaseToDatabaseName.clear();
+ refreshMetaCacheOnly();
Review Comment:
[P1] Keep object-cache invalidation outside the catalog monitor
This call does more than fence the names snapshot:
`MetaCache.invalidateAll()` reaches the Caffeine
`metaObjCache.invalidateAll()`. A concurrent missing-DB `metaObjCache.get()`
owns Caffeine's per-key computation while its `buildDbForInit(..., true)`
loader calls `getDbNames()` and waits in `makeSureInitialized()` for this
catalog monitor. Reset now holds the monitor here and waits for that same
computation to be removed, so the two threads deadlock. The updated deadlock
test moves its synthetic invalidator after reset, while the new fence harness
never constructs the real `metaCache`, so neither covers this path. Please keep
only names/routing publication inside the initialization fence and invalidate
object/downstream caches after releasing it, with a real `MetaCache`
paused-loader regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCache.java:
##########
@@ -180,6 +443,54 @@ public void addObjForTest(long id, String name, T db) {
* Should only be used after creating new database/table
*/
public void resetNames() {
- namesCache.invalidateAll();
+ synchronized (namesMutationLock) {
+ minimumLoadGeneration = advanceNamesGeneration();
+ namesCache.invalidateAll();
+ }
+ }
+
+ private long advanceNamesGeneration() {
+ long generation = namesGeneration.incrementAndGet();
+ if (activeNamesLoad != null && activeNamesLoad.generation <
generation) {
+ activeNamesLoad.result.complete(null);
Review Comment:
[P1] Bound obsolete foreground loads as well
Completing this future releases joiners, but it does not stop the owner
still blocked in `namesCacheLoader.load()`, and clearing the only pointer lets
the next generation start another physical foreground load. Repeating an
event/reset plus a lookup while each prior connector call is hung therefore
accumulates one RPC and occupied request thread per generation until external
process limits are exhausted. `MAX_NAMES_LOAD_ATTEMPTS` does not help because
each owner is stuck in its first attempt, and `MAX_NAMES_REFRESH_FLIGHTS` only
counts asynchronous refreshes. Please retain/cancel and bound all physical
name-load owners while reserving progress for the newest generation, and test
several generation advances with every older foreground loader paused.
--
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]
