github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4059039990
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -281,6 +288,7 @@ public void refreshPartitions(String catalogName, String
dbName, String tableNam
for (String partitionName : partitionNames) {
cache.invalidatePartitionCache(externalTable, partitionName);
}
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(externalTable);
((HMSExternalTable) table).setUpdateTime(updateTime);
Review Comment:
[P1] Fence the non-rename ALTER_PARTITION path before touching Hive caches.
This method is called from AlterPartitionEvent after HMS has already committed
the mutation, but hive(...) and each invalidatePartitionCache(...) run before
this new fence and can throw while resolving/decoding or mutating cache state.
MetastoreEventsProcessor then rewinds/retries the event while the old row-count
future remains publishable. Move the table fence before engine lookup/the loop
(or make it finally-safe) and cover a throwing ALTER_PARTITION refresh. This is
distinct from the existing insert/replay/DROP ordering thread: the non-rename
ALTER path still has the old ordering.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1228,10 +1238,24 @@ public void unregisterDatabase(String dbName) {
if (LOG.isDebugEnabled()) {
LOG.debug("unregister database [{}]", dbName);
}
- if (isInitialized()) {
- metaCache.invalidate(dbName, Util.genIdByName(name, dbName));
+ // Resolve the canonical database object before removing it from the
local metadata cache.
+ // The row-count cache can outlive that object and must be invalidated
by its numeric id.
+ boolean catalogInitialized = isInitialized();
+ Optional<ExternalDatabase<? extends ExternalTable>> db =
catalogInitialized
+ ? getDbForReplay(dbName) : Optional.empty();
+ String localDbName =
db.map(ExternalDatabase::getFullName).orElse(dbName);
+ long dbId = db.map(ExternalDatabase::getId).orElseGet(() ->
Util.genIdByName(name, localDbName));
+ try {
+ if (db.isPresent()) {
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(),
dbId, localDbName);
+ } else {
+ Env.getCurrentEnv().getExtMetaCacheMgr().invalidateDb(getId(),
dbName);
+ }
+ } finally {
+ if (catalogInitialized) {
+ metaCache.invalidate(localDbName, dbId);
+ }
Review Comment:
[P2] Avoid invalidating a resident DB twice. The explicit invalidateDb call
already performs routed invalidation and an O(N), write-locked row-count DB
scan; this synchronous MetaCache removal then invokes the catalog removal
listener, whose resetMetaToUninitialized(true) calls the same manager overload
and repeats both operations. Keep the explicit call for the new failure-safe
ordering, but remove the object through a callback path that only resets local
state, and test that resident DROP DATABASE has one invalidation owner.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1228,10 +1238,24 @@ public void unregisterDatabase(String dbName) {
if (LOG.isDebugEnabled()) {
LOG.debug("unregister database [{}]", dbName);
}
- if (isInitialized()) {
- metaCache.invalidate(dbName, Util.genIdByName(name, dbName));
+ // Resolve the canonical database object before removing it from the
local metadata cache.
+ // The row-count cache can outlive that object and must be invalidated
by its numeric id.
+ boolean catalogInitialized = isInitialized();
+ Optional<ExternalDatabase<? extends ExternalTable>> db =
catalogInitialized
+ ? getDbForReplay(dbName) : Optional.empty();
+ String localDbName =
db.map(ExternalDatabase::getFullName).orElse(dbName);
+ long dbId = db.map(ExternalDatabase::getId).orElseGet(() ->
Util.genIdByName(name, localDbName));
Review Comment:
[P2] Preserve the canonical name even when the DB object is cold. HMS events
lowercase dbName, while lower_case_database_names=2 stores the mixed-case name
in MetaCache. getDbForReplay resolves that canonical key but returns empty
after object eviction, and this fallback discards the resolved spelling; the
finally then invalidates the lowercase key/hash, so the canonical names/id
entries survive and the dropped DB remains listed until a names refresh. Carry
the resolved local name independently of object presence and add a cold
mixed-case DROP_DATABASE regression.
--
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]
